Skip to content

Commit

Permalink
improve stretch histogram performance (downsampling and decorator) (#…
Browse files Browse the repository at this point in the history
…2408)

* downsample stretch histogram with UI warning
* simplify stretch_curve_visible switch to use traitlet directly since it does not interact with any glue state objects
* use decorator to avoid updating curve when plugin closed
  • Loading branch information
kecnry authored Aug 31, 2023
1 parent 7896cdd commit 60ddb58
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 18 deletions.
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ Imviz
- Add a curve to stretch histograms in the Plot Options plugin representing the colormap
stretch function. [#2390]

- The stretch histogram is now downsampled for large images for improved performance. [#2408]

Mosviz
^^^^^^

Expand Down
54 changes: 38 additions & 16 deletions jdaviz/configs/default/plugins/plot_options/plot_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class PlotOptions(PluginTemplateMixin):
limits instead of all data within the layer; not exposed for Specviz.
* ``stretch_hist_nbins`` : number of bins to use in creating the histogram; not exposed
for Specviz.
* ``stretch_curve_visible`` : (:class:`~jdaviz.core.template_mixin.PlotOptionsSyncState`):
* ``stretch_curve_visible`` : bool
whether the stretch histogram's colormap "curve" is visible; not exposed for Specviz.
* ``image_visible`` (:class:`~jdaviz.core.template_mixin.PlotOptionsSyncState`):
whether the image bitmap is visible; not exposed for Specviz.
Expand Down Expand Up @@ -207,8 +207,7 @@ class PlotOptions(PluginTemplateMixin):
stretch_hist_nbins = IntHandleEmpty(25).tag(sync=True)
stretch_histogram_widget = Unicode().tag(sync=True)

stretch_curve_visible_value = Bool().tag(sync=True)
stretch_curve_visible_sync = Dict().tag(sync=True)
stretch_curve_visible = Bool().tag(sync=True)

subset_visible_value = Bool().tag(sync=True)
subset_visible_sync = Dict().tag(sync=True)
Expand Down Expand Up @@ -394,12 +393,6 @@ def state_attr_for_line_visible(state):
'stretch_vmax_value', 'stretch_vmax_sync',
state_filter=is_image)

self.stretch_curve_visible = PlotOptionsSyncState(self, self.viewer, self.layer,
'stretch_curve_visible',
'stretch_curve_visible_value',
'stretch_curve_visible_sync',
state_filter=is_image)

self.stretch_histogram = Plot(self)
self.stretch_histogram.add_bins('histogram', color='gray')
self.stretch_histogram.add_line('vmin', x=[0, 0], y=[0, 1], ynorm=True, color='#c75d2c')
Expand Down Expand Up @@ -600,7 +593,18 @@ def _update_stretch_histogram(self, msg={}):
y_min = max(y_limits.min(), 0)
y_max = y_limits.max()

sub_data = comp.data[y_min:y_max, x_min:x_max].ravel()
arr = comp.data[y_min:y_max, x_min:x_max]

size = arr.shape[0] * arr.shape[1]
if size > 400**2:
xstep = max(1, round(arr.shape[1] / 400))
ystep = max(1, round(arr.shape[0] / 400))
arr = arr[::ystep, ::xstep]
stretch_hist_downsampled = [size, arr.shape[0] * arr.shape[1]]
else:
stretch_hist_downsampled = size

sub_data = arr.ravel()

else:
# spectrum-2d-viewer, for example. We'll assume the viewer
Expand All @@ -615,9 +619,23 @@ def _update_stretch_histogram(self, msg={}):
(y_data <= viewer.state.y_max))

sub_data = comp.data[inds].ravel()

# downsampling not currently implemented for 2d spectrum
stretch_hist_downsampled = len(sub_data)

else:
# include all data, regardless of zoom limits
sub_data = comp.data.ravel()
arr = comp.data
size = arr.shape[0] * arr.shape[1]
if size > 400**2:
xstep = max(1, round(arr.shape[1] / 400))
ystep = max(1, round(arr.shape[0] / 400))
arr = arr[::ystep, ::xstep]
stretch_hist_downsampled = [size, arr.shape[0] * arr.shape[1]]
else:
stretch_hist_downsampled = size

sub_data = arr.ravel()

# filter out nans (or else bqplot will fail)
if np.any(np.isnan(sub_data)):
Expand All @@ -640,12 +658,16 @@ def _update_stretch_histogram(self, msg={}):
# in case only the sample has changed but its length has not,
# we'll force the traitlet to trigger a change
hist_mark.send_state('sample')
if isinstance(stretch_hist_downsampled, list):
title = f"{stretch_hist_downsampled[1]} of {stretch_hist_downsampled[0]} pixels"
else:
title = f"{stretch_hist_downsampled} pixels"
self.stretch_histogram.figure.title = title

self._stretch_histogram_needs_update = False

@observe('stretch_vmin_value', 'stretch_vmax_value', 'layer_selected',
@observe('is_active', 'stretch_vmin_value', 'stretch_vmax_value', 'layer_selected',
'stretch_hist_nbins', 'image_contrast_value', 'image_bias_value',
'stretch_curve_visible_value')
'stretch_curve_visible')
@skip_if_no_updates_since_last_active()
def _update_stretch_curve(self, msg=None):
mark_label_prefix = "stretch_curve: "

Expand All @@ -654,7 +676,7 @@ def _update_stretch_curve(self, msg=None):
# or the stretch histogram hasn't been initialized:
return

if not self.stretch_curve_visible_value:
if not self.stretch_curve_visible:
# clear marks if curve is not visible:
for existing_mark_label, mark in self.stretch_histogram.marks.items():
if existing_mark_label.startswith(mark_label_prefix):
Expand Down
4 changes: 2 additions & 2 deletions jdaviz/configs/default/plugins/plot_options/plot_options.vue
Original file line number Diff line number Diff line change
Expand Up @@ -386,15 +386,15 @@
</v-row>
<v-row>
<v-switch
v-model="stretch_curve_visible_value"
v-model="stretch_curve_visible"
class="hide-input"
label="Show stretch function curve"
style="z-index: 1"
></v-switch>
<!-- NOTE: height defined here should match that in the custom CSS rules
below for the bqplot class -->
</v-row>
<jupyter-widget :widget="stretch_histogram_widget"/>
<jupyter-widget :widget="stretch_histogram_widget"/>
</div>

<!-- IMAGE:IMAGE -->
Expand Down

0 comments on commit 60ddb58

Please sign in to comment.