Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct level populations for ionization and recombination processes #223

Merged
merged 7 commits into from
Mar 6, 2023

Conversation

wtbarnes
Copy link
Owner

@wtbarnes wtbarnes commented Feb 24, 2023

Fixes #24

This PR accomplishes two main things:

  1. It implements the correction to the level populations calculation by incorporating processes due to level-resolved ionization and recombination as stored in the cilvl and reclvl files as described in Landi et al. (2006)
  2. It fixes the interpolation of the ionization equilibrium by switching the interpolation from linear to a Piecewise Cubic Hermite Interpolating Polynomial (PCHIP) approach. This is more accurate over the full temperature range and even for small ionization fractions.

A few remaining TODOs:

  • Tests that compare level populations with and without correction factor (note that this may involve adding another file to the test dataset as few actually have cilvl and reclvl files). This should test that only the levels inlcuded in those files are modified in the level populations calculation.
  • Deal with divide-by-zero warning in numerator calculation in correction factor
  • Docstring for level populations
  • Clarifying comments where needed regarding interpolation methods
  • Run IDL comparison checks against continuum results to ensure ioneq interpolation has not caused a regression
  • Compare against Fe XX correction factor from v8 IDL code

@codecov-commenter
Copy link

codecov-commenter commented Feb 24, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.42 ⚠️

Comparison is base (c674d97) 91.67% compared to head (37fb084) 91.26%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #223      +/-   ##
==========================================
- Coverage   91.67%   91.26%   -0.42%     
==========================================
  Files          23       23              
  Lines        1947     2015      +68     
==========================================
+ Hits         1785     1839      +54     
- Misses        162      176      +14     
Impacted Files Coverage Δ
fiasco/ions.py 96.59% <100.00%> (+0.53%) ⬆️
fiasco/util/setup_db.py 64.83% <0.00%> (-15.39%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@wtbarnes
Copy link
Owner Author

wtbarnes commented Mar 2, 2023

Based on the test failures, it looks like the free-free and free-bound values have changed, likely due to the differences in how I'm interpolating the ioneq. The differences are surprisingly large which is worrying.

@wtbarnes
Copy link
Owner Author

wtbarnes commented Mar 3, 2023

For Mg 12, when comparing to the equivalent IDL correction factors sent by Peter, the calculation here shows good agreement,

image

image

@wtbarnes
Copy link
Owner Author

wtbarnes commented Mar 4, 2023

I've finally settled on using PchipInterpolator to perform the cubic interpolation of the ionization fractions in log-space. This seems to be best solution for balancing smoothness and reducing oscillations. see plots of Fe V, VI, and XX ionization fractions below:

image

image

@wtbarnes wtbarnes marked this pull request as ready for review March 6, 2023 00:16
@wtbarnes
Copy link
Owner Author

wtbarnes commented Mar 6, 2023

Comparing the correction factors as implemented here and those in CHIANTI IDL for Fe XX for T=10 MK and $n_e=10^{10}$ $\mathrm{cm}^{-3}$,

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add correction for ionization and recombination in level populations calculation
2 participants