Make tests green - #438
Draft
nauaneed wants to merge 5 commits into
Draft
Conversation
added 2 commits
August 9, 2026 12:13
This reverts commit 5122046. Pinning "mpi4py<4.0" makes installation of mpi4py fail with latest setuptools
`generate_body` in `acceleration_nnps_helper.py` hardcoded `sorted=True` when rendering `NNPS_TEMPLATE`. When `OctreeGPUNNPS` ran with `allow_sort=False` (the default), `NNPS_TEMPLATE` evaluated cell particle offsets directly as particle array indices (`d_idx = _pbound_here.s0 + lid`) instead of indexing into `_pids_dst` / `_pids_src`. On unsorted particle arrays, this resulted in out-of-bounds reads and caused segmentation faults on strict OpenCL runtimes (e.g., `pocl` on CI). This changes `generate_body` to default `sorted=False` so that `NNPS_TEMPLATE` correctly looks up particle indices via `_pids_dst` and `_pids_src`.
added 3 commits
August 9, 2026 14:09
pocl compiles OpenCL kernels with LLVM's CPU vectoriser, which groups
work-items into SIMD vectors and speculatively executes all lanes before
applying the predicate mask from conditionals such as `if (_svalid)` and
`if (lid < _m)`.
In NNPS_TEMPLATE the destination and source particle indices were computed
inside those guards:
if (_svalid)
d_idx = _pids_dst[_pbound_here.s0 + lid]; // OOB for lid >= s1-s0
if (lid < _m)
_pid_src = _pids_src[_pbound_here2.s0 + lid]; // OOB for lid >= _m
For the 10-particle test the pids arrays have exactly 10 elements; SIMD
lanes with lid = 10..31 speculatively read past the end of those buffers,
producing a SIGSEGV on pocl. On NVIDIA the GPU memory model is more
lenient and the same OOB read returns garbage silently, so the crash only
appeared on CI (pocl).
Fix both sites by clamping the array index unconditionally before entering
the guard, ensuring every SIMD lane accesses a valid (though possibly
wrong-but-safe) memory location:
unsigned int _d_lid = min((unsigned int)lid,
_pbound_here.s1 - _pbound_here.s0 - 1u);
d_idx = _pids_dst[_pbound_here.s0 + _d_lid]; // always in-bounds
Also corrects generate_body() to default sorted=False, matching the
OctreeGPUNNPS default of allow_sort=False. When sorted=True was
hardcoded the template skipped the _pids_dst indirection entirely,
which produced wrong particle indices for unsorted arrays (benign for
the 10-particle test because all particles land in one cell with s0=0,
but incorrect in general).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This reverts commit 5122046. Pinning "mpi4py<4.0" makes installation of mpi4py fail with latest setuptools