Broadening bug fixes and enhancements #32
Conversation
Codecov Report
@@ Coverage Diff @@
## main #32 +/- ##
==========================================
+ Coverage 76.25% 78.18% +1.92%
==========================================
Files 13 13
Lines 417 463 +46
==========================================
+ Hits 318 362 +44
- Misses 99 101 +2
Continue to review full report at Codecov.
|
- the first is some small math tweaks to use multiplication instead of division in a couple spots - the second involves a more significant overhaul. This commit introduces dynamic calculation of line windows. See the docstring of line_absorption for details.
|
These commits introduce changes to make line absorption much faster. There are both math tweaks and now, more importantly, line windows are dynamically calculated. This takes us from 15x to 2x slower than MOOG for the example linelist I've been using. Apologies for the absolute spaghetti going on between theses PRs. |
…ydrogen and helium. This modifies the `Line` type to work with ABO parameters, and uses values of Gamma instead of log(Gamma).
|
This commit introduces ABO broadening. It also refactors |
|
Sorry the github diff is messed up by my PR spaghetti. Would rebasing fix that? |
I've encountered this type of problem before. It's really annoying! When I encountered it, I ultimately opened a new PR. It looks like this approach might work (of course, you need to replace |
|
Great, I think that worked. Here's what's remaining for me to do.
|
Codecov Report
@@ Coverage Diff @@
## main #32 +/- ##
==========================================
+ Coverage 77.55% 78.97% +1.42%
==========================================
Files 13 13
Lines 450 509 +59
==========================================
+ Hits 349 402 +53
- Misses 101 107 +6
Continue to review full report at Codecov.
|
ajwheeler
changed the title
| @@ -65,12 +66,16 @@ function synthesize(atm, linelist, λs::AbstractVector{F}; metallicity::F=0.0, v | |||
| number_densities = molecular_equilibrium(MEQs, layer.temp, layer.number_density, | |||
| layer.electron_density) | |||
|
|
|||
| α[i, :] = line_absorption(linelist, λs, layer.temp, number_densities, atomic_masses, | |||
| partition_funcs, ionization_energies, vmic*1e5) | |||
| α_cntm = LinearInterpolation(cntmλs, | |||
There was a problem hiding this comment.
It might be useful to add a comment explaining that why we are allowing for extrapolation at all (i.e. to serve as as a heuristic for determining the window size of the lines). When I first saw this, I got very concerned (before I realized what was going on).
There was a problem hiding this comment.
I've added a comment to explain the interpolation, and I've tweaked the code to make extrapolation unnecessary.
Co-authored-by: Matthew Abruzzo <[email protected]>
Co-authored-by: Matthew Abruzzo <[email protected]>
Co-authored-by: Matthew Abruzzo <[email protected]>
|
I think this is ready to be merged. It's not just not obvious to me how to resolve the documentation merge conflict (related to line_profile) |
This first commit fixes a bug in how broadening parameters are used. The per-particle stark and vdW gamma are now multiplied by the appropriate number densities.
I plan on adding more advanced vdW broadening (ABO theory, see links in #1) in this PR as well. That would take into account the effect of temperature on pressure broadening, at least for lines with data available.
#30 should be merged before this.