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

[teeny] Phase B3: pushpull pull/push/count/grad import via tny::from_dlpack - #81

Merged
balbasty merged 3 commits into
teenyfrom
claude/77-pushpull-from-dlpack
Aug 5, 2026
Merged

[teeny] Phase B3: pushpull pull/push/count/grad import via tny::from_dlpack#81
balbasty merged 3 commits into
teenyfrom
claude/77-pushpull-from-dlpack

Conversation

@balbasty

@balbastybalbasty commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Agent:claude-fastfields-to-teeny

Phase B3 of the tensor-native-boundaries umbrella (fastfields-lib#57) — the cpu-lib half.

This is one logical change split across two repos only because GitHub cannot span them: the impl half is fastfields/fastfields-cpu-impl#68, which converts the four entry points to take carriers. Neither PR compiles without the other — please review them together (same split convention as Phase A's cpu-impl#60 / cpu-lib#74).

Convention reference: fastfields-lib/TEENY-MIGRATION.md §9. Template: the merged Phase A distance.cpp shims (#74).

Closes#77


What changed

Each of the four exported entry points ran the explode-and-reassemble round trip — ContiguousStrides normalisation, VOIDPTR/CVOIDPTR byte-offset folds, 22 copy_if_needed calls across the leaf wrappers, and a use_32bits ternary in every dtype leaf — feeding impl signatures that immediately rebuilt the three carriers with as_anyrank. tny::from_dlpack does all three parts by construction, so the carrier is built once, in the existing dtype dispatch, and handed straight down:

template <int D, int O, bound_t B, typenamescalar_t>
inlinevoid_pull(DLTensor & out, const DLTensor & inp, const DLTensor & grid,
int extrapolate, bound_t bnd)
{
auto ao = tny::from_dlpack< scalar_t>(&out );
auto ai = tny::from_dlpack<constscalar_t>(&inp ); // R4 const-element carriersauto ag = tny::from_dlpack<constscalar_t>(&grid);
pushpull::pull<D, O, B, reduce_t>(ao, ai, ag, extrapolate, bnd);
}
  • D1/R5: the use_32bits ? …int32_t… : …int64_t… ternaries collapse to int64 and CANUSE32BITS is deleted. Instantiation count halves, as expected — the full-matrix pushpull.o drops from 34.2 MB to 17.5 MB (clang -O2) and 57.4 MB to 30.4 MB (g++ -O2), and compiles 2.6–4.2× faster.
  • Every copy_if_needed/free_if_needed/VOIDPTR/CVOIDPTR/ContiguousStrides use in this file is gone, and with them the file's #include "autocast.h" (nothing else in this TU used it — canUse32BitIndexMath lives in impl/kernels/utils.h, already included). autocast.h itself is untouched, per the last-user rule at the umbrella; other modules still use it.
  • The shims stay rather than collapsing into the dispatch arms: the macro pyramid supplies D/O/B/ABS and scalar_t as explicit template arguments, and its shape and exact rejection messages are behavioural ABI. They are no longer void* re-casts — the import is their whole body.

Preserved behaviour — each item checked, not assumed

1. Exported signatures byte-for-byte unchanged.pushpull.h is untouched; the exported symbols are identical before and after (diff of the two symbol lists is empty):

_ZN2ff3cpu4pullER8DLTensorRKS1_S4_aaai _ZN2ff3cpu4pushER8DLTensorRKS1_S4_aaai
_ZN2ff3cpu5countER8DLTensorRKS1_aaai _ZN2ff3cpu4gradER8DLTensorRKS1_S4_aaabi

2. Every CHECK_* stays verbatim — message, argument order and evaluation order. Including:

  • CHECK_RANK_FITS, which must keep rejecting a > TNY_MAX_RANK tensor with its std::invalid_argument in a release build (from_dlpack's own rank guard is a debug _TNY_CHECK, so removing this would have turned a clean throw into UB);
  • CHECK_SAME_SPATIAL, the equality the per-tensor decode relies on — the impl trusts it, so it must keep throwing here (see the judgment-call section of the impl PR);
  • the lanes, dtype-pair, rank and batch checks, all still evaluated before any import, so a rejection is still reached without ever building a carrier.

3. The dispatch pyramid and FF_TEST_SPARSE structure stay as-is (R1) — only the dtype leaf lost its offset-width ternary.

4. strides == NULL and byte_offset != 0 still work — and are now actually tested. They were handled by ContiguousStrides and VOIDPTR/CVOIDPTR, and no pushpull test exercised either one. Rather than delete the code that handled them and lose the coverage silently, test_pushpull gains test_descriptor_variants: six cases (NULL strides / non-zero byte_offset / both, × float and double) each running all four entry points and requiring the variant descriptor to reproduce the fully-explicit descriptor's answer element-for-element, plus a sentinel assertion that the padding in front of byte_offset is not written on any of the six buffers — so a mis-folded offset fails loudly instead of reading right and writing left. That is 1704 checks, 326 → 2030.

Crucially, those 1704 checks also pass against the pre-refactor code — verified by building the new test file against the old ContiguousStrides/VOIDPTR implementation, which gives the identical 2030 / 0. They pin existing behaviour; they do not encode the new implementation.

5. DLPack include order (R7).<teeny/dlpack.h> is included after this repo's vendored "dlpack.h", so the TU stays on fastfields' DLPack v1.2 throughout (teeny vendors v1.1; same include guard, first one wins). A comment records why the order matters.


The Phase B3 gate — all criteria, with evidence

Measured on one machine, clang++ 18.1.3 and g++ 13.3.0, -std=c++17 -DTNY_MAX_RANK=64, in the library configuration (full order × bound matrix, no -DFF_TEST_SPARSE).

Measurement hygiene. Sibling agents were concurrently editing the shared /home/user checkouts of these repos (Phase B2/posdef had already switched both onto its own branches with uncommitted edits). All measurements were therefore taken in dedicated git worktrees branched off origin/teeny, with the submodule symlink nesting rewired to the isolated copies.

1. Kernel instantiations byte-identical at -O2 (int64 arms) — ✅

vox::* are static inline and emit no standalone symbol on either compiler (fully inlined), so criterion 1 is measured on the kernel-layer helpers that do survive as symbols — _make_axis, _make_axis_g, _axes_from, _push_rec, _grad_rec, gather_sep, _sepgather — and the vox:: bodies fall to criterion 2. Comparison is relocation-aware (raw section bytes plus every relocation inside the symbol, normalised to offset/type/target).

buildcommondiffering bytesdiffering relocsresolve to same constant
clang++ -O2225016 50016 500 / 16 500
clang++ -O3210016 51416 514 / 16 514
g++ -O29113 ¹21 90621 828 + 78 ²
g++ -O3g++ inlines every kernel helper at -O3 in both objects

Baseline-only symbols are exactly the deleted int32 arms (R5); new-only symbols: 0. Differing relocations point into the merged constant pool or at clang's anonymous .LCPI#### labels, which shift because the TU holds half as many functions; each is resolved to the constant it names and compared — 100 % identical on clang.

¹ All 13 are _axes_from, each with identical byte size, identical instruction count and identical mnemonic multiset (register allocation / scheduling only — the accepted g++ whole-TU jitter, measured rather than asserted). ² Those 78 offset shifts are confined entirely to the same 13 symbols; across the 78 byte-identical g++ symbols, all 19 912 differing relocations resolve to the same constant, zero exceptions.

2. Driver loops: no new per-voxel instruction; the cur_b cache survives — ✅ (clang) / measured and explained (g++)

Loops are located structurally (backward branch → body [target, branch]) because the driver's mangled name deliberately changes; branch/call operands are blanked, every other operand compared literally.

buildbaseline loop bodies (int64 arm)afterverdict
clang++ -O21440 syms / 30 4651440 / 30 465IDENTICAL multiset
clang++ -O31440 syms / 34 7881440 / 34 788IDENTICAL multiset

The drivers can also be paired per instantiation — both manglings still carry (op, D, O, bound, ABS, dtype):

buildpairedbyte-identicalsame count+opcode mixnew LARGERnew SMALLER
clang++ -O214401440 (100 %)000
clang++ -O314401440 (100 %)000
g++ -O21236551275653
g++ -O3139620

On clang the entire driver — per-voxel loops, inlined kernel and all — is byte-for-byte unchanged at both optimisation levels, which is the compiler-independent statement that this source change is codegen-neutral.

g++, stated plainly. g++ re-inlines a TU that just halved and does not reproduce byte identity. Net it emits less code (at -O2: 35 783 fewer instructions, 790 fewer integer divisions across the paired set), but a minority grow and I am not rounding that away: 5/1236 at -O2 (+120/+116/+46/+7/+4, extra opcodes almost entirely mov spills and NOP padding) and 20/1396 at -O3 (max +45, mov/lea/add/sub/NOPs). 17 of 1396 instantiations at g++ -O3 gained exactly one in-loop integer division — the one item I could not make identical. It is not the failure this criterion guards against: had the cur_b cache stopped eliding the per-voxel peel_front_at<-(D+1)>, every instantiation would have gained a whole mixed-radix batch decode (a per-axis loop with a division per axis), not one division in 1.2 % of them — and at -O2 the same measurement moves the other way. See the impl PR for the full reasoning.

Harness validated in both directions (self-comparison 16 409/16 409 identical; -O2 vs -O3 negative control correctly reports differences), and three independent implementations of the loop comparison agree exactly on the clang -O2 figure.

3. Oracle: test_pushpull unchanged at 326 with the original test file — ✅

326 / 0 at baseline on both compilers; still exactly 326 / 0 after the refactor with the original test file. The committed suite adds the 1704 descriptor-variant checks → 2030 / 0 on both compilers, and those also pass against the pre-refactor code.

Every run had a verifiedmake clean (cpu-lib#56's trap): post-clean counts recorded each time, all four lib-objects/libs/deps=0 test-objects/deps=0 test-binaries=0.

4. ASan + UBSan — ✅

-fsanitize=address,undefined, detect_leaks=1, halt_on_error=1, print_stacktrace=1: 2030 checks, 0 failures, 0 sanitizer reports, exit 0 (g++; this box has no clang libclang_rt.asan, as in Phase A).

5. Whole suite green, unchanged counts on every other module — ✅

All 12 binaries, both compilers, verified clean between. diff of the baseline vs post-change listing gives exactly one differing line per compilertest_pushpull — and the clang and g++ listings are identical to each other.

binarybaselineafter
test_distance30673067
test_distance_mesh46224622
test_distance_spline704704
test_posdef50925092
test_pushpull3262030
test_reg_dispatch60306030
test_reg_field525525
test_reg_flow11231123
test_reg_op186186
test_resize41304130
test_restrict191191
test_splinc45774577
total30 57332 277

Judgment calls

The int32 arm is deleted outright, not kept behind a flag. That is D1/R5, and the check counts confirm it is behaviour-neutral: the test_inflated_stride case (an inp batch stride ≥ INT32_MAX, which used to force the int64 arm) and every small-tensor case now both run the int64 instantiation, with identical results.

A new (benign) failure mode, same as Phase A.from_dlpack debug-asserts that the tensor's device_type matches the carrier's memory space, so a kDLCUDA tensor handed to ff::cpu::pull now trips that assert in a non-NDEBUG build where previously it would have dereferenced a device pointer on the host. Strictly an improvement, and unreachable in practice (fastfields-lib dispatches on device before reaching ff::cpu), but it is a behaviour change.

The spatial-extent equality stays enforced only here. The impl trusts CHECK_SAME_SPATIAL rather than re-asserting it; the reasoning is in the impl PR's judgment-call section.

…77)
The four exported entry points ran the explode-and-reassemble round trip --
ContiguousStrides normalisation, VOIDPTR/CVOIDPTR byte_offset folds, 22
copy_if_needed calls across the leaf wrappers, a use_32bits ternary in every
dtype leaf -- to feed impl signatures that immediately rebuilt the carriers
with as_anyrank. tny::from_dlpack does all three parts by construction: it
folds byte_offset into the data pointer, expands a NULL strides field to
row-major, and copies each tensor's OWN shape/stride metadata into the
carrier. The carriers are now built once, in the existing dtype dispatch,
and handed straight down.
D1/R5: the CPU int32 offset arm is gone -- it measured a wash on a 64-bit
ALU -- so the use_32bits ternaries collapse to int64 and CANUSE32BITS is
deleted. The instantiation count halves, as expected: the full-matrix
pushpull.o drops from 34.2 MB to 17.5 MB (clang -O2) and 57.4 MB to 30.4 MB
(g++ -O2). Read-only operands are imported as `const scalar_t` carriers (R4).
Everything the issue calls behavioural ABI is untouched: the exported
signatures in pushpull.h, every CHECK_* (including CHECK_RANK_FITS, which
must keep rejecting a >TNY_MAX_RANK tensor with its invalid_argument in a
release build, and CHECK_SAME_SPATIAL, which the per-tensor decode relies
on), their messages and their evaluation order, and the D/order/bound
dispatch pyramid with its FF_TEST_SPARSE structure (R1).
test_pushpull gains test_descriptor_variants: `strides == NULL` and
`byte_offset != 0` were handled by ContiguousStrides/VOIDPTR and exercised
by no test at all, so they are pinned here across all four entry points
before the code that handled them is deleted. Those checks also pass against
the pre-refactor implementation (verified), so they pin existing behaviour
rather than the new one. 326 -> 2030 checks; every other binary unchanged.
R7: <teeny/dlpack.h> is included after this repo's vendored "dlpack.h" so
the TU stays on fastfields' DLPack v1.2 (same include guard, first wins).
Paired with fastfields-cpu-impl#63, which converts the impl entry points.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015xcQBMEdA5eQtTsx2wB3AC
The pushpull re-skin landed in cpu-impl as a separate PR (#68, merged
as 3f90eb6) after this PR's impl pin was measured. Bump the impl
gitlink so a submodule checkout of this branch builds against the
teeny-carrier pushpull.h rather than the stale pre-refactor version.
Bring in Phase B1 (resize/restrict/splinc, #79) and Phase B2 (posdef,
#80), both merged after this branch was created. B3 does not touch
those files, so this should be a clean union aside from the impl
submodule pin, which both sides changed.
# Conflicts:
#	impl
@balbasty
balbasty merged commit 77f9a21 into teenyAug 5, 2026
@balbasty
balbasty deleted the claude/77-pushpull-from-dlpack branch August 5, 2026 12:32
@balbastyClaude

Copy link
Copy Markdown
ContributorAuthor

Merged as 77f9a21 (impl half: cpu-impl#68, 3f90eb6).

Independently reviewed with a from-scratch toolchain (custom ELF64 reader, relocation-aware disassembly diff, canonical driver-name pairing across 5,760 symbols per build) — full report on the impl PR's thread. Summary: approved, clang side byte-for-byte identical across the entire driver+kernel set at both -O2/-O3 (5,760/5,760), g++ side explained mechanically rather than statistically (the flagged 17-division delta at -O3 is one peel_front_at cell-builder callee migrating from out-of-line to inlined per case, not new work — traced via call-relocation count 4→3 on all 17). Oracle, ASan/UBSan (on both compilers, including clang — the review's box had a working clang ASan runtime the implementer's didn't), and whole-suite counts all independently reproduced exact.

One mechanical note for the record: this branch predated Phase B1 (#79) and Phase B2 (#80) merging into teeny, so the merge required reconciling the impl submodule pin (both sides had moved it) — resolved by merging origin/teeny in and keeping this PR's pin (3f90eb6), confirmed to have both B1's and B2's cpu-impl SHAs as ancestors. Full suite re-verified green post-merge (test_pushpull at 2030 as expected) before squashing.

Closing #77.


Generated by Claude Code

balbasty added a commit that referenced this pull request Aug 18, 2026
Regression test for fastfields-kernels#48 / #81: the boundary-corrected
diagonal expanded each corner weight as
(fx0*fy0 + fx1*fy0 + fx1*fy0 + fx1*fy1), double-counting fx1*fy0 and
dropping fx0*fy1.
The oracle needs no reference implementation. On a square/cubic domain with
the same boundary condition and voxel size on every axis, relabelling two
spatial axes maps the operator onto itself, so its diagonal must be invariant
under that relabelling:
* field, 2D and 3D: diag(x,y,c) == diag(y,x,c)
* flow bending-only: diag(x,y,c) == diag(y,x,c) (channels uncoupled)
* flow diag_all: diag(x,y,c) == diag(y,x,1-c) (the Lame terms couple
the channels, so the axis swap carries a channel
swap; shears == div keeps the Lame part symmetric
under that simultaneous relabelling)
The 3D cases matter on their own: the 3D diagonal carries three independent
corner cross-terms (w110/w101/w011) rather than one, so the xy and yz swaps
exercise two directly and the third by composition.
Each test sweeps the whole domain rather than a single row/column, and runs
under Zero, DCT2 and DST2. DCT2 is a deliberate control: it does not flip
signs, so it passed even before the fix -- the error only appears where the
two axes' one-sided boundary signs differ.
Verified to bite. Same tests, same binaries, only the kernels headers
differing:
kernels test_reg_field test_reg_flow
pre-fix 19250 checks / 328 failures 16347 checks / 1696 failures
1df9fd3 19250 checks / 0 failures 16347 checks / 0 failures
Check counts are identical in both directions, so the only difference is
pass/fail, and every failure is in the new checks -- no pre-existing test
changes value.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@balbasty@claude