Skip to content

docs: correct the Blackwell/Triton attribution behind the #278 compile fallback - #2085

Open
Moep90 wants to merge 1 commit into
debpalash:mainfrom
Moep90:fix/triton-sm120-attribution
Open

Moep90 wants to merge 1 commit into
debpalash:mainfrom
Moep90:fix/triton-sm120-attribution

Conversation

@Moep90

@Moep90 Moep90 commented Sep 14, 2026

Copy link
Copy Markdown

Three comments justify the #278 compile fallback by naming Blackwell sm_120 as an architecture Triton/Inductor does not support. On the pinned build it is supported, and the error text they quote is not an architecture symptom at all.

Evidence

Measured on an sm_120 Blackwell GPU (capability (12, 0)) with the repo's pinned environment, torch 2.8.0+cu128 / triton 3.4.0:

Check Result
torch.compile default, incl. recompile on a new shape passes
mode="reduce-overhead" ×3 — the cudagraph_trees.py path #278 names passes
attention module, growing sequence length, reduce-overhead passes
raw Triton kernel with Inductor bypassed passes, exact
compiled vs eager numerics allclose=True, maxdiff 1.19e-06 and 0.0

The app's own gate agrees: arch_unsupported(torch) returns None and _cuda_arch_supported_for_compile() returns (True, ""), so VoiceStudio already attempts compile on sm_120 with the pinned build — and per the table that is fine.

The quoted error is device-independent

#278 quotes:

Detected that you are using FX to symbolically trace a dynamo-optimized function. This is not supported at the moment.

That still reproduces — and reproduces identically with CUDA_VISIBLE_DEVICES="" and CUDA unavailable. Dynamo raises it unconditionally whenever FX traces a compiled function (torch/_dynamo/eval_frame.py). It is a compile-stack error the classifier should keep catching, but it was never evidence of a missing-architecture problem.

What changes

File Change
tests/test_compile_fallback.py docstring: generic "arch missing from the build's arch list", sm_120 noted as no longer an example, FX error de-attributed
backend/services/model_manager.py same correction in _is_compile_runtime_failure
backend/services/engine_env.py same correction in _cuda_arch_supported_for_compile

What does not change

No behaviour, no test logic, no gate. The fallback contract — compile is an optimization, never a point of failure — is correct regardless of which architecture exposed it, and the arch-list gate is already generic rather than a Blackwell blocklist. I am not claiming the #278 reports were invalid: on a build whose arch list lacks the device, the described mechanism is exactly right. What is stale is the example and the FX attribution.

Scope limit: the tests exercise synthetic modules, not the shipped TTS engines, and a cudagraph_trees AssertionError not reproducing here is not proof it cannot occur with a specific model. That is precisely why the fallback stays.

Verification

tests/test_compile_fallback.py
tests/test_torch_compile_gate.py
tests/test_cuda_arch_compat.py      33 passed

Follows #2084, which corrects the neighbouring claim that the pinned torch lacks sm_120 kernels. Independent of it — different issue (#278 vs #1931), different files, no overlap.

Updated compile-fallback documentation to state that pinned torch 2.8.0+cu128 and triton 3.4.0 support Blackwell sm_120, and that the FX tracing error is device-independent. No behavior or architecture gates changed; review risk is limited to documentation accuracy.

… fallback

