Performance improvements

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)
 --------------------
 

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:

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)

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

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):

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

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)

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])

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