[fastcache] Fix parameter pruning for dead-static-branch and swapped-slot forwarding by hughperkins · Pull Request #809 · Genesis-Embodied-AI/quadrants

hughperkins · 2026-07-23T18:28:13Z

Issue: #

Brief Summary

copilot:summary

Walkthrough

copilot:walkthrough

…-slot forwarding (#808) Adds failing repros for the two fastcache parameter-pruning bugs from #808: - dead-static-branch field forwarded through a caller path walked before the live instantiation populates the shared used-set (flat and 3-level nested) - same-flat-name forwarding into swapped callee slots, cross-caller and same-caller Also adds a reversed walk-order variant of the flat dead-static-branch case, which passes on main and guards against an order-dependent fix.

…rding (#808) Record per-call-site call-graph edges during the discovery pass and propagate used-sets to a worklist fixpoint afterwards, instead of a single-shot copy keyed by callee. - Dead-static-branch (bug A): a callee's used-set (shared across template instantiations via its func id) can grow after an earlier caller copied it, so the enforcing pass pruned a parameter the callee still needed. The fixpoint makes propagation order-independent. - Swapped-slot forwarding (bug B): the caller-arg -> callee-param map was keyed by callee func id alone, so call sites forwarding the same flat name into different callee slots collided. Edges are now keyed per call site (source position). filter_call_args now takes the caller func id and looks up the per-call-site edge; kernel.materialize runs propagate_fixpoint() between discovery and the prefix-collapse. The fastcache-load path is unaffected (it skips discovery and only re-parses the kernel function def).

…runing test (#808) Forwards the dataclass by keyword (md=md) at every call site, exercising the kwarg pruning path (_expand_Call_dataclass_kwargs gated on the callee used-set) rather than the positional filter_call_args, confirming the used-set fixpoint also closes over keyword-forwarded edges. use_deep stays positional to avoid passing the template value by keyword (an unrelated concern).

hughperkins · 2026-07-23T18:28:20Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4725667cfa

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-07-23T18:31:35Z

-        # We can get the child's name directly from our own keyword node.
+                edge.pairs.append((caller_arg_name, callee_param_name))
+                if caller_arg_name.startswith("__qd_"):
+                    edge.positional_map[caller_arg_name] = callee_param_name


Key forwarded positional args by slot

When a caller passes the same dataclass field into multiple positional parameters, e.g. inner(md, md), the expanded call contains the same flat name in two different slots. This dict is keyed only by that caller flat name, so the later slot overwrites the earlier one; if the callee uses the first parameter but not the second, the enforcing pass consults the overwritten mapping, treats both occurrences as the unused parameter, and prunes the required argument, causing a missing-argument failure or wrong call wiring. Store the positional mapping by argument index, or otherwise keep per-slot entries, rather than by caller name alone.

Useful? React with 👍 / 👎.

…#808)

…callee slots (#808) Key CallEdge.positional_map by caller arg name with an ordered list of callee params (one per occurrence) instead of a single value, and consume them in left-to-right order during the enforcing pass. A single name->name map let a later slot (e.g. the second md in inner(md, md)) overwrite an earlier one, so the enforcing pass could prune a positional arg the callee still needs, raising 'Missing argument'. Keying by name (not slot index) keeps the mapping robust to the positional shift that occurs when an upstream caller prunes fields out of a forwarded dataclass.

hughperkins · 2026-07-23T18:49:53Z

@codex review

chatgpt-codex-connector · 2026-07-23T18:55:00Z

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 7654dab4c3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

hughperkins · 2026-07-23T18:58:29Z

Genesis Benchmarks fine:

hughperkins · 2026-07-23T18:59:23Z

Probably should create a pre-release, and check compilation time, on genesis benchmarks (or read from above benchmark csv reulst perhaps)

hughperkins · 2026-07-23T19:10:46Z

compile time up a bunch apparently 🤔

Let's try running this also on @duburcqa 's branch, in case his does not increase compile time.

duburcqa · 2026-07-23T19:13:55Z

ok to merge

duburcqa · 2026-07-23T19:14:35Z

compile time up a bunch apparently 🤔

WoW. 40% is hardcore. Hopefully it is "noise".

github-actions · 2026-07-23T19:20:25Z

Total: 4 file(s) changed, +274 -48 code lines.

hughperkins · 2026-07-23T19:53:18Z

Timings on alexis's branch (both sets of timings are predicated on AI analysing the resutls to be clear).

hughperkins · 2026-07-23T19:53:50Z

I might make AI write me a script to outputthes tables, so I dont have to rely on AI doing the right thing.

hughperkins · 2026-07-23T20:09:23Z

Alexis's branch, same run as above, but compile time processed using a standalone script, without needing AI:

hughperkins · 2026-07-23T20:17:27Z

In the meantime, Genesis unit tests passing:

github-actions · 2026-07-23T20:20:16Z

Diff coverage: 100% · 272 lines, 0 missing

hughperkins · 2026-07-23T20:44:21Z

Tried benchmarking main, as a control. Also shows higher compile time, though not quite as bad, so above compile time delta could be an artifact, plus some variance, maybe (continuing to dig)

hughperkins · 2026-07-23T21:14:15Z

AI investigated compile time. Seems not an issue:

Compile-time investigation

TL;DR: the reported "+11.5% compile time" for this branch is almost entirely a measurement artifact of comparing a locally source-built branch against the WandB CI main baseline, not a real cost of the fix. A same-node, source-built A/B (main vs this branch, interleaved on one exclusive RTX node) shows ~0 real compile overhead and identical pruning output on the worst-case benchmark.

Evidence 1 - the WandB baseline is not a valid zero

Benchmarking source-built main against the WandB main baseline (i.e. the same code on both sides, which should read ~0%) instead shows a systematic slowdown:

mean compile: wandb-main=105.0s  source-main=111.4s  delta=+6.4s (+6.1%)
anymal_uniform:   127.5 -> 156.4  (+22.7%)
box_pyramid_5:    118.4 -> 134.2  (+13.3%)
franka_random CG: 116.4 -> 130.3  (+11.9%)

So a local source build on the cluster node compiles ~6% slower (single-shot) than the WandB baseline for identical code. The three "vs WandB-main" runs line up as:

run (vs WandB-main)	mean compile delta
source-`main` (same code!)	+6.1%
duburcqa fix	+8.3%
this branch	+11.5%

The per-benchmark regression pattern (worst on anymal_uniform, box_pyramid_5, ...) is the same for main-vs-main as for the branches - the tell that it's a baseline artifact, not code.

Evidence 2 - same-node A/B (removes the artifact)

Built both main and this branch from source on the same exclusive RTX node and interleaved them, with per-phase pruning instrumentation. Worst case (anymal_uniform, 136 kernels):

metric	`main`	this branch	delta
kernels compiled	136	136	-
discovery pass	0.178s	0.180s	+0.003s
used-set fixpoint	0.000s	0.006s	+6 ms
enforce pass	0.161s	0.161s	0
backend codegen	61.55s	61.99s	+0.44s (+0.7%)
params kept (pruning output)	4190	4190	0 (+0.0%)
Python pruning total	0.339s	0.348s	+9 ms
runtime FPS	-	-	-0.13%

Harness verdict: "NO significant before/after difference."

Conclusion

The new propagate_fixpoint() costs ~6 ms across 136 kernels (656 edges, 46k forwarding pairs) - 0.01% of a ~62s compile. Compile time is dominated by backend codegen (~62s); the entire Python pruning phase is ~0.35s either way.
On anymal_uniform the pruning output is identical (params_kept 4190 == 4190), so the two builds emit the same kernels; the +0.7% backend delta is noise.
The apparent "+11.5%" is the WandB-baseline confound (independently reproduced at +6.1% by the main-vs-main control), not this change.

Caveat: anymal_uniform does not exercise the #808 dead-static path (pruning output is unchanged), so this cleanly measures the machinery overhead (edge recording + fixpoint) = negligible. It does not measure the intended cost in bug cases, where the fix correctly keeps params that main was wrongly pruning - a localized correctness cost on affected kernels only.

hughperkins added 3 commits July 23, 2026 06:57

chatgpt-codex-connector Bot reviewed Jul 23, 2026

View reviewed changes

hughperkins added 3 commits July 23, 2026 11:32

[fastcache] Collapse filter_call_args call to a single line (black)

9f73e7b

[test] Add same-name-into-two-slots fastcache pruning regression test (…

4b2cf70

…#808)

hughperkins merged commit 47e9dfb into main Jul 23, 2026
57 checks passed

hughperkins deleted the hp/fastcache-prune-dead-static-tests branch July 23, 2026 21:15

hughperkins mentioned this pull request Jul 23, 2026

fastcache=True fails to compile a kernel when a @qd.func/kernel dataclass field is read only inside a compile-time-dead qd.static branch and the enclosing struct is forwarded through more than one caller path #808

Closed

Uh oh!

Conversation

hughperkins commented Jul 23, 2026

Brief Summary

Walkthrough

Uh oh!

hughperkins commented Jul 23, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

hughperkins commented Jul 23, 2026

Uh oh!

chatgpt-codex-connector Bot commented Jul 23, 2026

Uh oh!

hughperkins commented Jul 23, 2026

Uh oh!

hughperkins commented Jul 23, 2026

Uh oh!

hughperkins commented Jul 23, 2026

Uh oh!

duburcqa commented Jul 23, 2026

Uh oh!

duburcqa commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

hughperkins commented Jul 23, 2026

Uh oh!

hughperkins commented Jul 23, 2026

Uh oh!

hughperkins commented Jul 23, 2026

Uh oh!

hughperkins commented Jul 23, 2026

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

hughperkins commented Jul 23, 2026

Uh oh!

hughperkins commented Jul 23, 2026

Compile-time investigation

Evidence 1 - the WandB baseline is not a valid zero

Evidence 2 - same-node A/B (removes the artifact)

Conclusion

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

duburcqa commented Jul 23, 2026 •

edited

Loading