Uh oh!
There was an error while loading. Please reload this page.
fix: widen the hub's public stream parameter to intptr_t + bump cpu/cuda pins - #50
Merged
Conversation
…uda pins A `cudaStream_t` is a pointer and therefore 64-bit, but every public `stream` parameter in the hub's device-dispatch functions was declared `int`. `ff::dt_euclidean` and friends inspect the DLTensor's device and forward the same argument to either `FF_CPU::` or `FF_CUDA::`: if (is_cuda) return FF_CUDA::dt_euclidean(inp_out, voxel_spacing, stream); else return FF_CPU::dt_euclidean(inp_out, voxel_spacing, stream); so both backends must agree on the parameter's type. Widens 114 parameter declarations across the 8 modules to `intptr_t`, matching the now-widened cpu/cuda dispatch layers. This is a signature-only change (verified with the same non-signature-line grep used in the cpu-lib/cuda-lib PRs: empty). Bumps `cpu` -> d969e2d (fastfields-cpu-lib#61) and `cuda` -> 33aa9b3 (fastfields-cuda-lib#35). Verified with `make all CXX=clang++`: the full hub (cpu library + hub object files + link) builds successfully end to end against the bumped pins, producing libfastfields.so linked against libfastfields-cpu.so. This is a real, complete build of the CPU path; the CUDA path is not linked by this target (CUDA needs `USE_CUDA=1` and nvcc, exercised at the cuda-lib layer instead). Refs #4 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
This was referenced Aug 2, 2026
Uh oh!
There was an error while loading. Please reload this page.
balbasty
commented
Aug 2, 2026
CollaboratorAuthor
Review summary (triage, jitfields-to-fastfields): merged as squash. 114 hub-level Workstream: claude-jitfields-to-fastfields Generated by Claude Code |
balbasty added a commit
to fastfields/fastfields-dlpack
that referenced
this pull request
Aug 2, 2026
…dary (#22) * fix: widen the CUDA stream parameter to intptr_t at the nanobind boundary Closes#4. A `cudaStream_t` is a pointer and therefore 64-bit, but every `stream` parameter accepted at this boundary was declared plain `int` in the nanobind lambda signatures. A real stream handle from a caller (e.g. a torch/cupy stream pointer forwarded as an integer) whose upper 32 bits are non-zero is silently truncated before it ever reaches the C++ library -- the last and most externally visible link in the truncation chain fixed by this issue across all five repos. Widens all 45 `stream` parameters in `src/ext.cpp` to `intptr_t`. Bumps the `_fastfields_lib` submodule pin to fastfields-lib's 16f3eb7 (fastfields/fastfields-lib#50), which carries the matching widening through the hub and both device-dispatch layers (fastfields/fastfields-cpu-lib#61, fastfields/fastfields-cuda-lib#35) down to the CUDA launchers themselves (fastfields/fastfields-cuda-impl#25). Verified end to end on this machine (CPU path; no GPU available): - `pip install --no-build-isolation .` succeeds: builds `_core.so` via nanobind against the bumped `_fastfields_lib`, links `-lfastfields`, and installs the wheel. - `import fastfields.dlpack._core` succeeds and exposes the expected entry points (dt_euclidean, dt_l1, dt_mesh, field_matvec, ...). - Regression check for the actual bug: calling `dt_euclidean(x, 1.0, 1 << 40)` -- a stream value with bits set above the 32-bit range -- completes without truncation or OverflowError, which a plain `int` parameter could not guarantee. - `pytest tests/`: 36 passed, 0 failed. Refs fastfields/fastfields-lib#4 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z * fix: bump _fastfields_lib pin past the RLS broadcast-stride fix fastfields-cpu-lib#62 (field_matvec_rls's RLS/shared-weight broadcast path reading the wrong stride, corrupting output for wc=1, C>1) has now been fixed and threaded all the way up through cpu-impl#48, cuda-impl#27, cpu-lib#64, cuda-lib#38, and fastfields-lib#55. This repo's _fastfields_lib pin (16f3eb7) predated fastfields-lib#55, so the fix was not reachable through this PR's pin -- the extension built here would still have linked against the broken field_matvec_rls. Bumps _fastfields_lib -> fd9e6c5 (fastfields-lib main HEAD, which now carries cpu -> 4d46a02 and cuda -> e96440c, both past the fix). Verified for real: CXX=clang++ pip install --no-build-isolation . cd /tmp && python -m pytest <repo>/tests -q Install succeeds end to end (builds libfastfields.so/-cpu.so via the bumped _fastfields_lib's Makefile, links the nanobind extension, packages and installs the wheel). pytest: 36 passed, 0 failed. Also re-ran the stream-widening regression check this PR introduced (stream = 2**40 via dt_euclidean) against the freshly built extension: still passes. part of fastfields-cpu-lib#62 Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This was referenced Aug 5, 2026
This was referenced Aug 20, 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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Part of #4. Repo 4 of 5; depends on fastfields/fastfields-cpu-lib#61 and fastfields/fastfields-cuda-lib#35.
What changed
Width.
ff::dt_euclideanand friends are the device-dispatch boundary: they inspect the DLTensor'sdevice.device_typeand forward the samestreamargument to eitherFF_CPU::orFF_CUDA:::Both branches must share one signature, so this hub was
inteverywhere the two lower layers wereint. 114 parameter declarations across the 8 modules (distance,posdef,pushpull,reg_field,reg_flow,resize,restrict,splinc) are nowintptr_t, matching the now-widened cpu/cuda dispatch layers. Signature-only — verified with the same non-signature-line grep used in the other PRs in this series (empty diff outsidestream/#include <cstdint>lines).Submodule pins.
cpu→d969e2d(fastfields/fastfields-cpu-lib#61),cuda→33aa9b3(fastfields/fastfields-cuda-lib#35).How this was verified
Really run:
make all CXX=clang++— a full, real build of this repo against the bumped pins: it recursively buildslibfastfields-cpu.so(viamake -C cpu install), compiles all 8 hub object files, and linkslibfastfields.soagainst it. Exit 0, artifact produced (build/libfastfields.so, 55 KB;build/lib/libfastfields-cpu.so, 163 MB).This exercises the CPU path end to end. The CUDA path is not linked by the default
alltarget (needsUSE_CUDA=1+ nvcc); CUDA compile+link verification lives at thefastfields-cuda-liblayer (PR #35), which is where the CUDA object files are actually produced.PR series
Workstream: claude-jitfields-to-fastfields
🤖 Generated with Claude Code