[tp] fix torch compile regression #111521
Conversation
The most recent refactor of TP #111160 breaks torch compile path, so reverting the behavior back by: 1. use the old default prepare_input/output 2. add the colwise/rowwise parallel test instead [ghstack-poisoned]
wanchaol
requested review from
H-Huang,
awgu,
d4l3k,
fduwjj,
fegin,
kiukchung,
kwen2501,
mrshenli,
rohan-varma,
wz337 and
zhaojuanmao
as code owners
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/111521
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (3 Unrelated Failures)As of commit 7f29dec with merge base 5c39552 ( FLAKY - The following jobs failed but were likely due to flakiness present on trunk:
UNSTABLE - The following job failed but was likely due to flakiness present on trunk and has been marked as unstable:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
| _prepare_input=None, | ||
| _prepare_output=None, |
There was a problem hiding this comment.
But this somehow revert the input_layouts and output_layouts? So does this mean when users want to use input_layouts and output_layouts, they have to set _prepare_input to None?
There was a problem hiding this comment.
This is just a quick fix, I'll do a big rewrite on top to make sure user behavior won't change
There was a problem hiding this comment.
OK I gave up on the big rewrite and found a quicker solution right now, however it does not fundamentally fix the torch.compile problem in the new style input_layouts and output_layouts path (i.e. to configure sequence parallel torch.compile still fail). We should do a full rewrite soon to make the TP code cleaner and tracing friendly
There was a problem hiding this comment.
this "hack" preserves the intended behavior, but I dislike it..
The most recent refactor of TP #111160 breaks torch compile path, so reverting the behavior back by: 1. use the old default prepare_input/output 2. add the colwise/rowwise parallel test instead [ghstack-poisoned]
wanchaol
added
ciflow/trunk
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
wanchaol
added
the
ciflow/periodic
Merge failedReason: 1 mandatory check(s) failed. The first few are: Dig deeper by viewing the failures on hud |
The most recent refactor of TP #111160 breaks torch compile path, so reverting the behavior back by: 1. use the old default prepare_input/output 2. add the colwise/rowwise parallel test instead [ghstack-poisoned]
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
The most recent refactor of TP pytorch#111160 breaks torch compile path, so reverting the behavior back by: 1. use the old default prepare_input/output 2. add the colwise/rowwise parallel test instead Pull Request resolved: pytorch#111521 Approved by: https://github.com/fduwjj
3 participants
Stack from ghstack (oldest at bottom):
The most recent refactor of TP
#111160 breaks torch compile
path, so reverting the behavior back by: