Try to simplify FloorDiv axioms implications when needed during evaluations. #141267
Conversation
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
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/141267
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (4 Unrelated Failures)As of commit f3b78c1 with merge base 995e307 ( FLAKY - The following jobs failed but were likely due to flakiness present on trunk:
UNSTABLE - The following jobs failed but were 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. |
|
This pull request was exported from Phabricator. Differential Revision: D66307433 |
laithsakka
force-pushed
the
export-D66307433
branch
from
73bc344 to
063c514
Compare
|
This pull request was exported from Phabricator. Differential Revision: D66307433 |
laithsakka
force-pushed
the
export-D66307433
branch
from
063c514 to
e587f3c
Compare
|
This pull request was exported from Phabricator. Differential Revision: D66307433 |
laithsakka
force-pushed
the
export-D66307433
branch
from
e587f3c to
017db3f
Compare
|
This pull request was exported from Phabricator. Differential Revision: D66307433 |
laithsakka
commented
Nov 21, 2024
laithsakka
force-pushed
the
export-D66307433
branch
from
017db3f to
0410ea6
Compare
|
This pull request was exported from Phabricator. Differential Revision: D66307433 |
laithsakka
force-pushed
the
export-D66307433
branch
from
0410ea6 to
4dfffa5
Compare
|
This pull request was exported from Phabricator. Differential Revision: D66307433 |
laithsakka
force-pushed
the
export-D66307433
branch
from
4dfffa5 to
68803fb
Compare
|
This pull request was exported from Phabricator. Differential Revision: D66307433 |
|
removing k.free_symbols.issubset(expr.free_symbols) remove the regression on torch rec. update the PR |
bobrenjc93
reviewed
Nov 21, 2024
bobrenjc93
reviewed
Nov 21, 2024
laithsakka
force-pushed
the
export-D66307433
branch
from
931e969 to
da0d573
Compare
|
This pull request was exported from Phabricator. Differential Revision: D66307433 |
laithsakka
force-pushed
the
export-D66307433
branch
from
da0d573 to
fd8829c
Compare
|
This pull request was exported from Phabricator. Differential Revision: D66307433 |
laithsakka
added a commit
to laithsakka/pytorch
that referenced
this pull request
Nov 23, 2024
…is changed. (pytorch#141267) Summary: This very much the same solution proposed by bobrenjc93 except that it restrict it to expressions and axioms that have FloorDiv, since those are the only ones that could have became CleanDiv. and the only one that can changes as shape env changes. This also does not break torchrec benchmarks, it might be worth it to know why the generalization of this does break the torchrec benchmarks, but we could just be hitting another bug or NYI situation. ovearhead? None on ``` buck2 run fbcode//mode/opt fbcode//torchrec/distributed/tests:pt2_compile_benchmark -- --num-features=2000 ``` Differential Revision: D66307433 cc ezyang SherlockNoMad EikanWang jgong5 wenzhe-nrv D66307433 Test Plan: run existing tests. ``` TORCH_LOGS="+dynamo" TORCHDYNAMO_VERBOSE=1 buck2 run mode/opt //minimal_viable_ai/fire:light -- -d ~/fbsource/fbcode/hpc/ -d ~/fbsource/fbcode/hammer -d ~/fbsource/fbcode/minimal_viable_ai/ minimal_viable_ai/new_format/preproc_ta/experimental/pt2_export_train_preproc_roo_for_umia.py --start-ts 2024-11-10+10:00:00 --end-ts 2024-11-10+11:00:00 --steps 1 --preproc-type TA_accel_cpu_eager --export-archive-path /home/lsakka ``` ``` buck2 run fbcode//mode/opt fbcode//torchrec/distributed/tests:pt2_compile_benchmark -- --num-features=10 ``` Reviewed By: ezyang
laithsakka
force-pushed
the
export-D66307433
branch
from
fd8829c to
3bc41fc
Compare
|
This pull request was exported from Phabricator. Differential Revision: D66307433 |
laithsakka
force-pushed
the
export-D66307433
branch
from
3bc41fc to
00908d9
Compare
laithsakka
force-pushed
the
export-D66307433
branch
from
00908d9 to
176949b
Compare
|
This pull request was exported from Phabricator. Differential Revision: D66307433 |
1 similar comment
|
This pull request was exported from Phabricator. Differential Revision: D66307433 |
laithsakka
force-pushed
the
export-D66307433
branch
from
176949b to
b37ba44
Compare
|
This pull request was exported from Phabricator. Differential Revision: D66307433 |
…is changed. (pytorch#141267) Summary: This very much the same solution proposed by bobrenjc93 except that it restrict it to expressions and axioms that have FloorDiv, since those are the only ones that could have became CleanDiv. and the only one that can changes as shape env changes. This also does not break torchrec benchmarks, it might be worth it to know why the generalization of this does break the torchrec benchmarks, but we could just be hitting another bug or NYI situation. ovearhead? None on ``` buck2 run fbcode//mode/opt fbcode//torchrec/distributed/tests:pt2_compile_benchmark -- --num-features=2000 ``` Differential Revision: D66307433 cc ezyang SherlockNoMad EikanWang jgong5 wenzhe-nrv D66307433 Test Plan: run existing tests. ``` TORCH_LOGS="+dynamo" TORCHDYNAMO_VERBOSE=1 buck2 run mode/opt //minimal_viable_ai/fire:light -- -d ~/fbsource/fbcode/hpc/ -d ~/fbsource/fbcode/hammer -d ~/fbsource/fbcode/minimal_viable_ai/ minimal_viable_ai/new_format/preproc_ta/experimental/pt2_export_train_preproc_roo_for_umia.py --start-ts 2024-11-10+10:00:00 --end-ts 2024-11-10+11:00:00 --steps 1 --preproc-type TA_accel_cpu_eager --export-archive-path /home/lsakka ``` ``` buck2 run fbcode//mode/opt fbcode//torchrec/distributed/tests:pt2_compile_benchmark -- --num-features=10 ``` Reviewed By: ezyang
laithsakka
force-pushed
the
export-D66307433
branch
from
b37ba44 to
f3b78c1
Compare
|
This pull request was exported from Phabricator. Differential Revision: D66307433 |
|
@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 |
pobin6
pushed a commit
to pobin6/pytorch
that referenced
this pull request
Dec 5, 2024
…ations. (pytorch#141267) Summary: This very much the same solution proposed by bobrenjc93 except that it restrict it to expressions and axioms that have FloorDiv, since those are the only ones that could have became CleanDiv. and the only one that can changes as shape env changes. This also does not break torchrec benchmarks, it might be worth it to know why the generalization of this does break the torchrec benchmarks, but we could just be hitting another bug or NYI situation. ovearhead? None on ``` buck2 run fbcode//mode/opt fbcode//torchrec/distributed/tests:pt2_compile_benchmark -- --num-features=1000 ``` Differential Revision: D66307433 Pull Request resolved: pytorch#141267 Approved by: https://github.com/ezyang
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Summary:
This very much the same solution proposed by bobrenjc93 except that it restrict it to expressions and axioms that have FloorDiv, since those are the only ones that could have became CleanDiv. and the only one that can changes as shape env changes.
This also does not break torchrec benchmarks, it might be worth it to know why the generalization of this does break the torchrec benchmarks, but we could just be hitting another bug or NYI situation.
ovearhead?
None on
Differential Revision: D66307433
cc @ezyang @SherlockNoMad @EikanWang @jgong5 @wenzhe-nrv @voznesenskym @penguinwu @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @jiayisunx @chenyang78 @kadeng @chauhang @amjames