diff --git a/CHANGES.rst b/CHANGES.rst index c4f59d2035..985dd7ee86 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -143,6 +143,9 @@ Other Changes and Additions Bug Fixes --------- +- Histogram in Plot Options now uses random sampling to better + represent the data without sacrificing performance. [#2771] + Cubeviz ^^^^^^^ diff --git a/jdaviz/configs/default/plugins/plot_options/plot_options.py b/jdaviz/configs/default/plugins/plot_options/plot_options.py index 6cf4ad8a6a..c79360b3dd 100644 --- a/jdaviz/configs/default/plugins/plot_options/plot_options.py +++ b/jdaviz/configs/default/plugins/plot_options/plot_options.py @@ -4,9 +4,8 @@ import matplotlib import numpy as np -from astropy.visualization import ( - ManualInterval, ContrastBiasStretch, PercentileInterval -) +from astropy.visualization import ManualInterval, ContrastBiasStretch + from echo import delay_callback from traitlets import Any, Dict, Float, Bool, Int, List, Unicode, observe @@ -36,6 +35,8 @@ __all__ = ['PlotOptions'] +RANDOM_SUBSET_SIZE = 10_000 + def _register_random_cmap( cmap_name, @@ -966,13 +967,8 @@ def _update_stretch_histogram(self, msg={}): x_max = x_limits.max() y_min = max(y_limits.min(), 0) y_max = y_limits.max() - arr = comp.data[y_min:y_max, x_min:x_max] - if self.config == "imviz": - # Downsample input data to about 400px (as per compass.vue) for performance. - xstep = max(1, round(arr.shape[1] / 400)) - ystep = max(1, round(arr.shape[0] / 400)) - arr = arr[::ystep, ::xstep] - sub_data = arr.ravel() + + sub_data = comp.data[y_min:y_max, x_min:x_max] else: # spectrum-2d-viewer, for example. We'll assume the viewer @@ -989,28 +985,29 @@ def _update_stretch_histogram(self, msg={}): (y_data >= viewer.state.y_min) & (y_data <= viewer.state.y_max)) - sub_data = comp.data[inds].ravel() + sub_data = comp.data[inds] else: - if self.config == "imviz": - # Downsample input data to about 400px (as per compass.vue) for performance. - xstep = max(1, round(data.shape[1] / 400)) - ystep = max(1, round(data.shape[0] / 400)) - arr = comp[::ystep, ::xstep] - else: - # include all data, regardless of zoom limits - arr = comp.data - sub_data = arr.ravel() - - # filter out nans (or else bqplot will fail) - if np.any(np.isnan(sub_data)): - sub_data = sub_data[~np.isnan(sub_data)] + # include all data, regardless of zoom limits + sub_data = comp.data + self.stretch_histogram.viewer.state.random_subset = RANDOM_SUBSET_SIZE self.stretch_histogram._update_data('histogram', x=sub_data) if len(sub_data) > 0: - interval = PercentileInterval(95) - hist_lims = interval.get_limits(sub_data) + + # Use glue to compute the statistics since this allows us to use + # a random subset of the data to compute the histogram. + # The 2.5 and 97.5 hardcoded here is equivalent to + # PercentileInterval(95).get_limits(sub_data) + glue_data = self.stretch_histogram.app.data_collection['histogram'] + hist_lims = ( + glue_data.compute_statistic('percentile', glue_data.id['x'], + percentile=2.5, random_subset=RANDOM_SUBSET_SIZE), + glue_data.compute_statistic('percentile', glue_data.id['x'], + percentile=97.5, random_subset=RANDOM_SUBSET_SIZE) + ) + # set the stepsize for vmin/vmax to be approximately 1% of the range of the # histogram (within the percentile interval), rounded to 1-2 significant digits # to avoid random step sizes. This logic is somewhat arbitrary and can be safely diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 685879878d..f1cbc5dede 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -4669,13 +4669,13 @@ def _update_data(self, label, reset_lims=False, **kwargs): data = self.app.data_collection[label] # if not provided, fallback on existing data - length_mismatch = False + shape_mismatch = False for component in self._viewer_components: kwargs.setdefault(component, data[component]) - if len(kwargs[component]) != len(data[component]): - length_mismatch = True + if np.asarray(kwargs[component]).shape != data[component].shape: + shape_mismatch = True - if not length_mismatch: + if not shape_mismatch: # then we can update the existing entry components = {c.label: c for c in data.components} data.update_components({components[comp]: kwargs[comp] diff --git a/jdaviz/core/tests/test_tools.py b/jdaviz/core/tests/test_tools.py index 4390c03fed..912176a8ea 100644 --- a/jdaviz/core/tests/test_tools.py +++ b/jdaviz/core/tests/test_tools.py @@ -74,6 +74,11 @@ def test_stretch_bounds(imviz_helper): def test_stretch_bounds_and_spline(imviz_helper): + + # As the histogram randomly samples the array, we should make sure the + # values used here are reproducible + np.random.seed(42) + image_1 = NDData(make_4gaussians_image(), unit=u.nJy) imviz_helper.load_data(image_1) po = imviz_helper.plugins["Plot Options"] @@ -93,7 +98,7 @@ def test_stretch_bounds_and_spline(imviz_helper): knots_after_drag_move = ( [0.0, 0.1, 0.21712585033417825, 0.7, 1.0], - [0.0, 0.05, 0.2900993441358025, 0.9, 1.0], + [0.0, 0.05, 0.2852214046563617, 0.9, 1.0], ) stretch_tool.on_mouse_event(knot_move_msg) diff --git a/pyproject.toml b/pyproject.toml index cf312f2d05..3647ced0e4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,8 +12,8 @@ dependencies = [ "traitlets>=5.0.5", "bqplot>=0.12.37", "bqplot-image-gl>=1.4.11", - "glue-core>=1.18.0", - "glue-jupyter>=0.20", + "glue-core>=1.20.0", + "glue-jupyter>=0.21.0", "echo>=0.5.0", "ipykernel>=6.19.4", "ipyvue>=1.6",