Repository navigation
fix(das): never read outside the IQ data for out-of-range ALG1 samples - #4
Merged
Merged
Conversation
das_alg1_kernel zeroes the weight of a sample whose index falls outside [0, samples - 2], but still loads x[k] and x[k + 1] at that index. The load can land before or past the IQ buffer -- an illegal address for deep voxels whose offset + range exceeds the recording, and a NaN in the output whenever the value it returns is not finite (0 * NaN). Closes #2. The guard is branch-free: the load index is clamped into the channel's row (k -> 0 when out of range) and the interpolated value is replaced by an exact zero through a select, so the warp keeps main's control flow and a NaN/Inf at the clamped sample cannot reach the accumulator. The second interpolation tap steps by 1, or by 0 for a 1-sample recording, so the clamped pair stays in range for every samples >= 1. Outputs are bit-identical to main whenever the data is finite. The sparse path shares the kernel and is covered too. Measured on a real 4D-TEE problem (192 channels, 16 diverging waves, 255 samples, 7.95 M voxels, locked 1200 MHz, Quadro RTX 6000): within +0.6 % of main at batch 1-4, +1.1 % at batch 16-32 when every voxel fits the recording, and 2.0-2.6 % faster than main at batch 4-32 when 15 % of the voxels overshoot it. Branching around the load (#3) measured +1.9 % / +3.8 %. Tests: the reproduction from #2; reads before the start and past the end of a NaN-padded buffer (float32 / complex64, batch 1 / 3 / 16, dense and sparse); NaN in the first or last sample of every row; a 1-sample recording. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U7SVxqLx9EQwHmAi1kK7JA
There was a problem hiding this comment.
🟢 Approval recommended
The focused kernel fix correctly prevents invalid reads and is comprehensively covered by regression tests.
0 open findings
What changed in this PR
Prevents ALG1 from reading outside IQ buffers for invalid interpolation indices while preserving zero contribution behavior.
Changes:
- Clamps invalid sample loads and safely handles one-sample recordings.
- Adds dense, sparse, batched, real, complex, and NaN regression tests.
| File | Description |
|---|---|
src/das/das_alg1_kernels.cuh |
Safely clamps ALG1 interpolation loads. |
bindings/python/tests/test_das.py |
Covers out-of-range and single-sample cases. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Collaborator
|
Looks good. |
This was referenced Oct 12, 2026
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
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.
das_alg1_kernel zeroes the weight of a sample whose index falls outside [0, samples - 2], but still loads x[k] and x[k + 1] at that index. The load can land before or past the IQ buffer -- an illegal address for deep voxels whose offset + range exceeds the recording, and a NaN in the output whenever the value it returns is not finite (0 * NaN). Closes #2.
The guard is branch-free: the load index is clamped into the channel's row (k -> 0 when out of range) and the interpolated value is replaced by an exact zero through a select, so the warp keeps main's control flow and a NaN/Inf at the clamped sample cannot reach the accumulator. The second interpolation tap steps by 1, or by 0 for a 1-sample recording, so the clamped pair stays in range for every samples >= 1. Outputs are bit-identical to main whenever the data is finite. The sparse path shares the kernel and is covered too.
Tests: the reproduction from Issue #2; reads before the start and past the end of a NaN-padded buffer (float32 / complex64, batch 1 / 3 / 16, dense and sparse); NaN in the first or last sample of every row; a 1-sample recording.