diff --git a/CHANGES.md b/CHANGES.md index 88e5a635b..c0a77fe6d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -26,6 +26,11 @@ v0.16.0 (unreleased) * Preserve ``RectangularROI`` rather than converting them to ``PolygonalROI``. [#2112] +* Significantly improve performance of ``compute_fixed_resolution_buffer`` when + using linked datasets with some axes uncorrelated. [#2115] + +* Improve performance of profile viewer when not all layers are visible. [#2115] + v0.15.7 (2020-03-12) -------------------- diff --git a/glue/core/data.py b/glue/core/data.py index 4615a3b89..6647c74e7 100644 --- a/glue/core/data.py +++ b/glue/core/data.py @@ -1606,31 +1606,43 @@ def compute_statistic(self, statistic, cid, subset_state=None, axis=None, # We operate in chunks here to avoid memory issues. - # TODO: there are cases where the code below is not optimized - # because the mask may be computable for a single slice and - # broadcastable to all slices - normally ROISubsetState takes care - # of that but if we call it once per view it won't. In the future we - # could ask a SubsetState whether it is broadcasted along - # axis_index. - axis_index = [a for a in range(self.ndim) if a not in axis][0] - result = np.zeros(self.shape[axis_index]) + # In the specific case where the subset state depends only on pixel + # component IDs but not the one for the chunk iteration axis used + # here, we should not need to chunk. + efficient_subset_state = False + if subset_state is not None: + from glue.core.link_manager import pixel_cid_to_pixel_cid_matrix + for att in subset_state.attributes: + # TODO: in principle we cold still deal with non-pixel + # componnet IDs, so this should be fixed. + if not isinstance(att, PixelComponentID): + break + matrix = pixel_cid_to_pixel_cid_matrix(att.parent, self) + if matrix[att.axis, axis_index]: + break + else: + efficient_subset_state = True + + if not efficient_subset_state: + + result = np.zeros(self.shape[axis_index]) - chunk_shape = list(self.shape) + chunk_shape = list(self.shape) - # Deliberately leave n_chunks as float to not round twice - n_chunks = self.size / n_chunk_max + # Deliberately leave n_chunks as float to not round twice + n_chunks = self.size / n_chunk_max - chunk_shape[axis_index] = max(1, int(chunk_shape[axis_index] / n_chunks)) + chunk_shape[axis_index] = max(1, int(chunk_shape[axis_index] / n_chunks)) - for chunk_view in iterate_chunks(self.shape, chunk_shape=chunk_shape): - values = self.compute_statistic(statistic, cid, subset_state=subset_state, - axis=axis, finite=finite, positive=positive, - percentile=percentile, view=chunk_view) - result[chunk_view[axis_index]] = values + for chunk_view in iterate_chunks(self.shape, chunk_shape=chunk_shape): + values = self.compute_statistic(statistic, cid, subset_state=subset_state, + axis=axis, finite=finite, positive=positive, + percentile=percentile, view=chunk_view) + result[chunk_view[axis_index]] = values - return result + return result if subset_state: if isinstance(subset_state, SliceSubsetState) and view is None: diff --git a/glue/core/fixed_resolution_buffer.py b/glue/core/fixed_resolution_buffer.py index 8cf5ef5ab..656a58fb0 100644 --- a/glue/core/fixed_resolution_buffer.py +++ b/glue/core/fixed_resolution_buffer.py @@ -2,7 +2,7 @@ from glue.core.exceptions import IncompatibleDataException from glue.core.component import CoordinateComponent from glue.core.coordinate_helpers import dependent_axes -from glue.utils import unbroadcast, broadcast_to +from glue.utils import unbroadcast, broadcast_to, broadcast_arrays_minimal # TODO: cache needs to be updated when links are removed/changed @@ -57,7 +57,15 @@ def translate_pixel(data, pixel_coords, target_cid): values, dimensions = translate_pixel(data, pixel_coords, cid) values_all.append(values) dimensions_all.extend(dimensions) - return link._using(*values_all), dimensions_all + # Unbroadcast arrays to smallest common shape for performance + if len(values_all) > 0: + shape = values_all[0].shape + values_all = broadcast_arrays_minimal(*values_all) + results = link._using(*values_all) + result = broadcast_to(results, shape) + else: + result = None + return result, sorted(set(dimensions_all)) elif isinstance(component, CoordinateComponent): # FIXME: Hack for now - if we pass arrays in the view, it's interpreted return component._calculate(view=pixel_coords), dependent_axes(data.coords, component.axis) diff --git a/glue/utils/array.py b/glue/utils/array.py index ebd0bab8c..edc9a7bf1 100644 --- a/glue/utils/array.py +++ b/glue/utils/array.py @@ -10,7 +10,8 @@ 'coerce_numeric', 'check_sorted', 'broadcast_to', 'unbroadcast', 'iterate_chunks', 'combine_slices', 'nanmean', 'nanmedian', 'nansum', 'nanmin', 'nanmax', 'format_minimal', 'compute_statistic', - 'categorical_ndarray', 'index_lookup', 'ensure_numerical'] + 'categorical_ndarray', 'index_lookup', 'ensure_numerical', + 'broadcast_arrays_minimal'] def unbroadcast(array): @@ -29,6 +30,13 @@ def unbroadcast(array): return as_strided(array, shape=new_shape) +def broadcast_arrays_minimal(*arrays): + """ + Unbroadcast arrays then broadcast to smallest common shape. + """ + return np.broadcast_arrays(*[unbroadcast(array) for array in arrays]) + + def unique(array): """ Return the unique elements of the array U, as well as diff --git a/glue/utils/tests/test_array.py b/glue/utils/tests/test_array.py index 011a4a77f..b6765d756 100644 --- a/glue/utils/tests/test_array.py +++ b/glue/utils/tests/test_array.py @@ -10,7 +10,7 @@ shape_to_string, check_sorted, pretty_number, unbroadcast, iterate_chunks, combine_slices, nanmean, nanmedian, nansum, nanmin, nanmax, format_minimal, compute_statistic, categorical_ndarray, - index_lookup) + index_lookup, broadcast_arrays_minimal) @pytest.mark.parametrize(('before', 'ref_after', 'ref_indices'), @@ -128,6 +128,19 @@ def test_unbroadcast(): np.testing.assert_allclose(z[0, 0], x) +def test_broadcast_arrays_minimal(): + + a = np.array([1, 2, 3]) + b = broadcast_to(a, (2, 4, 3)) + + c = np.ones((2, 1, 1)) + d = broadcast_to(c, (2, 4, 3)) + + e, f = broadcast_arrays_minimal(b, d) + assert e.shape == (2, 1, 3) + assert f.shape == (2, 1, 3) + + @pytest.mark.parametrize(('chunk_shape', 'n_max'), [(None, 121), (None, 100000), (None, 5), ((3, 1, 2, 4, 1, 2), None)]) def test_iterate_chunks(chunk_shape, n_max): diff --git a/glue/viewers/profile/layer_artist.py b/glue/viewers/profile/layer_artist.py index 2039f30af..28d722ce5 100644 --- a/glue/viewers/profile/layer_artist.py +++ b/glue/viewers/profile/layer_artist.py @@ -63,11 +63,14 @@ def _calculate_profile_postthread(self): except Exception: return + self.enable() + + # The following can happen if self.state.visible is None - in this case + # we just terminate early. If the visible property is changed, it will + # trigger the _calculate_profile code to re-run. if visible_data is None: return - self.enable() - x, y = visible_data # Update the data values. @@ -150,10 +153,10 @@ def _update_profile(self, force=False, **kwargs): changed = set() if force else self.pop_changed_properties() - if force or any(prop in changed for prop in ('layer', 'x_att', 'attribute', 'function', 'normalize', 'v_min', 'v_max')): + if force or any(prop in changed for prop in ('layer', 'x_att', 'attribute', 'function', 'normalize', 'v_min', 'v_max', 'visible')): self._calculate_profile(reset=force) - if force or any(prop in changed for prop in ('alpha', 'color', 'zorder', 'visible', 'linewidth')): + if force or any(prop in changed for prop in ('alpha', 'color', 'zorder', 'linewidth')): self._update_visual_attributes() @defer_draw diff --git a/glue/viewers/profile/state.py b/glue/viewers/profile/state.py index c681e8a5e..7b4332f51 100644 --- a/glue/viewers/profile/state.py +++ b/glue/viewers/profile/state.py @@ -244,6 +244,9 @@ def update_profile(self, update_limits=True): if self._profile_cache is not None: return self._profile_cache + if not self.visible: + return + if not self._viewer_callbacks_set: self.viewer_state.add_callback('x_att', self.reset_cache, priority=100000) self.viewer_state.add_callback('function', self.reset_cache, priority=100000) diff --git a/glue/viewers/profile/tests/test_state.py b/glue/viewers/profile/tests/test_state.py index 32f0e17c6..7b45fc26c 100644 --- a/glue/viewers/profile/tests/test_state.py +++ b/glue/viewers/profile/tests/test_state.py @@ -147,3 +147,15 @@ def test_limits(self): assert self.viewer_state.x_min == -0.5 assert self.viewer_state.x_max == 2.5 + + def test_visible(self): + + self.layer_state.visible = False + + assert self.layer_state.profile is None + + self.layer_state.visible = True + + x, y = self.layer_state.profile + assert_allclose(x, [0, 2, 4]) + assert_allclose(y, [3.5, 11.5, 19.5]) diff --git a/setup.cfg b/setup.cfg index d82de7aa3..068518db4 100644 --- a/setup.cfg +++ b/setup.cfg @@ -73,6 +73,9 @@ all = PyAVM astrodendro spectral-cube + # See https://github.com/python-pillow/Pillow/issues/4509 + # for why we exclude pillow 7.1.0 + pillow!=7.1.0 docs = sphinx sphinx-automodapi