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

Adds accessor for the 3D fit parameters in CubeViz #263

Merged
merged 5 commits into from
Sep 10, 2020
Merged

Adds accessor for the 3D fit parameters in CubeViz #263

merged 5 commits into from
Sep 10, 2020

Conversation

ibusko
Copy link
Contributor

@ibusko ibusko commented Sep 2, 2020

This implements the second simplest solution as proposed in #241 : a raw attribute in the helper, but with individual parameters accessed by name .

This makes the 3D fitter usable in a notebook.

The Cube_fit notebook in the concepts directory exercises this functionality.

@codecov
Copy link

codecov bot commented Sep 2, 2020

Codecov Report

Merging #263 into master will decrease coverage by 0.33%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #263      +/-   ##
==========================================
- Coverage   64.78%   64.45%   -0.34%     
==========================================
  Files          44       45       +1     
  Lines        2272     2307      +35     
==========================================
+ Hits         1472     1487      +15     
- Misses        800      820      +20     
Impacted Files Coverage Δ
...igs/default/plugins/model_fitting/model_fitting.py 40.55% <0.00%> (-0.23%) ⬇️
jdaviz/configs/cubeviz/helper.py 66.66% <40.00%> (-33.34%) ⬇️
...s/default/plugins/model_fitting/fitting_backend.py 85.71% <100.00%> (+0.15%) ⬆️
jdaviz/configs/specviz/helper.py 26.00% <0.00%> (-8.40%) ⬇️
jdaviz/configs/mosviz/helper.py 51.72% <0.00%> (-7.37%) ⬇️
jdaviz/configs/mosviz/plugins/parsers.py 63.26% <0.00%> (-6.51%) ⬇️
...specviz/plugins/unit_conversion/unit_conversion.py 63.30% <0.00%> (-0.25%) ⬇️
jdaviz/configs/specviz/plugins/__init__.py 100.00% <0.00%> (ø)
jdaviz/configs/specviz/plugins/parsers.py 68.96% <0.00%> (ø)
jdaviz/app.py 82.64% <0.00%> (+0.27%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27297da...444d3bf. Read the comment docs.

@ibusko ibusko requested review from eteq and PatrickOgle September 2, 2020 17:46
@PatrickOgle
Copy link
Contributor

The accessor 'cubeviz.fitted3d' delivers a dictionary of parameter planes fitted over the cube. However, I am having trouble with the initial 1D model fitting on the collapsed data, which yields the following error message, which may be
interfering with the proper fitting of the cube. I have therefore not been able to positively verify that this use case is working from end-to-end.


ValueError Traceback (most recent call last)
~/miniconda3/envs/jdaviz16/lib/python3.8/site-packages/ipyvue/VueTemplateWidget.py in _handle_event(self, , content, buffers)
48 event = content.get("event", "")
49 data = content.get("data", {})
---> 50 getattr(self, 'vue
' + event)(data)
51
52

~/Desktop/jdat/jdaviz/jdaviz/configs/default/plugins/model_fitting/model_fitting.py in vue_model_fitting(self, *args, **kwargs)
267 values
268 """
--> 269 fitted_model, fitted_spectrum = fit_model_to_spectrum(
270 self._spectrum1d,
271 self._initialized_models.values(),

~/Desktop/jdat/jdaviz/jdaviz/configs/default/plugins/model_fitting/fitting_backend.py in fit_model_to_spectrum(spectrum, component_list, expression, run_fitter)
58 return _fit_3D(initial_model, spectrum)
59 else:
---> 60 return _fit_1D(initial_model, spectrum, run_fitter)
61
62

~/Desktop/jdat/jdaviz/jdaviz/configs/default/plugins/model_fitting/fitting_backend.py in _fit_1D(initial_model, spectrum, run_fitter)
84 """
85 if run_fitter:
---> 86 output_model = fit_lines(spectrum, initial_model)
87 output_values = output_model(spectrum.spectral_axis)
88 else:

~/miniconda3/envs/jdaviz16/lib/python3.8/site-packages/specutils/fitting/fitmodels.py in fit_lines(spectrum, model, fitter, exclude_regions, weights, window, **kwargs)
362 ignore_units = getattr(model_guess, model_guess.param_names[0]).unit is None
363
--> 364 fit_model = _fit_lines(spectrum, model_guess, fitter,
365 exclude_regions, weights, model_window,
366 ignore_units, **kwargs)

~/miniconda3/envs/jdaviz16/lib/python3.8/site-packages/specutils/fitting/fitmodels.py in _fit_lines(spectrum, model, fitter, exclude_regions, weights, window, ignore_units, **kwargs)
517
518 if not ignore_units:
--> 519 fit_model = _add_units_to_model(fit_model_unitless, model, spectrum)
520 else:
521 fit_model = QuantityModel(fit_model_unitless,

~/miniconda3/envs/jdaviz16/lib/python3.8/site-packages/specutils/fitting/fitmodels.py in _add_units_to_model(model_in, model_orig, spectrum)
814 equivalencies=u.equivalencies.spectral_density(dispersion))
815 else:
--> 816 raise ValueError(
817 "The parameter '{}' with unit '{}' is not convertible "
818 "to either the current flux unit '{}' or spectral "

ValueError: The parameter 'slope' with unit '1e-17 erg / (Angstrom cm2 m s)' is not convertible to either the current flux unit '1e-17 erg / (Angstrom cm2 s)' or spectral axis unit 'm'.

@PatrickOgle
Copy link
Contributor

Tried again to fit a small subset with a Constant plus Gaussian, but get a different error on the model fitting,
which doesn't deliver a data product:


TypeError Traceback (most recent call last)
~/miniconda3/envs/jdaviz16/lib/python3.8/site-packages/ipyvue/VueTemplateWidget.py in _handle_event(self, , content, buffers)
48 event = content.get("event", "")
49 data = content.get("data", {})
---> 50 getattr(self, 'vue
' + event)(data)
51
52

~/Desktop/jdat/jdaviz/jdaviz/configs/default/plugins/model_fitting/model_fitting.py in vue_model_fitting(self, *args, **kwargs)
323
324 # Update component model parameters with fitted values
--> 325 self._update_parameters_from_fit()
326
327 self.save_enabled = True

~/Desktop/jdat/jdaviz/jdaviz/configs/default/plugins/model_fitting/model_fitting.py in _update_parameters_from_fit(self)
132 name = m["id"]
133 if len(self.component_models) > 1:
--> 134 m_fit = self._fitted_model[name]
135 else:
136 m_fit = self._fitted_model

TypeError: 'QuantityModel' object is not subscriptable

@rosteen
Copy link
Collaborator

rosteen commented Sep 9, 2020

I just fixed the compound model bug in #276 - that was something that was discussed in passing earlier in the year and fell off the radar, glad you mentioned it so I could fix it before release.

Copy link
Contributor

@PatrickOgle PatrickOgle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accessor successfully returns a dictionary of 2D image arrays with associated units, one for each fit parameter. The models are currently misfit, because the model fitting machinery does not reference the collapse method, but this is being fixed in another PR.

Copy link
Collaborator

@rosteen rosteen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, I was able to retrieve the fitted parameter arrays with the fitted3d property.

@rosteen rosteen merged commit 9ad287c into spacetelescope:master Sep 10, 2020
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.

3 participants