Three comments name Blackwell sm_120 as an architecture Triton/Inductor does
not support. Measured on an sm_120 device with the pinned torch 2.8.0+cu128
and triton 3.4.0: torch.compile default and reduce-overhead (the
cudagraph_trees path debpalash#278 names), an attention module over growing sequence
lengths, and a raw Triton kernel all run, with compiled output matching eager
(maxdiff 1.19e-06 and 0.0). The app's own probe agrees — arch_unsupported
returns None, so compile is already attempted there.

The error text debpalash#278 quotes is also misattributed. "Detected that you are using
FX to symbolically trace a dynamo-optimized function" reproduces with CUDA
unavailable: Dynamo raises it whenever FX traces a compiled function,
regardless of device. It belongs in the compile-stack classifier, not in the
evidence for a missing-architecture failure.

Comments only. The fallback contract and the arch-list gate are unchanged and
still correct: the gate is generic rather than a Blackwell blocklist, and on a
build whose arch list lacks the device the described mechanism holds. Only the
example and the FX attribution are stale.

Signed-off-by: Moep90 <volleyballlive@googlemail.com>

Signed-off-by: moep90 <volleyballlive@googlemail.com>
@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 2d6fcdc0-adb1-4165-ae24-ad82e78da135

📥 Commits

Reviewing files that changed from the base of the PR and between eaf8bb9 and df18ac1.

📒 Files selected for processing (3)
  • backend/services/engine_env.py
  • backend/services/model_manager.py
  • tests/test_compile_fallback.py

Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

The pull request updates service documentation, explanatory comments, and test documentation. It records current sm_120 support in the pinned Torch build and distinguishes architecture failures from FX symbolic-trace errors. No functional code changed.

Changes

Compile fallback documentation

Layer / File(s) Summary
Align compile failure documentation
backend/services/engine_env.py, backend/services/model_manager.py, tests/test_compile_fallback.py
Documentation now states that sm_120 works with the pinned Torch 2.8.0+cu128 build. It identifies FX symbolic tracing as an architecture-independent compile-stack error.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other

Suggested reviewers: debpalash

Merge Risk: ⚪ Minimal · up to df18a

This PR updates compile-fallback documentation and comments without changing runtime behavior or test logic, so it is mergeable.

🚥 Pre-merge checks | ✅ 8 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title uses Conventional Commit syntax but omits the required scope. It includes the issue reference in the description, and its subject matches the documentation changes. Add a scope, such as docs(compile-fallback): correct the Blackwell/Triton attribution behind the #278 compile fallback. Keep the issue reference in the title or body.
✅ Passed checks (8 passed)
Check name Status Explanation
Description check ✅ Passed The description is detailed and covers the change, evidence, testing, scope, and issue references. It does not use the template headings or complete the Type and Checklist sections, but the missing in…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cross-Platform Default Parity ✅ Passed The PR does not change default-mode behavior. The scoped diff changes only documentation strings in three Python files, and executable AST comparison is unchanged after removing docstrings; therefore …
I18n Completeness (21 Locales) ✅ Passed The pull request changes only two backend files and one test file. The authoritative diff contains no frontend changes, no new or changed t('...') keys, and no frontend user-facing strings that bypass…
Local-First Guarantee ✅ Passed PASS — The review-scoped diff changes only comments and docstrings in three Python files. The executable AST is unchanged for all three files, and the added lines introduce no network endpoints, crede…
Backward Compatibility ✅ Passed PASS — The authoritative PR diff changes only docstrings in two backend files and one test module. No database schema, Alembic migration, voice/project/settings data, engine installation, or model-wei…
  • Fix all pre-merge checks with AI

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Retrigger

The PR should not merge until the documented architecture-mismatch failure path is aligned with the actual pre-compile gate.

Fix All in Claude CodeFindings

  1. P1 Impossible Architecture Failure Path

Summary

  • The comments correctly separate the device-independent FX symbolic-tracing error from architecture compatibility, but the replacement explanation still attributes a first-generation cudagraph failure to an architecture mismatch that the existing pre-compile gate rejects.

Reviews (1) · Last reviewed commit: "docs: correct the Blackwell/Triton attri..."

Comment on lines +3 to 6
On a GPU architecture missing from the running torch build's arch list,
`torch.compile` succeeds at load time but the *first generation* dies inside
the Dynamo/FX/Inductor stack (an AssertionError out of
torch/_inductor/cudagraph_trees.py) and was mislabeled as an OOM.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Impossible Architecture Failure Path

The revised narrative says a missing architecture reaches first generation and triggers the cudagraph failure, but should_torch_compile() rejects that mismatch before torch.compile() is applied, so this cannot occur during normal execution. Describe the assertion as an independent compile-stack failure, or explicitly limit this scenario to configurations that force compilation past the architecture gate.

Knowledge Base Used:

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant