Skip to content

Rely on Recipe object for fusion - #3717

Merged
vthumbe1503 merged 10 commits into
NVIDIA:mainfrom
vthumbe1503:recipe_hash
Oct 27, 2026
Merged

vthumbe1503 merged 10 commits into
NVIDIA:mainfrom
vthumbe1503:recipe_hash

Conversation

@vthumbe1503

@vthumbe1503 vthumbe1503 commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Use the recipe's string representation to select an operation-fusion plan. The previous cache key used the recipe type, backward boundary, and backward override, so a change to another setting on the same recipe type could reuse a plan chosen for the old settings. The new key uses str(recipe) and the backward boundary. Recipe classes describe their own settings; OperationFuser does not need a growing list of MXFP8, NVFP4, or grouped-MLP recipe options to decide when to re-evaluate fusion.

Recipe representations are cached and invalidated when attributes change. Unexpected public attributes are included too, so changing the dynamically attached enable_cutedsl_fused_grouped_mlp flag changes the fusion key. The NVFP4 representation now also includes disable_rht.

This follows the motivation in the original discussion.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Code refactoring

Changes

  • Key the fusion-plan cache by str(recipe) and the first operation requiring backward. Keep fusion-plan selection separate from the existing recipe-state reset conditions.
  • Cache the module and op-fuser grouped-tensor support checks using the recipe settings each check reads, dtype, split alignment where applicable, and CUDA device. The internal call sites pass the tensor's device, preventing a result from one GPU from being reused for another.
  • Cover recipe-representation changes and dynamically attached attributes in the fusion tests.

CPU overhead of grouped-tensor support checks

BF16 with no recipe on a dlcluster GB200, using cuda:0 explicitly. Values are median CPU time per call over nine runs of 200,000 calls; the cached path was warm. The uncached functions came from this PR's base commit (491d557), and the device-key code is in 793c5ac.

Predicate Before PR (uncached) After PR (cached)
Module 1.668 µs 0.267 µs
Op-fuser 1.549 µs 0.253 µs

These numbers use the explicit-device path taken by the internal callers. Omitting device requires querying the current CUDA device on each call and has different CPU cost. No GPU kernel changes are included.

Scope of recipe updates

The fusion-plan cache and quantizer-state reset are separate. This PR changes plan selection; it does not rebuild quantizers when a same-type recipe update changes their configuration. Dynamic recipe update handling is being addressed in PR #3343.

Checklist

  • I have read and followed the contributing guidelines
  • The functionality is complete
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

…the grouped tensor path

Signed-off-by: Varun Thumbe <vthumbe@nvidia.com>
@greptile-apps

greptile-apps Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new blocking issue was found.

Summary

The PR selects fusion plans using the recipe string and the backward boundary.

  • Fusion plans follow the recipe settings and backward boundary.
  • Grouped-tensor routing reuses support checks for matching inputs and GPUs.

No new actionable issues were found. The previous GPU-cache, grouped-MLP flag, and repeated-settings findings are addressed. vthumbe1503 explained that refreshing quantizers after same-type recipe changes predates this PR and is deferred to PR #3343; Greptile withdrew that finding.

Review checks were based on source inspection; tests were not run.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Recipe and backward boundary] --> B[Check existing recipe-state reset conditions]
    B --> C[Build key from recipe string and backward boundary]
    C --> D{Cached plan?}
    D -->|Yes| E[Reuse forward and backward plans]
    D -->|No| F[Apply fusion rules]
    F --> G[Cache forward and backward plans]
Loading

Reviews (9) · Last reviewed commit: "Merge branch 'main' into recipe_hash" · Reviewed by Greptile

Comment thread transformer_engine/common/recipe/__init__.py Outdated
Comment thread transformer_engine/pytorch/module/grouped_linear.py Outdated
Comment thread transformer_engine/pytorch/ops/fuser.py Outdated
Comment thread transformer_engine/pytorch/ops/basic/grouped_linear.py Outdated
Comment thread transformer_engine/common/recipe/__init__.py Outdated
Comment thread transformer_engine/pytorch/ops/fuser.py Outdated
Comment thread transformer_engine/common/recipe/__init__.py Outdated
Comment thread tests/pytorch/test_recipe.py Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This might conflict with #3343.

@vthumbe1503 vthumbe1503 Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Do you mean merge conflict? or design conflict? If it is the latter could you explain how?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That PR adds infrastructure for the recipe to create nested tuples that describe its config, which are easier to compare than the recipes themselves. This PR uses the string representation to do something similar. I like this PR's approach more.

@vthumbe1503 vthumbe1503 changed the title Rely on Recipe object on fusion Rely on Recipe object for fusion Oct 20, 2026
Signed-off-by: Varun Thumbe <vthumbe@nvidia.com>
Comment thread transformer_engine/pytorch/ops/fuser.py Outdated
Signed-off-by: Varun Thumbe <vthumbe@nvidia.com>
@vthumbe1503

Copy link
Copy Markdown
Collaborator Author

/te-ci pytorch

Comment thread transformer_engine/common/recipe/__init__.py
Signed-off-by: Varun Thumbe <vthumbe@nvidia.com>
timmoon10
timmoon10 previously approved these changes Oct 27, 2026

@timmoon10 timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall LGTM, with some minor suggestions.

Comment thread transformer_engine/pytorch/module/grouped_linear.py Outdated
Comment thread transformer_engine/pytorch/ops/basic/grouped_linear.py Outdated
Comment thread transformer_engine/pytorch/ops/fuser.py Outdated
Co-authored-by: Tim Moon <4406448+timmoon10@users.noreply.github.com>
Signed-off-by: vthumbe1503 <vthumbe@nvidia.com>
vthumbe1503 and others added 3 commits October 27, 2026 12:03
Co-authored-by: Tim Moon <4406448+timmoon10@users.noreply.github.com>
Signed-off-by: vthumbe1503 <vthumbe@nvidia.com>
Co-authored-by: Tim Moon <4406448+timmoon10@users.noreply.github.com>
Signed-off-by: vthumbe1503 <vthumbe@nvidia.com>
@vthumbe1503

Copy link
Copy Markdown
Collaborator Author

/te-ci pytorch

@vthumbe1503

Copy link
Copy Markdown
Collaborator Author

Pipeline 77096236

@vthumbe1503
vthumbe1503 merged commit 9b7e959 into NVIDIA:main Oct 27, 2026
12 of 14 checks passed
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.

2 participants