Skip to content
This repository was archived by the owner on Aug 20, 2026. It is now read-only.

fix: treat DLTensor.strides == NULL as compact row-major - #4

Merged
balbasty merged 1 commit into
mainfrom
claude/fix-6-null-strides
Jul 24, 2026
Merged

fix: treat DLTensor.strides == NULL as compact row-major#4
balbasty merged 1 commit into
mainfrom
claude/fix-6-null-strides

Conversation

@balbasty

Copy link
Copy Markdown
Contributor

What

DLPack permits DLTensor.strides == NULL for a compact row-major tensor, but every cpu-lib module fed x.strides straight into CANUSE32BITS / copy_if_needed and the impl loops — all of which dereference it, so a producer that omits strides would segfault.

  • autocast.h: new ContiguousStrides RAII wrapper holding a normalised copy of the (POD) descriptor; when strides are NULL it synthesises explicit contiguous strides (in elements) that outlive the call. An enable flag skips optional placeholder tensors (posdef weight, mesh nearest_vertex) whose shape may be invalid and whose strides are never read.
  • Every public entry (distance/posdef/resize/restrict/splinc/pushpull/reg_field/reg_flow) normalises its tensors through the wrapper before dispatch; the bodies are otherwise unchanged, so the strides-supplied path is byte-for-byte identical.
  • tests/test_resize: a null-strides regression — strides=NULL must match the same call with explicit contiguous strides.

Why

nanobind always supplies strides, so the current bindings never hit this — but the library's "accepts any __dlpack__ object" contract means an external DLPack producer can.

Verified

All 8 module test suites pass with the change compiled in (each test binary links all 8 module .cpp): distance 2350, mesh 4620, spline 510, posdef 3580, pushpull 175, reg_field 272, reg_flow 282, resize 211 (incl. the new null-strides case).

Independent of the kernels half (fastfields-kernels#…, a defense-in-depth guard in canUse32BitIndexMath) — this PR supplies non-null strides so it never relies on that guard.

Part of #6.

🤖 Generated with Claude Code


Generated by Claude Code

@balbastybalbasty left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

DLPack permits DLTensor.strides == NULL for a compact row-major tensor, but
every cpu-lib module fed x.strides straight into CANUSE32BITS / copy_if_needed
and the impl loops, all of which dereference it -> a producer that omits
strides would segfault. nanobind always supplies strides so the current
bindings never hit it, but the "accepts any __dlpack__ object" contract means
an external DLPack producer can.
- autocast.h: add a ContiguousStrides RAII wrapper holding a normalised copy of
the (POD) descriptor; when strides are NULL it synthesises explicit contiguous
strides (in elements) that outlive the call. An `enable` flag skips optional
placeholder tensors (posdef `weight`, mesh `nearest_vertex`) whose shape may be
invalid and whose strides are never read.
- Every public entry (distance/posdef/resize/restrict/splinc/pushpull/reg_field/
reg_flow) normalises its tensors through the wrapper before dispatch; the
bodies are otherwise unchanged, so the strides-supplied path is identical.
- tests/test_resize: a null-strides regression (strides=NULL must match the
same call with explicit contiguous strides).
Verified: all 8 module test suites pass (test_resize now 211 checks incl. the
null-strides case). Companion kernels change guards canUse32BitIndexMath.
Part of #6.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
@balbasty
balbastyforce-pushed the claude/fix-6-null-strides branch from 29d632f to e745872CompareJuly 24, 2026 11:47
@balbasty
balbasty merged commit 4f27247 into mainJul 24, 2026
1 check passed
@balbasty
balbasty deleted the claude/fix-6-null-strides branch July 26, 2026 06:04
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@balbasty@claude