Repository navigation
[AutoSP] Honor seq_dim when AutoSP shards inputs - #8720
Merged
Merged
Conversation
prepare_autosp_inputs() accepted seq_dim but only used it for mark_dynamic: the graph passes always read the sequence length from shape[1] and sliced dimension 1. Sequence-first [seq, batch] inputs therefore sharded the batch dimension, or failed because that dimension is static. Dynamo copies only the `tag` attribute into node metadata, so record seq_dim in the tag as (key, seq_dim). Tag lookup accepts both forms, and a bare key keeps the previous default of dimension 1. The sequence length and the slice now use the recorded dimension, and label_id is bounds-checked like the other prepared inputs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Jin, Youzhi <youzhi.jin@intel.com>
Contributor
Author
|
Could you take a look at this PR when you have a chance? Thanks @pengdurice |
tohtana
approved these changes
Oct 11, 2026
tohtana
left a comment
Collaborator
There was a problem hiding this comment.
Thank you, @jinyouzhi! Looks good to me.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split from #8457.
Problem
prepare_autosp_inputs()takes aseq_dimargument, but only passes it tomark_dynamic. The AutoSP graph passes ignore it:pass_shard_seq_dimandshard_tensor_nodealways read the sequence length fromshape[1]and slice dimension 1.Sequence-first
[seq, batch]inputs (seq_dim=0) are therefore mishandled:expected sequence dimension to be of type SymInt but found int.Fix
Dynamo copies only the
tagattribute of an input tensor into graph metadata (_extract_tensor_dict), soseq_dimhas to travel in the tag:prepare_autosp_inputs()tags inputs as(key, seq_dim).find_node_by_tag()accepts both the tuple and a bare key. A bare key, as set manually in existing code and tests, keeps the previous default of dimension 1.get_autosp_seq_dim()reads the recorded dimension.pass_shard_seq_dimandshard_tensor_nodeuse it to find the sequence length, andshard_tensor_nodebuilds its slice for the tensor's full rank.label_idis now bounds-checked againstseq_dim, like the other prepared inputs, since it is also sharded along that dimension.seq_dimapplies to every prepared tensor, and thatseq_dim=0is for sequence-first inputs.Tests
TestAutoSPSeqDim(2 ranks,sp_size=2) runs a small causal SDPA model with the sequence onseq_dim∈ {0, 1}. For each of three batches, the mean of the per-rank AutoSP losses must match a single-device run on the full sequence (rtol=1e-4). Causal attention makes the loss depend on which dimension is sharded, so slicing the batch instead of the sequence would not match.seq_dim=0fails on master and passes with this PR.seq_dim=1passes on both, which covers the default path with the new tuple tags.The test compares forward losses only. AutoSP currently sums gradients over the SP group, while each rank's loss is a local mean, so gradients are
sp_size× a full-sequence reference. That is part of the causal-loss aggregation follow-up from #8457, not this change.Results on 4× NVIDIA RTX 5090 D, torch 2.13.0, transformers 5.17.0:
tests/unit/v1/compile/test_compile_autosp.py -k "not TestAutoSPCompile": 38 passed.TestAutoSPCompilefails identically on unmodified master in this environment. The error is a DynamoInternalTorchDynamoErrorin the Ulysses baseline'storch.compileforward, before AutoSP runs, so I excluded it.pre-commitpasses on the changed files.