Use array size directly when checking multiscale arrays to prevent overflow

[kephale]

Fixes/Closes

Closes #5758

Description

This PR changes the way array sizes are compared when guessing if data is multiscale.

The problem was that np.prod(a.shape) was being used which leads to an overflow (see Notes in numpy.ndarray.size). That has been changed to be a.size instead (ty @aganders3).

Additionally, during testing there was an issue with what seemed to be an out of memory error with the Python 3.8 Windows min req job (ty @jaimergp). As a result the test now also uses a Dask delayed array to avoid the large memory allocation (ty @aganders3).

Type of change

• Bug-fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How has this been tested?

• example: the test suite for my feature covers cases x, y, and z
• example: all tests pass with my change
• example: I check if my changes works with both PySide and PyQt backends
  as there are small differences between the two Qt bindings.

Final checklist:

• My PR is the minimum possible work for the desired functionality
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• If I included new strings, I have used trans. to make them localizable.
  For more information see our translations guide.

[codecov]

Codecov Report

Merging #5759 (88ecf5e) into main (ac82660) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #5759      +/-   ##
==========================================
+ Coverage   89.83%   89.86%   +0.02%     
==========================================
  Files         609      610       +1     
  Lines       52046    52134      +88     
==========================================
+ Hits        46755    46848      +93     
+ Misses       5291     5286       -5     

Impacted Files	Coverage Δ
napari/layers/image/_image_utils.py	93.10% <100.00%> (-0.23%)	⬇️
napari/layers/image/_tests/test_image_utils.py	98.50% <100.00%> (+0.09%)	⬆️

... and 34 files with indirect coverage changes

[alisterburt]

Nice! Looks good to me 🚀

[alisterburt]

rerunning a failed test as it seemed spurious

[kephale]

I've been having issues using act to recreate this. The test passes locally on my M1 mac with python 3.10 and 3.8, and attempting to locally recreate this is taking a lot of time. As the CI tests indicate the only issue seems to be from PR Test / ubuntu-20.04 3.8 pyqt5 min_req (pull_request).

I'm not suggesting merging without that passing, but this is turning into a time sink for me. Since this is a pretty minor change that tests an overflow and it is breaking due to testing on min_req libraries, it doesn't feel that valuable to keep investigating this. Also note that the "failure" is more of a timeout/termination instead of a failing assertion. I'm open to being convinced otherwise, but I'm moving on for now.

[Czaki]

I will try to reproduce this on Linux machine these evening.

[aganders3]

@jaimergp suggested in the meeting this might be OOM and the exit codes make me suspicious of that as well. It shouldn't be that much memory really, but you could try this to allocate way less. Maybe one of the older deps is causing this to allocate more? Even on my machine with up-to-date dependencies this change speeds up testing this file about 7x:

diff --git a/napari/layers/image/_tests/test_image_utils.py b/napari/layers/image/_tests/test_image_utils.py
index bc0305376..127e028da 100644
--- a/napari/layers/image/_tests/test_image_utils.py
+++ b/napari/layers/image/_tests/test_image_utils.py
@@ -1,5 +1,6 @@
 import time
 
+import dask
 import dask.array as da
 import numpy as np
 import pytest
@@ -76,7 +77,10 @@ def test_guess_multiscale():
 
     # Test for overflow in calculating array sizes
     s = 17179869184
-    data = [da.ones((s,) * 2), da.ones((s // 2,) * 2)]
+    data = [
+        da.from_delayed(dask.delayed(lambda: None), shape=(s,) * 2, dtype=np.float64),
+        da.from_delayed(dask.delayed(lambda: None), shape=(s // 2,) * 2, dtype=np.float64),
+    ]
     assert guess_multiscale(data)[0]

Edit: apologies if this is too much nitpick/code-golf but I find this a bit clearer. d.size should be equivalent to np.prod(d.shape, dtype='object') (you could also do half this and keep sizes as a numpy array).

diff --git a/napari/layers/image/_image_utils.py b/napari/layers/image/_image_utils.py
index 5e85a0d84..2a49559c7 100644
--- a/napari/layers/image/_image_utils.py
+++ b/napari/layers/image/_image_utils.py
@@ -65,12 +65,12 @@ def guess_multiscale(data) -> Tuple[bool, LayerDataProtocol]:
         return False, data[0]
 
     shapes = [d.shape for d in data]
-    sizes = np.array([np.prod(shape, dtype='object') for shape in shapes])
+    sizes = [d.size for d in data]
     if len(sizes) <= 1:
         return False, data
 
-    consistent = bool(np.all(sizes[:-1] > sizes[1:]))
-    if np.all(sizes == sizes[0]):
+    consistent = all(s1 > s2 for s1, s2 in zip(sizes[:-1], sizes[1:]))
+    if all(s == sizes[0] for s in sizes):
         # note: the individual array case should be caught by the first
         # code line in this function, hasattr(ndim) and ndim > 1.
         raise ValueError(

[kephale]

@aganders3, my hero!

One thing these fixes highlights is that guess_multisize checks for order of sizes, but not size per dimension. It is plausible that there could be a list of arrays where the total array size decreases, but one of the dimensions increases in size. That edge case could be fixed now, but that would require another (easy) change.

[kephale]

Does this need any other changes?

[Czaki]

After fast reading the current code, I think that the title and description should be updated to reflect changes in this PR.

[kephale]

Good point, @Czaki! Both have been updated.