Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: out-of-bounds memory access when trying to zoom on a ParticleProjectionPlot #4603

Closed
neutrinoceros opened this issue Jul 27, 2023 · 3 comments · Fixed by #4604
Closed

Comments

@neutrinoceros
Copy link
Member

Bug report

Bug summary

Zooming may produce out-of-bounds errors. I'm not sure if this is specific to "SPH" datasets or to AREPO (or, none of the above).

Code for reproduction

import yt

ds = yt.load_sample("ArepoBullet")
p = yt.ParticleProjectionPlot(ds, "z",  [('all', 'particle_mass')])
p.zoom(10)
p.save()

Actual outcome

Traceback (most recent call last):
  File "/Users/robcleme/dev/yt-project/yt/ttt.py", line 6, in <module>
    p.save()
  File "/Users/robcleme/dev/yt-project/yt/yt/visualization/_commons.py", line 167, in newfunc
    self._recreate_frb()
  File "/Users/robcleme/dev/yt-project/yt/yt/visualization/plot_window.py", line 333, in _recreate_frb
    self._frb.render(key)
  File "/Users/robcleme/dev/yt-project/yt/yt/visualization/fixed_resolution.py", line 204, in render
    return self[item]
           ~~~~^^^^^^
  File "/Users/robcleme/dev/yt-project/yt/yt/visualization/fixed_resolution.py", line 776, in __getitem__
    add_points_to_greyscale_image(
  File "yt/utilities/lib/image_utilities.pyx", line 28, in yt.utilities.lib.image_utilities.add_points_to_greyscale_image
    buffer[i, j] += pv[pi]
IndexError: Out of bounds on buffer access (axis 0)

Expected outcome
no error

Version Information

  • Operating System: macOS
  • Python Version: 3.11.4
  • yt version: 4.3.dev0
  • Other Libraries (if applicable): numpy 1.25.0, matplotlib 3.7.1

yt installed from source

@matthewturk
Copy link
Member

My guess is this is out-of-bounds-ing on the buffer access, not the pv access.

@matthewturk
Copy link
Member

Probably worth clamping it and also seeing if the difference between the initial and clamped values is > 1. If we're looking at edge effects (which we might be since it's zooming in) then perhaps we just need a better system than calling <int>. It's possible that some particles may be included that are technically centered outside the bounds of the image.

@neutrinoceros
Copy link
Member Author

I think that's exactly what's happening. The following patch resolves the issue

diff --git a/yt/utilities/lib/image_utilities.pyx b/yt/utilities/lib/image_utilities.pyx
index 7dc939ac4..5360ccf4e 100644
--- a/yt/utilities/lib/image_utilities.pyx
+++ b/yt/utilities/lib/image_utilities.pyx
@@ -25,6 +25,11 @@ def add_points_to_greyscale_image(
     for pi in range(npx):
         j = <int> (xs * px[pi])
         i = <int> (ys * py[pi])
+        if (i >= buffer.shape[0]) or (j >= buffer.shape[1]):
+            # some particles might intersect the image buffer
+            # but actually be centered out of bounds. Skip those.
+            continue
+
         buffer[i, j] += pv[pi]
         buffer_mask[i, j] = 1
     return

note that it's not expected to have much of a performance cost since the branching should be very well predicted (even in the reproduction script I've given, only one particle falls into the if clause)

@neutrinoceros neutrinoceros changed the title BUG: out-of-bounds memory access when trying to zoom on a ParticleProjectionPlot (AREPO) BUG: out-of-bounds memory access when trying to zoom on a ParticleProjectionPlot Jul 27, 2023
@neutrinoceros neutrinoceros added this to the 4.2.2 milestone Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants