P3 + remaining P1: fix the last two tiers of the 2026-08-29 review (3 rows device-unverified), and unblock the CUDA build - #643
Merged
Conversation
Two pre-existing breaks in examples/advanced/power_retention, both reachable ONLY with CUDA enabled — which is exactly why they survived: every CPU-only configure skips this subtree and never evaluates either. 1. add_subdirectory(src/extension) referenced a directory holding only a two-line pybind stub over <torch/extension.h> and no CMakeLists, so configure aborted. A torch extension cannot be a build target here (Decision #23), so guard the add_subdirectory rather than write the missing CMakeLists. 2. TesseraPower.td declared DeclareOpInterfaceMethods<MemoryEffectOpInterface> while including only OpBase.td, so tablegen failed with 'Variable not defined'. Add the SideEffectInterfaces.td include. Also add 120 to CUDA_ARCHITECTURES: consumer Blackwell (RTX 50-series) is the fleet's only CUDA device and was not in the list. Found while enabling the NVIDIA backend on The-Super-Bear to get real sm_120 device evidence. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Graph IR verifier rejects an attribute holding an SSA value
(GRAPH_IR_SSA_VALUE_IN_ATTRIBUTE) and points at
_KEYWORD_OPERANDS['tessera.matmul'] == ('bias','residual'). package_matmul
read them from op.kwargs instead, so the two halves of the compiler
disagreed about where a matmul's bias lives: valid IR produced
bias_name=None, no buffer binding, and the launch failed with
E_LAUNCH_BINDING_MISMATCH -- only IR the verifier REJECTS could reach the
working path. Rank disambiguates bias (1-D) from residual (2-D) without
new metadata; it is the contract this function already asserts when it
emits their buffer rows. The kwargs form is kept as a fallback for
legacy IR.
tests/device/nvidia/test_e2e_spine_native.py::_epilogue_module built the
invalid form, and its epilogue-provenance assertion predated the
packager's 'output' dtype field. Both corrected.
Measured on The-Super-Bear (RTX 5070, sm_120, CUDA 13.3), NVIDIA-enabled
build: test_e2e_spine_native went 80 failed/222 passed -> 20 failed/282
passed. The remaining 20 are the no-bias/no-residual cases failing at the
actual device invoke with rc=5 -- a genuine runtime defect that was
unreachable behind the verifier error, still open.
None of this is a regression from PR #640/#642: the same 80 failures
reproduce at 6478d94, main before #640.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Follow-on to d635808. The ops declared DeclareOpInterfaceMethods<MemoryEffectOpInterface>, which is wrong twice: the tablegen record is spelled MemoryEffectsOpInterface (the un-suffixed name is the C++ class), and declaring it obliges a getEffects() that exists nowhere in this example, so it would fail at link even once tablegen accepted it. These are tensor-in/tensor-out ops with no memory effects, so Pure states what they actually are and needs no C++. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
conv1d/conv_transpose (nn/functional.py): six-deep Python scalar loops replaced with sliding_window_view + einsum. Measured interleaved min-of-3: conv1d (8,64,256,k3) 6430 ms -> 0.94 ms; conv_transpose (8,64,256,k3) 522 -> 3.97 ms. The replaced loops are kept in the new test as oracles and compared over 16 corner configs x bias (max abs diff 1.5e-5, fp32 accumulation-order noise). Tradeoff: the window expansion is an im2col, so it costs ~kernel-times bounded temporary memory (3.7 MB peak for a 0.5 MB input) where the loop cost none. fusion_core.py pointwise exit scan: the by-input consumer index is now built in the pass that already computes use_count, replacing a per-member rescan. 7000 ops 606 -> 10.9 ms; region output byte-identical. Pinned by a complexity-class test (5x graph grew 22.3x before, 5.5x now). nvidia_cuda.py flash-backward: TSR_ATOMIC_ENTRY and the f16 wrapper now free through the same goto-fail block their siblings use, and the atomic entry CHECKS its H2D copies -- it previously fired them unchecked and a failed upload yielded a confidently wrong gradient. Verified as generated text and a clean clang++ -fsyntax-only parse; NOT device-verified. apple_msl.py: bf16 pointwise graph/reduce lanes cast the result back to bf16 instead of silently returning float32, matching the fused-region and gated-matmul lanes that already round-trip it. Executed on this M1 Max. f16/f32 paths byte-for-byte unchanged. nvidia_solver_krylov.py: tsr_matvec assigns a warp per row with lane-strided columns and a shfl_down butterfly. Reasoned claim only -- at a fixed inner iteration a load's 32 lanes now touch 32 consecutive elements of one row instead of 32 rows n*sizeof(T) apart, dropping transactions per load from 32 to 4 for f32. NOT device-measured, and the per-row summation order changed, so results are no longer bit-identical to a sequential sum. spectral_candidates.py: the Hermitian mirror is now a vectorized slice (bit-identical to the loop across 11 sizes of both parities; irfft n=4096 0.44 -> 0.045 ms), and the FFT lane resolves once per call rather than once per frame (istft 512 frames/win256 18.4 -> 4.11 ms). The ROCm batch seam is written from the .hip signature and declines cleanly when the image lacks the batch ABI, but is unexecuted -- no ROCm on this host. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
laws.py kink check (the correctness one): a forward-mode check that could not be evaluated returned status='pass' with a detail claiming both modes agree. A rigged maximum-JVP that hard-selects operand 0 and returns a keepdims-reduced tangent was reported as agreement it never verified. Every unevaluable path now returns a named rule_error, including three newly-closed empty-mask/no-probe-ran holes. All 12 registered kink ops still pass with real probes. vjp_moe: per-token matvec + outer loop replaced with expert-segmented batched GEMMs (the shape vjp_grouped_gemm already uses). T=4096 D=256 E=128: 171.6 -> 9.0 ms (19x), gradients agree to 1.9e-13. jacfwd: output shape now read from the first jvp's primal, which jacfwd was already discarding. Primitive-execution counter confirms (n+1) -> n passes exactly. The methodology test asserting per_pass*(n+1) was measuring the extra pass rather than justifying it, and is updated. law_inputs atan2: the tie_gap guard protected a kink that does not exist. Given |y| >= 0.25 with x free, since the derivative is smooth across the y=0 branch cut and only the value jumps — bounding x>0 as suggested would have cost quadrant coverage for nothing. Margin verified structural over 500 reseeds. degeneracy check_full_rank: judged batched spectra globally, so two element-wise healthy spectra were refused when stacked. Now per-row like check_factor_rank, reporting the worst element. degeneracy per-batch loop: added an exact vectorized screen — both _cluster_indices and _warn_if_thin_gap reduce to the same per-row minimum sorted gap, so rows above both thresholds provably cannot fire. svd_coupling (4096,8,8) 63.8 -> 1.2 ms (53x); exactness checked by 1600 randomized comparisons against a forced full-loop reference, 0 mismatches. GradScaler: the 1.0 backoff floor made a persistently-overflowing run stall forever with step() always False. Floor is now 2**-14 and hitting it raises E_GRAD_SCALE_EXHAUSTED (registered) rather than spinning; a zero floor would be its own silent-wrong-answer, since zero gradients never overflow. rng canonical hashing: 3, np.int64(3), np.int32(3) and np.uint8(3) forked four different streams. Normalization is stream-preserving — seed_high, four fold_in values and a split are byte-identical to HEAD, so no recorded checkpoint moves — and unsupported types now raise instead of falling back to repr. implicit.py FD rmatvec: memoize the FD columns rather than reroute to the dense path, because it fixes every consumer of the adjoint and does not silently substitute a solver the caller asked for. n=40 gmres 2964 -> 84 F-evals, exactly the dense budget; 13.45 -> 1.73 ms. A 64 MB element budget with a warned streaming fallback keeps the memo bounded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Records the two P3 rows that were correctness defects rather than improvements (the kink oracle claiming unverified agreement, and the GradScaler floor that stalled a run forever), the measured wins, and the two claims deliberately NOT made: the CUDA flash-backward cleanup and the Krylov warp-per-row rewrite are text- and parse-verified only, with the coalescing gain stated as a property of the emitted access pattern rather than a speedup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1bdf52f84
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The agent that produced them flagged that 'all four tiers closed' overstates its own work, and it is right. Two NVIDIA emitter rows (flash-backward cleanup, Krylov warp-per-row matvec) and the ROCm half of the spectral per-frame row are verified as generated text plus a clean clang++ -fsyntax-only parse only. No GPU executed any of them, and this repo does not count a source-level fix as a device claim. The review doc now says 'fixed in source, 3 device-unverified' rather than CLOSED, and the nvidia/rocm todos carry the specific runs owed. The Krylov row also flags that benchmarks/baselines/nvidia_sm120_solver_krylov_performance.json was recorded with the old matvec, so its ratchet will report a false result until re-recorded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
gstoner
changed the title
Owner
Author
|
Correction pushed in Three of the sixteen P3 rows are fixed in source but have never run on a GPU: The review doc now reads "fixed in source, 3 device-unverified" and the nvidia/rocm todos carry the specific runs owed. One of those is load-bearing beyond this PR: The other thirteen rows are host-measured, and |
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.
Closes the P3 tier (16 rows) and the 3 P1 rows left open as performance/scope items. With this, all four severity tiers of the 2026-08-29 review are done.
Also included: the sm_120 device work you asked for, which turned up a real compiler defect and three pre-existing build breaks.
Two P3 rows were correctness defects, not improvements
laws.py:749— a forward-mode kink check that could not be evaluated returnedstatus='pass'with a detail claiming both modes agree. Reproduced with a riggedmaximumJVP that hard-selects operand 0 and returns a keepdims-reduced tangent: reported as agreement it never verified. Every unevaluable path now returns a namedrule_error, and three further empty-mask/no-probe-ran holes were closed. All 12 registered kink ops still pass with real probes.mixed_precision.py:189— the 1.0 backoff floor made a persistently-overflowing run stall forever withstep()always False. Floor is now2**-14, raising the registeredE_GRAD_SCALE_EXHAUSTED. A zero floor would have been its own silent-wrong-answer, since zero gradients never overflow.Measured wins
conv1d(8,64,256,k3)svd_coupling(4096,8,8)vjp_moeT=4096 E=128irfftn=4096 Hermitian mirroristft512 frames / win256jacfwdExactness was checked, not assumed: the
degeneracyvectorized screen was validated by 1600 randomized comparisons against a forced full-loop reference (0 mismatches in result, warning count and raise message); the conv rewrite keeps the replaced loop nests as test oracles; the rng canonicalization is stream-preserving (seed_high, fourfold_invalues and asplitbyte-identical to HEAD, so no recorded checkpoint moves).Two claims deliberately NOT made
The CUDA flash-backward cleanup and the Krylov warp-per-row rewrite are verified as generated text plus a clean
clang++ -fsyntax-onlyparse only — no GPU ran them. The coalescing gain is stated as a property of the emitted access pattern (32 transactions per load down to 4 for f32), never as a speedup. The Krylov change also alters per-row summation order, sotests/performance/nvidia/test_solver_krylov_ratchet.pycompares against a baseline recorded with the old matvec and likely needs re-recording on the CUDA box.sm_120 device work
Running the NVIDIA device suite surfaced that it had never actually been running: the gate needs
nvidia-smionPATH, which under WSL2 lives in/usr/lib/wsl/liband is absent from a non-interactive shell. The suite reported 454 passed, 395 skipped, exit 0 while doing zero GPU work. With the GPU engaged: 768 passed, and none of the failures come from #640/#642 — the identical set reproduces at6478d945.That unmasked a real compiler defect, fixed here: the Graph IR verifier requires matmul
bias/residualto be operands (_KEYWORD_OPERANDSdeclares them) whilepackage_matmulread them fromop.kwargs, so only IR the verifier rejects could reach the working path. Valid IR produced no buffer binding and died atE_LAUNCH_BINDING_MISMATCH.test_e2e_spine_native.pywent 80 failed/222 passed → 20 failed/282 passed. The remaining 20 fail at the actual device invoke withrc=5, a genuine runtime defect that was unreachable behind two stacked masks; filed separately rather than rushed.Three pre-existing build breaks also fixed, all reachable only with
-DTESSERA_ENABLE_CUDA=ON— which is exactly why they survived: anadd_subdirectoryinto a directory with no CMakeLists holding atorch/extension.hstub (forbidden by Decision #23), a.tddeclaringMemoryEffectOpInterfacewith neither the correct tablegen name nor thegetEffects()it obliges, andCUDA_ARCHITECTURESmissing sm_120 — the fleet's only CUDA device.Verification
lit tests/tessera-ir/439/439 (Mac), and 348 passed / 91 unsupported / 0 failed on the CUDA boxmypyand the CIruffgate clean; generated-doc drift gate in sync🤖 Generated with Claude Code