Skip to content

[Bug] Propagate the adstack size-expr onto scalarized tensor components.#796

Merged
duburcqa merged 5 commits into
mainfrom
duburcqa/fix_adstack_scalarize_size_expr
Jul 21, 2026
Merged

[Bug] Propagate the adstack size-expr onto scalarized tensor components.#796
duburcqa merged 5 commits into
mainfrom
duburcqa/fix_adstack_scalarize_size_expr

Conversation

@duburcqa

Copy link
Copy Markdown
Contributor

Issue: #793

Brief Summary

copilot:summary

Walkthrough

copilot:walkthrough

duburcqa added 2 commits July 20, 2026 15:37
`determine_ad_stack_size` runs before the scalarize pass. For an adaptive tensor-typed
adstack it records a symbolic `size_expr` and leaves `max_size` at the seed value of 1
for the runtime to overwrite from that tree. When scalarize splits the tensor stack into
one scalar `AdStackAllocaStmt` per component it copied `max_size` but dropped `size_expr`,
so the launcher found no tree and sized the component heap to the seed. A scalar component
stack pushed inside a runtime-bounded loop then overflowed as soon as the loop ran more
than once - a loud assertion on SPIR-V, silently corrupted gradients on a `__debug__`-
disabled build.

Clone `size_expr` onto every scalar component. The components are pushed and popped in
lockstep with the tensor stack, so they share its depth exactly.

Fixes #793.
@github-actions

Copy link
Copy Markdown

// `max_size`, the launcher finds no symbolic tree and sizes the heap to that seed, and a scalar stack pushed
// inside a runtime-bounded loop overflows once the loop runs more than once.
if (stmt->size_expr) {
scalar_ad_stack->size_expr = stmt->size_expr->clone();

@hughperkins hughperkins Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

note to self: scoped to only adstack things because inside a visit(AdStackAllocaStmt *stmt)

@hughperkins

Copy link
Copy Markdown
Collaborator

skimming => looks good to me

  • scoped to adstack only => no need for genesis benchmarks
  • probably good to run genesis unit tests though I think? (since they do run some adstack things I think? or no?)
  • no user-facing doc changes required ✅

@github-actions

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

@duburcqa

Copy link
Copy Markdown
Contributor Author

Production CI: GS_ENABLE_NDARRAY=1 pytest -v -ra --dev --backend gpu --forked tests
==== 867 passed, 9 skipped, 7 xfailed, 65 warnings in 1331.19s (0:22:11) =====

@duburcqa

duburcqa commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

MacBook Pro: pytest tests/core/test_grad.py -n 8 --backend cpu
============ 10 passed, 2 skipped, 6 warnings in 138.96s (0:02:27) =============
MacBook Pro: pytest tests/core/test_grad.py -n 8 --backend gpu
============ 10 passed, 2 skipped, 6 warnings in 137.63s (0:02:17) =============

@github-actions

Copy link
Copy Markdown

@hughperkins

Copy link
Copy Markdown
Collaborator

ok to merge

@github-actions

Copy link
Copy Markdown

@duburcqa
duburcqa merged commit bbefab1 into main Jul 21, 2026
58 of 59 checks passed
@duburcqa
duburcqa deleted the duburcqa/fix_adstack_scalarize_size_expr branch July 21, 2026 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants