Expose parameter uncertainties #1597
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1597 +/- ##
==========================================
- Coverage 86.12% 86.09% -0.03%
==========================================
Files 94 94
Lines 9321 9330 +9
==========================================
+ Hits 8028 8033 +5
- Misses 1293 1297 +4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
The returned uncertainties from model fitting are now exposed, need to mess around with the styling a little more to improve the layout. I moved the parameter name/"fixed" checkbox to their own row since everything was getting way too squished horizontally, mainly I need to tighten things up vertically now I think.
|
rosteen
requested review from
duytnguyendtn,
javerbukh,
ojustino,
pllim and
kecnry
as code owners
|
I improved the UI slightly from the last screenshot, this is ready for review.
|
| # The submodels don't have uncertainties attached, only the compound model | ||
| if self._fitted_model.stds is not None: | ||
| std_name = temp_param[0]["name"] | ||
| if submodel_index is not None: | ||
| std_name = f"{std_name}_{submodel_index}" | ||
| temp_param[0]["std"] = self._fitted_model.stds[std_name] |
There was a problem hiding this comment.
this block confused me... what situation triggers this logic and what does it do?
There was a problem hiding this comment.
So, this is because in specutils the stds attribute is only populated on the top-level compound model, not in each of the component models, and they're labeled things like amplitude_0, amplitude_1. So we have to see which model component is 0, 1, etc, from the order of the submodel_names attribute in order to retrieve the correct uncertainties. It's a bit of a pain but fixing it would require some upstream refactoring.
| roundUncertainty(uncertainty) { | ||
| return uncertainty.toPrecision(2) |
There was a problem hiding this comment.
I think this is probably fine for now, but we might need to make it more advanced for some edge cases with really large/small uncertainties (or if we ever display values in scientific notation).
There was a problem hiding this comment.
Oooo I wish I knew this trick for the Angle PR. 😆
|
Also tested with specutils 1.7 (our min pinned version which does not have support for exposing uncertainties yet), and this just skips displaying them but does not raise any errors (which is what I'd expect). |
Co-authored-by: Kyle Conroy <kyleconroy@gmail.com>
!
Implements retrieving and displaying uncertainties from the fit models in the Model Fitting plugin. Opening as draft since I need to mess around with the plugin layout to accommodate the new information in a way that's visually appealing.