Fixed distance parameter not producing accurate SDEC plots #1442
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1442 +/- ##
==========================================
- Coverage 71.16% 71.13% -0.03%
==========================================
Files 67 67
Lines 5521 5523 +2
==========================================
Hits 3929 3929
- Misses 1592 1594 +2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Overall, looks good. Do we need a clause at line 501 for distances of 0, or negative values? Also, can we change the unit order for the flux axis to be erg/s/cm2/AA (just to be in line with convention)? Finally, can we make the units inside math mode not italicised?
|
This looks good to me. @jaladh-singhal I believe the reason this happened in the first place is that in my version of the code, I adjusted the weights in the calculation stage, and my code had the plotting and calculation done in the same place. When you separated the two I think the lum_to_flux got lost. |
jaladh-singhal
force-pushed
the
fix-distance
branch
from
5e73fd6 to
8635b2a
Compare
@jamesgillanders good point! Added one to raise ValueError
Done!
In code, I checked that we're doing nothing to make units look italicized - I think this is how matplotlib renders latex in labels by default. Do you have any example in notice where it does not italicize, we can check its code & use that if you want? |
…#1442) * Added lum_to_flux divison in element hist calculations * Raise error if distance <= 0 * changed unit order for the flux axis to be erg/s/cm2/AA * Instructed how to use luminosity case in error message
Added lum_to_flux division factor in emission and absorption elements' histogram calculations. And modified distance docstring to use any length unit since calculations automatically convert it to CGS cm.
Motivation and Context
Fixes #1440
How Has This Been Tested?
Plot looks as expected:
Types of changes
Checklist: