Added line_ratio and example with EIS data#441
Added line_ratio and example with EIS data#441nabobalis wants to merge 4 commits intowtbarnes:mainfrom
Conversation
| # First download the precomputed EIS products. These two FITS files are hosted | ||
| # in a separate data repository so that this example does not need to run the | ||
| # EIS fitting step with `eispac <https://eispac.readthedocs.io/en/latest/>`__. | ||
| data_base_url = 'https://media.githubusercontent.com/media/nabobalis/data/main/fiasco' |
There was a problem hiding this comment.
Update URL when PR merged into sunpy/data
wtbarnes
left a comment
wtbarnes
left a comment
There was a problem hiding this comment.
I've left a few detailed comments, but am holding off on a full line-by-line review as a I have a couple of high-level comments.
The functionality for interpolating between the theoretical and observed ratios is out of scope for fiasco. It is easy enough to do this in 2-3 lines in the example gallery without a dedicated function in fiasco. Additionally, the theoretical line ratio method should be a separate function (it can go in fiasco/fiasco.py). I am worried about overcrowding of the Ion namespace and this is more of an "analysis" function rather than something directly related to properties of the ion.
There was a problem hiding this comment.
Some text to explain how and when the sav file was created
There was a problem hiding this comment.
This should be be removed. See
fiasco/fiasco/tests/idl/test_idl_ioneq.py
Line 26 in 3194c1a
There was a problem hiding this comment.
You would rather have people use IDL to run the tests over test files?
| "A `~fiasco.Transitions` object holding the information about transitions for this ion." | ||
| return Transitions(self.levels, self._wgfa, n_levels=self.n_levels) | ||
|
|
||
| def _transition_indices(self, transitions): |
| temperature : `~astropy.units.Quantity` or `None`, optional | ||
| Temperature grid on which to evaluate the ratio. If `None`, the | ||
| temperature grid of the current ion instance is used. |
There was a problem hiding this comment.
Get rid of this input. The result should always be evaluated on the ion temperature grid.
| numerator, denominator : `~astropy.units.Quantity` | ||
| Transition wavelength(s) for the numerator and denominator. If multiple | ||
| transitions are provided, their contribution functions are summed before | ||
| taking the ratio. |
There was a problem hiding this comment.
It should be possible to provide either an array of wavelengths or an array of strings representing the transition labels (see
Line 263 in 3194c1a
| density arrays or ``(n_temperature, 1)`` for ``couple_density_to_temperature=True``. | ||
| Points where the denominator vanishes are returned as `~numpy.nan`. | ||
| """ | ||
| ion = self if temperature is None else self._new_instance(temperature=temperature) |
There was a problem hiding this comment.
| ion = self if temperature is None else self._new_instance(temperature=temperature) |
nabobalis
force-pushed
the
ratio-feature
branch
3 times, most recently
from
b28fc65 to
6678590
Compare
nabobalis
changed the title
2 participants
This is my attempt with AI to add helpers into fiasco which will allow me to create https://soho.nascom.nasa.gov/solarsoft/iris/idl/sao/util/tian/iris_ne.pro inside irispy.
Maybe fixes #161