[PT2][Optimus] Fix a corner case in merge splits #141194
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/141194
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ❌ 2 New Failures, 5 Unrelated FailuresAs of commit c36b50e with merge base 0155a11 ( NEW FAILURES - The following jobs have failed:
BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
This pull request was exported from Phabricator. Differential Revision: D66275387 |
mengluy0125
force-pushed
the
export-D66275387
branch
from
8cf6388 to
f390611
Compare
|
This pull request was exported from Phabricator. Differential Revision: D66275387 |
mengluy0125
force-pushed
the
export-D66275387
branch
from
f390611 to
a1dcfc5
Compare
|
This pull request was exported from Phabricator. Differential Revision: D66275387 |
mengluy0125
force-pushed
the
export-D66275387
branch
from
a1dcfc5 to
1aa4e67
Compare
|
This pull request was exported from Phabricator. Differential Revision: D66275387 |
Summary: The merge splits has a strong assumption that the next split node will always use all the split getitems, which are not always true and we have seen many such cases. Test Plan: # local reproduce ``` buck2 run mode/opt //scripts/jackiexu0313/pt2:local_model_with_pt2 -- --test_mode batch-split --flow_id 666002198 2>&1 | tee ~/cmf.txt ``` P1683429791 # E2E before fix f666002198 after fix f666722020 Differential Revision: D66275387
mengluy0125
force-pushed
the
export-D66275387
branch
from
1aa4e67 to
c36b50e
Compare
|
This pull request was exported from Phabricator. Differential Revision: D66275387 |
|
@pytorchbot merge -i |
|
@pytorchbot merge -f |
|
❌ 🤖 pytorchbot command failed: Try |
Merge failedReason: This PR has internal changes and must be landed via Phabricator! Please try reimporting/rexporting the PR! Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot merge (Initiating merge automatically since Phabricator Diff has merged) |
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 |
Merge failedReason: 2 jobs have failed, first few of them are: inductor-rocm / rocm6.2-py3.10-inductor / test (inductor, 1, 2, linux.rocm.gpu.2), inductor-rocm / rocm6.2-py3.10-inductor / test (inductor, 2, 2, linux.rocm.gpu.2) Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot merge -i |
Summary:
We find another corner case in the merge splits, where the first split node does not have consecutive getitem indices, we need to skip such cases.
{F1964255863}
Test Plan:
# local reproduce
```
buck2 run mode/opt //scripts/jackiexu0313/pt2:local_model_with_pt2 -- --test_mode batch-split --flow_id 666002198 2>&1 | tee ~/cmf.txt
```
P1683429791
Differential Revision: D66275387
Pull Request resolved: pytorch#141194
Approved by: https://github.com/jackiexu1992
5 participants
Summary:
We find another corner case in the merge splits, where the first split node does not have consecutive getitem indices, we need to skip such cases.
{F1964255863}
Test Plan:
local reproduce
P1683429791
Differential Revision: D66275387
cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @ColinPeppler @amjames @desertfire @chauhang @aakhundov