From fefd283c9b42afc03f578fbfdea6ae361a66d02a Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Wed, 11 Sep 2024 14:21:41 -0400 Subject: [PATCH 01/12] Fix NaN handling in cube fitting Remove debugging prints, add comment for context Codestyle, changelog Remove unit conversion stuff to just fix NaN handling --- CHANGES.rst | 1 + .../configs/default/plugins/model_fitting/initializers.py | 4 ++-- .../default/plugins/model_fitting/model_fitting.py | 8 +++++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index a0f2e924b5..95e79d2394 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -142,6 +142,7 @@ Cubeviz - No longer incorrectly swap RA and Dec axes when loading Spectrum1D objects. [#3133] +- Fixed fitting a model to the entire cube when NaNs are present. [#3190] Imviz ^^^^^ diff --git a/jdaviz/configs/default/plugins/model_fitting/initializers.py b/jdaviz/configs/default/plugins/model_fitting/initializers.py index c7aa327e71..1548873927 100644 --- a/jdaviz/configs/default/plugins/model_fitting/initializers.py +++ b/jdaviz/configs/default/plugins/model_fitting/initializers.py @@ -119,7 +119,7 @@ def initialize(self, instance, x, y): instance: `~astropy.modeling.Model` The initialized model. """ - y_mean = np.mean(y) + y_mean = np.nanmean(y) x_range = x[-1] - x[0] position = x_range / 2.0 + x[0] @@ -190,7 +190,7 @@ def initialize(self, instance, x, y): # width can be estimated by the weighted # 2nd moment of the X coordinate. - dx = x - np.mean(x) + dx = x - np.nanmean(x) fwhm = 2 * np.sqrt(np.sum((dx * dx) * y) / np.sum(y)) # amplitude is derived from area. diff --git a/jdaviz/configs/default/plugins/model_fitting/model_fitting.py b/jdaviz/configs/default/plugins/model_fitting/model_fitting.py index 64d2f68571..d5f2aa8155 100644 --- a/jdaviz/configs/default/plugins/model_fitting/model_fitting.py +++ b/jdaviz/configs/default/plugins/model_fitting/model_fitting.py @@ -493,7 +493,7 @@ def _initialize_model_component(self, model_comp, comp_label, poly_order=None): masked_spectrum.flux[~mask] if mask is not None else masked_spectrum.flux) # need to loop over parameters again as the initializer may have overridden - # the original default value + # the original default value. for param_name in get_model_parameters(model_cls, new_model["model_kwargs"]): param_quant = getattr(initialized_model, param_name) new_model["parameters"].append({"name": param_name, @@ -917,6 +917,12 @@ def _fit_model_to_cube(self, add_data): # Apply masks from selected spectral subset spec = self._apply_subset_masks(spec, self.spectral_subset) + # Also mask out NaNs for fitting. Simply adding filter_non_finite to the cube fit + # didn't work out of the box, so doing this for now. + if spec.mask is None: + spec.mask = np.isnan(spec.flux) + else: + spec.mask = spec.mask | np.isnan(spec.flux) try: fitted_model, fitted_spectrum = fit_model_to_spectrum( From db5ed01fe73d56261701a7fbf7a02e862141b622 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 12 Sep 2024 10:06:56 -0400 Subject: [PATCH 02/12] Move changelog --- CHANGES.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 95e79d2394..0e0da183b5 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -142,8 +142,6 @@ Cubeviz - No longer incorrectly swap RA and Dec axes when loading Spectrum1D objects. [#3133] -- Fixed fitting a model to the entire cube when NaNs are present. [#3190] - Imviz ^^^^^ @@ -240,6 +238,8 @@ Bug Fixes Cubeviz ^^^^^^^ +- Fixed fitting a model to the entire cube when NaNs are present. [#3191] + Imviz ^^^^^ From 7cec8154febb8fda2ed8531f0d2b31815d6b5a4c Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 12 Sep 2024 11:29:06 -0400 Subject: [PATCH 03/12] Add test --- .../plugins/model_fitting/tests/test_fitting.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py index cbddd12c59..37a2cf2432 100644 --- a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py +++ b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py @@ -378,3 +378,18 @@ def test_incompatible_units(specviz_helper, spectrum1d): with warnings.catch_warnings(): warnings.filterwarnings('ignore', message='Model is linear in parameters.*') mf.calculate_fit(add_data=True) + + +def test_cube_fit_with_nans(cubeviz_helper): + flux = np.ones((7, 8, 9)) * u.nJy + flux[:, :, 0] = np.nan + spec = Spectrum1D(flux=flux) + cubeviz_helper.load_data(spec, data_label="test") + + mf = cubeviz_helper.plugins["Model Fitting"] + mf.cube_fit = True + #mf.dataset = "test[FLUX]" + mf.create_model_component("Const1D") + mf.calculate_fit() + result = cubeviz_helper.app.data_collection['model'] + assert np.all(result.get_component("flux").data == 1) From 65115300fba561bdbebddcfebd34299f7cf95083 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 12 Sep 2024 11:32:17 -0400 Subject: [PATCH 04/12] Codestyle --- .../configs/default/plugins/model_fitting/tests/test_fitting.py | 1 - 1 file changed, 1 deletion(-) diff --git a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py index 37a2cf2432..be55cecd41 100644 --- a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py +++ b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py @@ -388,7 +388,6 @@ def test_cube_fit_with_nans(cubeviz_helper): mf = cubeviz_helper.plugins["Model Fitting"] mf.cube_fit = True - #mf.dataset = "test[FLUX]" mf.create_model_component("Const1D") mf.calculate_fit() result = cubeviz_helper.app.data_collection['model'] From 4252e8a92c09850706b9f70f4d275c87f3941e3e Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 12 Sep 2024 13:20:08 -0400 Subject: [PATCH 05/12] Adding test for spectrum with existing mask --- .../model_fitting/tests/test_fitting.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py index be55cecd41..85d9ae7114 100644 --- a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py +++ b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py @@ -392,3 +392,21 @@ def test_cube_fit_with_nans(cubeviz_helper): mf.calculate_fit() result = cubeviz_helper.app.data_collection['model'] assert np.all(result.get_component("flux").data == 1) + +def test_cube_fit_with_mask_and_nans(cubeviz_helper): + # Also test with existing mask + flux = np.ones((7, 8, 9)) * u.nJy + flux[:, :, 0] = np.nan + spec = Spectrum1D(flux=flux) + spec.mask = np.zeros((7, 8, 9)).astype(bool) + spec.mask[5,5,5] = True + spec.flux[5,5,5] = 10*u.nJy + cubeviz_helper.load_data(spec, data_label="test") + + mf = cubeviz_helper.plugins["Model Fitting"] + mf.cube_fit = True + mf.create_model_component("Const1D") + mf.calculate_fit() + result = cubeviz_helper.app.data_collection['model'] + print(cubeviz_helper.app.data_collection['model'].get_object(statistic=None).flux.max()) + assert np.all(result.get_component("flux").data == 1) \ No newline at end of file From 163e3ddbcf7f2864bf83ffd85c6f8d041c3230da Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 12 Sep 2024 14:14:37 -0400 Subject: [PATCH 06/12] Fix a couple bugs with a loading a masked cube --- .../cubeviz/plugins/spectral_extraction/spectral_extraction.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py index b82acec4f7..b649e9f06a 100644 --- a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py +++ b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py @@ -471,6 +471,9 @@ def _extract_from_aperture(self, cube, uncert_cube, aperture, wcs = cube.meta['_orig_spec'].wcs.spectral elif hasattr(cube.coords, 'spectral'): wcs = cube.coords.spectral + elif hasattr(cube.coords, 'spectral_wcs'): + # This is the attribute for a PaddedSpectrumWCS in the 3D case + wcs = cube.coords.spectral_wcs else: wcs = None From 95f9fdbb1108e73d43b9585c3c94122076777074 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 12 Sep 2024 14:16:51 -0400 Subject: [PATCH 07/12] Linking fix for case with extra components like mask --- jdaviz/app.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index 160eb9fae0..6df7f76217 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -735,8 +735,9 @@ def _link_new_data(self, reference_data=None, data_to_be_linked=None): return elif self.config == 'cubeviz' and linked_data.ndim == 1: - ref_wavelength_component = dc[0].components[-2] - ref_flux_component = dc[0].components[-1] + # Don't want to use negative indices in case there are extra components like a mask + ref_wavelength_component = dc[0].components[5] + ref_flux_component = dc[0].components[6] linked_wavelength_component = dc[-1].components[1] linked_flux_component = dc[-1].components[-1] From 3832873325fb7da9be010b5167e1e4ceca47692e Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 12 Sep 2024 14:51:12 -0400 Subject: [PATCH 08/12] Test mask addition behavior with a subset instead --- .../plugins/model_fitting/tests/test_fitting.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py index 85d9ae7114..b26f55ea22 100644 --- a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py +++ b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py @@ -8,6 +8,7 @@ from astropy.nddata import StdDevUncertainty from astropy.tests.helper import assert_quantity_allclose from astropy.wcs import WCS +from glue.core.roi import XRangeROI from numpy.testing import assert_allclose, assert_array_equal from specutils.spectra import Spectrum1D @@ -393,18 +394,20 @@ def test_cube_fit_with_nans(cubeviz_helper): result = cubeviz_helper.app.data_collection['model'] assert np.all(result.get_component("flux").data == 1) -def test_cube_fit_with_mask_and_nans(cubeviz_helper): +def test_cube_fit_with_subset_and_nans(cubeviz_helper): # Also test with existing mask flux = np.ones((7, 8, 9)) * u.nJy flux[:, :, 0] = np.nan spec = Spectrum1D(flux=flux) - spec.mask = np.zeros((7, 8, 9)).astype(bool) - spec.mask[5,5,5] = True - spec.flux[5,5,5] = 10*u.nJy + spec.flux[5,5,7] = 10*u.nJy cubeviz_helper.load_data(spec, data_label="test") + sv = cubeviz_helper.app.get_viewer('spectrum-viewer') + sv.apply_roi(XRangeROI(0, 5)) + mf = cubeviz_helper.plugins["Model Fitting"] mf.cube_fit = True + mf.spectral_subset = 'Subset 1' mf.create_model_component("Const1D") mf.calculate_fit() result = cubeviz_helper.app.data_collection['model'] From 766df6ac02975ce305b109eb8c945d50fcac373f Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 12 Sep 2024 14:52:16 -0400 Subject: [PATCH 09/12] Codestyle --- .../default/plugins/model_fitting/tests/test_fitting.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py index b26f55ea22..02d06f90c3 100644 --- a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py +++ b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py @@ -394,12 +394,13 @@ def test_cube_fit_with_nans(cubeviz_helper): result = cubeviz_helper.app.data_collection['model'] assert np.all(result.get_component("flux").data == 1) + def test_cube_fit_with_subset_and_nans(cubeviz_helper): # Also test with existing mask flux = np.ones((7, 8, 9)) * u.nJy flux[:, :, 0] = np.nan spec = Spectrum1D(flux=flux) - spec.flux[5,5,7] = 10*u.nJy + spec.flux[5, 5, 7] = 10 * u.nJy cubeviz_helper.load_data(spec, data_label="test") sv = cubeviz_helper.app.get_viewer('spectrum-viewer') @@ -412,4 +413,4 @@ def test_cube_fit_with_subset_and_nans(cubeviz_helper): mf.calculate_fit() result = cubeviz_helper.app.data_collection['model'] print(cubeviz_helper.app.data_collection['model'].get_object(statistic=None).flux.max()) - assert np.all(result.get_component("flux").data == 1) \ No newline at end of file + assert np.all(result.get_component("flux").data == 1) From ebd3572943cf017152c18f5a439299c53e65eb54 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 12 Sep 2024 16:48:45 -0400 Subject: [PATCH 10/12] Ignore UserWarning for Ubuntu tests --- .../default/plugins/model_fitting/tests/test_fitting.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py index 02d06f90c3..4878111503 100644 --- a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py +++ b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py @@ -7,6 +7,7 @@ from astropy.modeling import models from astropy.nddata import StdDevUncertainty from astropy.tests.helper import assert_quantity_allclose +from astropy.utils.exceptions import AstropyUserWarning from astropy.wcs import WCS from glue.core.roi import XRangeROI from numpy.testing import assert_allclose, assert_array_equal @@ -390,7 +391,8 @@ def test_cube_fit_with_nans(cubeviz_helper): mf = cubeviz_helper.plugins["Model Fitting"] mf.cube_fit = True mf.create_model_component("Const1D") - mf.calculate_fit() + with pytest.warns(AstropyUserWarning, match='Model is linear in parameters'): + mf.calculate_fit() result = cubeviz_helper.app.data_collection['model'] assert np.all(result.get_component("flux").data == 1) @@ -410,7 +412,8 @@ def test_cube_fit_with_subset_and_nans(cubeviz_helper): mf.cube_fit = True mf.spectral_subset = 'Subset 1' mf.create_model_component("Const1D") - mf.calculate_fit() + with pytest.warns(AstropyUserWarning, match='Model is linear in parameters'): + mf.calculate_fit() result = cubeviz_helper.app.data_collection['model'] print(cubeviz_helper.app.data_collection['model'].get_object(statistic=None).flux.max()) assert np.all(result.get_component("flux").data == 1) From 862dd871bbb9124544e594b644f1f5b14e294dbf Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <39831871+rosteen@users.noreply.github.com> Date: Thu, 12 Sep 2024 16:50:20 -0400 Subject: [PATCH 11/12] Update jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> --- .../configs/default/plugins/model_fitting/tests/test_fitting.py | 1 - 1 file changed, 1 deletion(-) diff --git a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py index 4878111503..c21e60309c 100644 --- a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py +++ b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py @@ -415,5 +415,4 @@ def test_cube_fit_with_subset_and_nans(cubeviz_helper): with pytest.warns(AstropyUserWarning, match='Model is linear in parameters'): mf.calculate_fit() result = cubeviz_helper.app.data_collection['model'] - print(cubeviz_helper.app.data_collection['model'].get_object(statistic=None).flux.max()) assert np.all(result.get_component("flux").data == 1) From f1e78983ec521d0eb3ceee7a28a5da365699dab1 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 12 Sep 2024 17:08:15 -0400 Subject: [PATCH 12/12] Do this as in above test --- .../default/plugins/model_fitting/tests/test_fitting.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py index c21e60309c..9044ad5a4c 100644 --- a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py +++ b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py @@ -7,7 +7,6 @@ from astropy.modeling import models from astropy.nddata import StdDevUncertainty from astropy.tests.helper import assert_quantity_allclose -from astropy.utils.exceptions import AstropyUserWarning from astropy.wcs import WCS from glue.core.roi import XRangeROI from numpy.testing import assert_allclose, assert_array_equal @@ -391,7 +390,8 @@ def test_cube_fit_with_nans(cubeviz_helper): mf = cubeviz_helper.plugins["Model Fitting"] mf.cube_fit = True mf.create_model_component("Const1D") - with pytest.warns(AstropyUserWarning, match='Model is linear in parameters'): + with warnings.catch_warnings(): + warnings.filterwarnings('ignore', message='Model is linear in parameters.*') mf.calculate_fit() result = cubeviz_helper.app.data_collection['model'] assert np.all(result.get_component("flux").data == 1) @@ -412,7 +412,8 @@ def test_cube_fit_with_subset_and_nans(cubeviz_helper): mf.cube_fit = True mf.spectral_subset = 'Subset 1' mf.create_model_component("Const1D") - with pytest.warns(AstropyUserWarning, match='Model is linear in parameters'): + with warnings.catch_warnings(): + warnings.filterwarnings('ignore', message='Model is linear in parameters.*') mf.calculate_fit() result = cubeviz_helper.app.data_collection['model'] assert np.all(result.get_component("flux").data == 1)