XRT DEM Iterative Solver Module - #367
Conversation
|
Ensure the function checks for minimum required inputs (filters, observed intensities, etc.) and validates that all observation values are non-zero before proceeding. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #367 +/- ##
===========================================
- Coverage 87.97% 36.96% -51.02%
===========================================
Files 26 40 +14
Lines 1855 4415 +2560
Branches 110 338 +228
===========================================
Hits 1632 1632
- Misses 180 2740 +2560
Partials 43 43 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2041e0a to
bd4687b
Compare
…ng at each temperature
| monte_carlo_runs=0, | ||
| ) | ||
|
|
||
| x.validate_inputs() |
There was a problem hiding this comment.
Should validate_inputs() be changed to _validate_inputs()? Is it something that needs to be user-facing?
| minimum_bound_temperature=5.5, | ||
| maximum_bound_temperature=7.5, | ||
| logarithmic_temperature_step_size=0.1, |
There was a problem hiding this comment.
| minimum_bound_temperature=5.5, | |
| maximum_bound_temperature=7.5, | |
| logarithmic_temperature_step_size=0.1, | |
| **temperature_kwargs, |
And define temperature_kwargs = {"minimum_bound_temperature": 5.5, ...} outside of the function, so that we can use keyword argument unpacking here?
|
|
||
| x.create_logT_grid() | ||
|
|
||
| assert (x.logT[0], x.logT[-1], x.dlogT) == pytest.approx((5.5, 7.5, 0.1)) |
There was a problem hiding this comment.
| assert (x.logT[0], x.logT[-1], x.dlogT) == pytest.approx((5.5, 7.5, 0.1)) | |
| np.testing.assert_allclose( | |
| [x.logT[0], x.logT[-1], x.dlogT], | |
| [5.5, 7.5, 0.1], | |
| ) |
| dem.validate_inputs() | ||
|
|
||
|
|
||
| def test_create_logT_grid(): |
There was a problem hiding this comment.
This would be a good use case for pytest fixtures to avoid having repeated assertions, but it's probably not worth the effort changing here.
| x.create_logT_grid() | ||
| x._interpolate_responses_to_grid() | ||
|
|
||
| dem, modeled, chi2, result = x._solve_single_dem( |
There was a problem hiding this comment.
🎗️ I'm wondering if this would also be useful to think about adding upstream to the interface in sunkit-dem, especially if many DEM solvers are iterative. 🤔
| temperature_responses=responses, | ||
| ) | ||
| x.solve() | ||
| x.plot_dem() |
There was a problem hiding this comment.
🎗️ Is there a way to do something like a regression test for figures?
| def test_init_rejects_negative_max_iterations(): | ||
| """max_iterations=-100 must raise ValueError. | ||
| Expected: ValueError because max_iterations must be a positive integer. | ||
| """ | ||
| filters = ["Al-poly", "Ti-poly"] | ||
| intensities = np.array([500.0, 800.0]) | ||
| responses = generate_temperature_responses(filters, "2012-10-27T00:00:00") | ||
|
|
||
| with pytest.raises(ValueError, match="max_iterations must be a positive integer"): | ||
| XRTDEMIterative(filters, intensities, responses, max_iterations=-100) | ||
|
|
||
|
|
||
| def test_init_rejects_zero_normalization_factor(): | ||
| """normalization_factor=0 must raise ValueError. | ||
| Expected: ValueError because normalization_factor must be positive. | ||
| """ | ||
| filters = ["Al-poly", "Ti-poly"] | ||
| intensities = np.array([500.0, 800.0]) | ||
| responses = generate_temperature_responses(filters, "2012-10-27T00:00:00") | ||
|
|
||
| with pytest.raises(ValueError, match="normalization_factor must be a positive"): | ||
| XRTDEMIterative(filters, intensities, responses, normalization_factor=0) |
There was a problem hiding this comment.
We might be able to use @pytest.mark.parametrize for some of these. 🤔
| filters = ["Al-poly", "Ti-poly", "Be-thin"] | ||
| intensities = np.array([100.0, 200.0, 300.0], dtype=float) |
There was a problem hiding this comment.
Some of these be defined outside of the test functions to reduce duplication.
Co-authored-by: Nick Murphy <namurphy@users.noreply.github.com>
Removing space. Co-authored-by: Nick Murphy <namurphy@users.noreply.github.com>
Removing space pt2. Co-authored-by: Nick Murphy <namurphy@users.noreply.github.com>
Nice touch. Co-authored-by: Nick Murphy <namurphy@users.noreply.github.com>
Co-authored-by: Nick Murphy <namurphy@users.noreply.github.com>
Co-authored-by: Nick Murphy <namurphy@users.noreply.github.com>
Co-authored-by: Nick Murphy <namurphy@users.noreply.github.com>
Co-authored-by: Nick Murphy <namurphy@users.noreply.github.com>
Co-authored-by: Nick Murphy <namurphy@users.noreply.github.com>
Co-authored-by: Nick Murphy <namurphy@users.noreply.github.com>
Co-authored-by: Nick Murphy <namurphy@users.noreply.github.com>
Co-authored-by: Nick Murphy <namurphy@users.noreply.github.com>
|
|
||
| I_i^{\mathrm{model}} | ||
| = \sum_j \mathrm{DEM}(T_j)\,R_i(T_j)\,T_j\,\Delta(\ln T) | ||
|
|
There was a problem hiding this comment.
One strategy I've used to document folders in GitHub is to include a README.md file in them, like for PlasmaPy in .github/workflows. That way, it gets rendered at the bottom of the GitHub page for that directory.
There was a problem hiding this comment.
Since (I think) these are interactive tests that are not meant to be run by pytest, my inclination would be to put them in a separate top-level folder if we still want to keep them in the repository — which would probably be worthwhile so long as the these are still valid and working.
We'd probably want to prune these from MANIFEST.in since they don't need to be included in the wheels that get distributed. 🛞
2 participants
Summary
This PR introduces
XRTDEMIterative, a Python implementation of the IDLroutine
xrt_dem_iterative2.profor estimating Differential EmissionMeasures (DEMs) from Hinode/XRT multi-filter observations.
What's Included
Core Solver (
dem_solver.py)XRTDEMIterativeclass with full DEM solving pipelinefitting via
lmfitmax(0.03 * intensity, 2 DN/s/pix), matching IDL default behaviormonte_carlo_runsparameterintensities, missing errors)
summary()method for diagnostic outputPlotting (
dem_plotting.py)plot_dem()— base DEM solution visualizationplot_dem_mc()— base DEM + Monte Carlo ensemble overlayTests
response interpolation, spline system, residuals, solver, and
Monte Carlo behavior
Documentation
dem_overview.rst— introduction to DEM analysis with XRTpyxrtpy.xrt_dem_iterativedocs/changelog/0.6.0.rstValidation -
xrt_dem_iterative2.proacross multiplesolar observation cases using Monte Carlo comparison