wtbarnes / fiasco

Python interface to the CHIANTI atomic database
http://fiasco.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
20 stars 15 forks source link

Fix the definition of hydrogenic and helium-like ions #252

Closed jwreep closed 5 months ago

jwreep commented 5 months ago

Fixes #251

Please review.

I don't think that this line https://github.com/wtbarnes/fiasco/blob/282b819f5a9b257db93d1d484fa63ada2cb0c610/fiasco/ions.py#L975 needs to be changed since the function it's embedded in is only called for heavier elements.

codecov-commenter commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (282b819) 90.56% compared to head (180fdc6) 90.56%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #252 +/- ## ======================================= Coverage 90.56% 90.56% ======================================= Files 36 36 Lines 2670 2670 ======================================= Hits 2418 2418 Misses 252 252 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jwreep commented 5 months ago

I'm not sure I understand that last check that's failing. Any idea what the issue is?

wtbarnes commented 5 months ago

The pre-commit job runs several style checks on the changes to ensure consistent code style across the package. I can fix that easily. You can also install the pre-commit hooks locally using these instructions: https://docs.sunpy.org/en/latest/dev_guide/contents/code_standards.html#formatting and it will fix the files for you.

jwreep commented 5 months ago

Thank you! I didn't know about that. I'll make sure to use it going forward.

wtbarnes commented 5 months ago

@pryoung also pointed out to me that the Fontes and Dere papers note that the fits to the Fontes cross-sections should be accurate for $Z\ge4$, but then there's an additional note that Li III and B V are better fit with the BT scaled cross-section data and that the Be IV cross-sections are fit using the B V cross-sections appropriately scaled by the ionization potential. Thus, the $Z\ge6$ requirement is presumably to ensure those three ions are not fit using Fontes and instead use the .diparams values.

Interestingly, .diparams values are available for C VI, despite the fact that that cross-section is fit using Fontes. A plot of these two cross-sections is shown below,

image