NuGrid / NuGridPy

Python tools to access and analyse NuGrid output data (including from MESA)
http://nugrid.github.io/NuGridPy
BSD 3-Clause "New" or "Revised" License
13 stars 10 forks source link

Panel plots #59

Closed OClarkson closed 6 years ago

fherwig commented 6 years ago

I have started to review this. The first problem I ran into was that the run I was trying to plot did not have species h2:

/user/fherwig/NuGridPy/nugridpy2/mesa.py in get(self, str_name)
    501         """
    502 
--> 503         column_array = self.data[:,self.cols[str_name]-1].astype('float')
    504         return column_array
    505 

KeyError: 'h2'

It is ok to have a default list of species, but the plotting function needs to check if the species is in fact available from the profile file, and if not just ignore. This can be done with a

try ...
...
except
...

structure.

I would also say that the user needs to have some control as to what species are plotted. One option would be to just look into the profile files with p.cols and plot all species available, but just checking which ones are there. That could be the option plot_species = 'All'. But I think for more control that option should also allow a sequence of species to be provided as a list.

fherwig commented 6 years ago

For ms2.other_profiles(p,show=True) I am getting this error:

---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
<ipython-input-15-f704e9b61afd> in <module>()
----> 1 ms2.other_profiles(p,show=True)

/user/fherwig/NuGridPy/nugridpy2/mesa.py in other_profiles(p, xlm, show, xaxis)
   3933 
   3934     # create subplot structure
-> 3935     t, ([ax1,ax2,ax3],[ax4, ax5,ax6]) = subplots(2, 3, sharex=False, sharey=False)
   3936 
   3937 # panel 1: burns: pp, cno, burn_c

NameError: name 'subplots' is not defined

You can access the notebook in which I have done these tests on wendi3.phys.uvic.ca in this directory: /user/fherwig/notebooks/M20Z0.0-analysis.ipynb

OClarkson commented 6 years ago

Ready to be checked again--all issues adressed. There is not an option for 'All' species yet.

fherwig commented 6 years ago

Feedback for ms2.abu_profiles

Much better! There is a lot of output a user does not need to see:

[['h1', 'he3', 'he4', 'li6', 'c12', 'c13', 'n14', 'n15', 'o16', 'o17', 'o18', 'f19'], ['ne20', 'ne21', 'ne22', 'na22', 'mg24', 'mg25', 'mg26', 'al26', 'al27', 'si28', 'si29', 'si30'], ['p31', 's32', 's33', 's34', 's36'], []]
[-0.12493874 -0.12493874 -0.12493874 ..., -0.44642843 -0.44642843
 -0.44642843]
[ -4.57503622  -4.57503622  -4.57503622 ..., -13.33499307 -13.33501164
 -13.33502634]
[-0.60210621 -0.60210621 -0.60210621 ..., -0.21317781 -0.21317781

We need ylimits as arguments as well. I would suggest you use one of the already used argument names to provide limits. the plot method uses limits=None while abu_chart used plotaxis=[0, 0, 0, 0] while elemental_abund uses , ylim=[-12, 0], and I am sure there are others. We do not need yet another one. So, please change xlm= to xlim= and include an option for ylim as well.

I really like the xaxis option!

In the title include a space before model number and after the = before the model number.

In my test it appears that there are 29 things that are plotted out of your default list. 2 panels Two panels are busy with 12 things each, and one panel has 5 species and the fourth is empty. Can you distribute what you have over the 4 panels? That would be cool!

If p is mesa_profile instance then p.get('mass')[0] gives you the mass of the outermost coordinate. That would be a better and dynamic default upper limit for xaxis, and it would also work well for xaxis option Eulerian.

The documentation says: Eulerian radius is radial coordinate, in Mm but the xaxis is in $R_{sun}$. I would indeed like Mm better.

Other profiles

Looks good!

in the convection velocity/diffusion plot the table $0forv_{conv}/cs$ should get some spaces between 0 and for and $v{conv}$. In that panel I do not see conv_vel_div_sound - maybe I am missing some output columns. But that is of course "Ma" - so maybe use that?

In the top-right panel it is not immediately clear what M and i labels mean. You could just suppress the legend and it would be clearer.

OK that's it for now.