Skip to content

[GraphTrainer] Support transformer_block_bucketing in aot_fx_trace mode#2934

Merged
yiming0416 merged 1 commit into
mainfrom
graph_trainer/transformer-block-bucketing-aot-fx-trace
Apr 20, 2026
Merged

[GraphTrainer] Support transformer_block_bucketing in aot_fx_trace mode#2934
yiming0416 merged 1 commit into
mainfrom
graph_trainer/transformer-block-bucketing-aot-fx-trace

Conversation

@yiming0416

@yiming0416 yiming0416 commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Enable transformer_block_bucketing pass in aot_fx_trace compile mode. Previously this only worked in aot and jit modes because aot_fx_trace traces a joint fwd+bwd graph where nn_module_stack metadata alone isn't sufficient to correctly identify and separate forward vs backward nodes for bucketing.

Root cause: In the joint graph produced by aot_fx_trace, forward and backward collectives coexist. The existing manual_overlap_bucketing pass needs to bucket each direction independently — forward collectives in forward execution order, backward collectives in reverse order — but had no way to distinguish them.

Solution: Leverage the existing annotate_fn mechanism (already used for AC region tagging) to:

  1. Tag each bucket-level submodule's forward with its FQN (annotate_module_fqns)
  2. Mark backward nodes with is_bwd=True during metadata propagation
  3. Provide a direction-aware module_stack_fn to manual_overlap_bucketing that filters by fwd/bwd

A new joint_transformer_block_bucketing_reordering_pass applies bucketing in two passes — once for forward nodes, once for backward — so each direction's collectives are bucketed and reordered independently.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Apr 10, 2026
@yiming0416 yiming0416 force-pushed the graph_trainer/transformer-block-bucketing-aot-fx-trace branch 4 times, most recently from e7a8559 to 3d07026 Compare April 10, 2026 22:42
Comment thread torchtitan/experiments/graph_trainer/make_fx_tracer.py Outdated
Comment thread torchtitan/experiments/graph_trainer/common_utils.py
@yiming0416 yiming0416 force-pushed the graph_trainer/transformer-block-bucketing-aot-fx-trace branch 2 times, most recently from 63e130f to 89b5762 Compare April 13, 2026 21:16
@yiming0416 yiming0416 changed the title [GraphTrainer][AutoDev] Support transformer_block_bucketing in aot_fx_trace mode [GraphTrainer] Support transformer_block_bucketing in aot_fx_trace mode Apr 13, 2026
@yiming0416 yiming0416 force-pushed the graph_trainer/transformer-block-bucketing-aot-fx-trace branch 7 times, most recently from a95cd87 to 83d06fc Compare April 14, 2026 22:43
@yiming0416 yiming0416 marked this pull request as ready for review April 15, 2026 22:52
Comment thread torchtitan/experiments/graph_trainer/deepseek_v3/parallelize.py
Comment thread torchtitan/experiments/graph_trainer/passes.py Outdated
Comment on lines +53 to +54
traced_result: "TracedResult",
model: torch.nn.Module,

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.

Keep the interface clean if possible.

  1. trace_result has some internal GraphModule, can we use that?
  2. if not, alternative is to cache an instance of model inside TracedResult

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I attached an instance of model to TracedResult.

We can use the internal GraphModule from TracedResult as they has been flattened and lost the module hierarchy info

Comment thread torchtitan/experiments/graph_trainer/passes.py Outdated
@yiming0416 yiming0416 force-pushed the graph_trainer/transformer-block-bucketing-aot-fx-trace branch 2 times, most recently from 991e351 to 1daa927 Compare April 16, 2026 00:38
@yiming0416 yiming0416 force-pushed the graph_trainer/transformer-block-bucketing-aot-fx-trace branch 2 times, most recently from 3362a5e to 68737e0 Compare April 16, 2026 17:34
tugsbayasgalan added a commit that referenced this pull request Apr 16, 2026
…ackward subgraphs"


Graph-trainer was only copying seq_nr metadata within the outer FX graph. That misses the way old AOT behaves for flex attention: the matching forward source for a backward flex node can live inside a nested GraphModule, including placeholder nodes from the captured flex subgraphs.

In practice that meant backward flex nodes in the graph-trainer path could miss the forward custom / nn_module_stack metadata that old AOT used to recover. The old outer-graph heuristic was also unreliable once remat-tagged nodes showed up in the same seq_nr bucket, because the first outer node with that seq_nr was no longer a reliable proxy for the true forward source.

Fix _copy_fwd_metadata_to_bw_nodes() to mirror the old AOT behavior more closely:
- walk all nested GraphModules
- build the seq_nr -> forward-node map across those subgraphs
- only copy onto nodes identified as backward via partitioner_tag or remat metadata

1) This should address failures in #2934 

[ghstack-poisoned]
tugsbayasgalan added a commit that referenced this pull request Apr 16, 2026
Graph-trainer was only copying seq_nr metadata within the outer FX graph. That misses the way old AOT behaves for flex attention: the matching forward source for a backward flex node can live inside a nested GraphModule, including placeholder nodes from the captured flex subgraphs.

In practice that meant backward flex nodes in the graph-trainer path could miss the forward custom / nn_module_stack metadata that old AOT used to recover. The old outer-graph heuristic was also unreliable once remat-tagged nodes showed up in the same seq_nr bucket, because the first outer node with that seq_nr was no longer a reliable proxy for the true forward source.

Fix _copy_fwd_metadata_to_bw_nodes() to mirror the old AOT behavior more closely:
- walk all nested GraphModules
- build the seq_nr -> forward-node map across those subgraphs
- only copy onto nodes identified as backward via partitioner_tag or remat metadata

1) This should address failures in #2934 

[ghstack-poisoned]
pytorchmergebot pushed a commit to pytorch/pytorch that referenced this pull request Apr 16, 2026
…nstance (#180579)

Problem:
When `manual_overlap_bucketing` is called multiple times on the same GraphModule (e.g. separate forward and backward passes), the old implementation stored bucketed node types in node.meta, which persists on the graph nodes across calls. The second call's _manual_reorder_graph iterates all graph nodes and misclassifies nodes bucketed by the first call, building incorrect overlap dependency edges.

Fix:
Moved bucketed node type tracking from `node.meta["manual_bucket_node_type"]` to `ManualOverlapPreservingBucketer.bucketed_node_types` dict, preventing metadata leaking across multiple `manual_overlap_bucketing` calls on the same graph.

Test:
In pytorch/torchtitan#2934
```
MODULE=graph_trainer.llama3 CONFIG=graph_trainer_llama3_debugmodel ./run_train.sh --compile.mode aot_fx_trace
```

Pull Request resolved: #180579
Approved by: https://github.com/SherlockNoMad
tugsbayasgalan added a commit that referenced this pull request Apr 16, 2026
Stack from [ghstack](https://github.com/ezyang/ghstack) (oldest at
bottom):
* __->__ #2875

Graph-trainer was only copying seq_nr metadata within the outer FX
graph. That misses the way old AOT behaves for flex attention: the
matching forward source for a backward flex node can live inside a
nested GraphModule, including placeholder nodes from the captured flex
subgraphs.

In practice that meant backward flex nodes in the graph-trainer path
could miss the forward custom / nn_module_stack metadata that old AOT
used to recover. The old outer-graph heuristic was also unreliable once
remat-tagged nodes showed up in the same seq_nr bucket, because the
first outer node with that seq_nr was no longer a reliable proxy for the
true forward source.

Fix _copy_fwd_metadata_to_bw_nodes() to mirror the old AOT behavior more
closely:
- walk all nested GraphModules
- build the seq_nr -> forward-node map across those subgraphs
- only copy onto nodes identified as backward via partitioner_tag or
remat metadata

1) This should address failures in
#2934
@yiming0416 yiming0416 force-pushed the graph_trainer/transformer-block-bucketing-aot-fx-trace branch 3 times, most recently from 236184d to 5ad46c8 Compare April 20, 2026 17:06
output_spec: Original output pytree spec used during reconstruction.
model: The original nn.Module, cached for downstream passes that need
module structure (e.g. transformer block bucketing). Only set when
traced via :func:`trace_train_step`.

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.

Only set when traced via :func:`trace_train_step`.

I am not a fan of conditional behavior.

Also TracedResult is now becoming a contract between Trainer and pre-compile, so adding model is not feasible now.

So this is pointing towards something more fundamental, we probably need a different way to come up with bucketing than get_transformer_block_buckets(traced_result.model).

Let's land for now, but add to TODO here to remove the model field from Traced Result.

@yiming0416 yiming0416 force-pushed the graph_trainer/transformer-block-bucketing-aot-fx-trace branch from 5ad46c8 to 6e0b03c Compare April 20, 2026 18:11
aot_fx_trace mode traces with record_module_stack=False, so nodes lack
nn_module_stack metadata that manual_overlap_bucketing uses to match
nodes to modules. This change uses the existing annotate_fn mechanism
(already used for AC region tagging) to tag module FQNs during tracing,
then provides a custom module_stack_fn to manual_overlap_bucketing.

Changes:
- Add annotate_module_fqns() and get_module_stack_from_annotation() to
  common_utils.py
- Call annotate_module_fqns() in both llama3 and deepseek_v3 annotate
  functions
- Add module_stack_fn parameter to
  transformer_block_bucketing_reordering_pass
- Update construct_default_graph_passes to accept compile_config, model,
  and parallel_dims, and wire up the bucketing pass with reassign_to_pg
- Pass config/model/parallel_dims from GraphTrainer to
  construct_default_graph_passes
- Add numerics tests for both llama3 and deepseek_v3 with aot_fx_trace
  + transformer_block_bucketing
@yiming0416 yiming0416 force-pushed the graph_trainer/transformer-block-bucketing-aot-fx-trace branch from 6e0b03c to b83b975 Compare April 20, 2026 18:36
@yiming0416 yiming0416 merged commit 000c178 into main Apr 20, 2026
11 of 15 checks passed
bobrenjc93 added a commit that referenced this pull request Apr 21, 2026
Stack from [ghstack](https://github.com/ezyang/ghstack/tree/0.14.0)
(oldest at bottom):
* #2916
* __->__ #3043

Precompile integration tests (run_precompile_tests.py) were never run
in CI, which allowed the bucketing + CooR incompatibility from #2934
(enabled in #3036)
to go undetected. Add them to both CI workflows:

- A10 workflow: Llama3 aot_fx_trace precompile test (FSDP+TP)
- H100 workflow: DSv3 aot_fx_trace precompile test (FSDP+TP+EP)
bobrenjc93 added a commit that referenced this pull request Apr 22, 2026
Precompile integration tests (run_precompile_tests.py) were never run
in CI, which allowed the bucketing + CooR incompatibility from #2934
to go undetected. Add them to both CI workflows:

- A10 workflow: Llama3 aot_fx_trace precompile test (FSDP+TP)
- H100 workflow: DSv3 aot_fx_trace precompile test (FSDP+TP+EP)

Also fix the DSv3 precompile test config from debugmodel to
debugmodel_ep — EP tests require AllToAllTokenDispatcher which is
only created when moe_comm_backend="standard".


ghstack-source-id: 35a8421
Pull-Request: #3043
aperson30 pushed a commit to aperson30/pytorch that referenced this pull request Jun 20, 2026
…nstance (pytorch#180579)

Problem:
When `manual_overlap_bucketing` is called multiple times on the same GraphModule (e.g. separate forward and backward passes), the old implementation stored bucketed node types in node.meta, which persists on the graph nodes across calls. The second call's _manual_reorder_graph iterates all graph nodes and misclassifies nodes bucketed by the first call, building incorrect overlap dependency edges.

Fix:
Moved bucketed node type tracking from `node.meta["manual_bucket_node_type"]` to `ManualOverlapPreservingBucketer.bucketed_node_types` dict, preventing metadata leaking across multiple `manual_overlap_bucketing` calls on the same graph.

Test:
In pytorch/torchtitan#2934
```
MODULE=graph_trainer.llama3 CONFIG=graph_trainer_llama3_debugmodel ./run_train.sh --compile.mode aot_fx_trace
```

Pull Request resolved: pytorch#180579
Approved by: https://github.com/SherlockNoMad
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/8gpu CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants