[export] kill _process_constraints() #123985
Conversation
pianpwk
requested review from
avikchaudhuri,
gmagogsfm,
tugsbayasgalan and
zhxchen17
as code owners
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/123985
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 339f3fa with merge base 6822507 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
This pull request was exported from Phabricator. Differential Revision: D56086223 |
|
This pull request was exported from Phabricator. Differential Revision: D56086223 |
pianpwk
force-pushed
the
export-D56086223
branch
from
ca04c69 to
20fbd4e
Compare
|
This pull request was exported from Phabricator. Differential Revision: D56086223 |
pianpwk
force-pushed
the
export-D56086223
branch
from
20fbd4e to
9691273
Compare
Summary: Pull Request resolved: #123985 Differential Revision: D56086223
facebook-github-bot
force-pushed
the
export-D56086223
branch
from
9691273 to
5676d99
Compare
Summary: Pull Request resolved: #123985 Differential Revision: D56086223
|
This pull request was exported from Phabricator. Differential Revision: D56086223 |
facebook-github-bot
force-pushed
the
export-D56086223
branch
from
5676d99 to
5e73fef
Compare
|
This pull request was exported from Phabricator. Differential Revision: D56086223 |
pianpwk
force-pushed
the
export-D56086223
branch
from
5e73fef to
90a21da
Compare
|
This pull request was exported from Phabricator. Differential Revision: D56086223 |
Summary: The process for populating range_constraints follows separate methods for non-strict (`make_constraints`), and strict (`_process_constraints`). The strict method is somewhat more convoluted, and the analysis that Dynamo performs for strict is already present as part of the non-strict process in make_constraints (i.e. produce_guards(), running the export constraint solver). This PR kills _process_constraints() and replaces calls with make_constraints, with an option to skip the work that Dynamo has already done. Test Plan: existing tests, all export calls now go through make_constraints Differential Revision: D56086223
facebook-github-bot
force-pushed
the
export-D56086223
branch
from
90a21da to
5798e58
Compare
|
This pull request was exported from Phabricator. Differential Revision: D56086223 |
pianpwk
force-pushed
the
export-D56086223
branch
from
5798e58 to
02c3fa8
Compare
Summary: Pull Request resolved: #123985 The process for populating range_constraints follows separate methods for non-strict (`make_constraints`), and strict (`_process_constraints`). The strict method is somewhat more convoluted, and the analysis that Dynamo performs for strict is already present as part of the non-strict process in make_constraints (i.e. produce_guards(), running the export constraint solver). This PR kills _process_constraints() and replaces calls with make_constraints, with an option to skip the work that Dynamo has already done. Test Plan: existing tests, all export calls now go through make_constraints Differential Revision: D56086223
pianpwk
force-pushed
the
export-D56086223
branch
from
02c3fa8 to
5475de1
Compare
Summary: Pull Request resolved: #123985 The process for populating range_constraints follows separate methods for non-strict (`make_constraints`), and strict (`_process_constraints`). The strict method is somewhat more convoluted, and the analysis that Dynamo performs for strict is already present as part of the non-strict process in make_constraints (i.e. produce_guards(), running the export constraint solver). This PR kills _process_constraints() and replaces calls with make_constraints, with an option to skip the work that Dynamo has already done. Test Plan: existing tests, all export calls now go through make_constraints Differential Revision: D56086223
torch/_export/non_strict_utils.py
Outdated
| num_lifted_inputs: int, | ||
| equalities_inputs: Optional[EqualityConstraint] = None, | ||
| original_signature: Optional[inspect.Signature] = None, | ||
| skip_produce_guards: Optional[bool] = False, |
There was a problem hiding this comment.
With a bit of renaming and refactoring can we get rid of this flag? Specifically I'm thinking maybe we should split make_constraints into a produce_guards and make_constraints instead? And then make non-strict export call the part inside not skip_produce_guards explicitly (the new produce_guards) before calling make_constraints and have strict just call the latter?
There was a problem hiding this comment.
Ah yes this makes complete sense.
|
@pianpwk has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
pianpwk
force-pushed
the
export-D56086223
branch
from
b55ffbb to
85c2cb5
Compare
|
This pull request was exported from Phabricator. Differential Revision: D56086223 |
facebook-github-bot
force-pushed
the
export-D56086223
branch
from
85c2cb5 to
8f9629f
Compare
|
This pull request was exported from Phabricator. Differential Revision: D56086223 |
Summary: The process for populating range_constraints follows separate methods for non-strict (`make_constraints`), and strict (`_process_constraints`). The strict method is somewhat more convoluted, and the analysis that Dynamo performs for strict is already present as part of the non-strict process in make_constraints (produce_guards(), running the export constraint solver). This PR kills _process_constraints() and replaces calls with make_constraints, without duplicating the work that Dynamo already does. Reviewed By: avikchaudhuri Differential Revision: D56086223 Pulled By: pianpwk
pianpwk
force-pushed
the
export-D56086223
branch
from
8f9629f to
18a2a92
Compare
|
This pull request was exported from Phabricator. Differential Revision: D56086223 |
facebook-github-bot
force-pushed
the
export-D56086223
branch
from
18a2a92 to
ee7f12b
Compare
|
This pull request was exported from Phabricator. Differential Revision: D56086223 |
pianpwk
force-pushed
the
export-D56086223
branch
from
ee7f12b to
8a802a6
Compare
Summary: The process for populating range_constraints follows separate methods for non-strict (`make_constraints`), and strict (`_process_constraints`). The strict method is somewhat more convoluted, and the analysis that Dynamo performs for strict is already present as part of the non-strict process in make_constraints (produce_guards(), running the export constraint solver). This PR kills _process_constraints() and replaces calls with make_constraints, without duplicating the work that Dynamo already does. Reviewed By: avikchaudhuri Differential Revision: D56086223 Pulled By: pianpwk
|
This pull request was exported from Phabricator. Differential Revision: D56086223 |
Summary: The process for populating range_constraints follows separate methods for non-strict (`make_constraints`), and strict (`_process_constraints`). The strict method is somewhat more convoluted, and the analysis that Dynamo performs for strict is already present as part of the non-strict process in make_constraints (produce_guards(), running the export constraint solver). This PR kills _process_constraints() and replaces calls with make_constraints, without duplicating the work that Dynamo already does. Pull Request resolved: #123985 Reviewed By: avikchaudhuri Differential Revision: D56086223 Pulled By: pianpwk
pianpwk
force-pushed
the
export-D56086223
branch
from
8a802a6 to
339f3fa
Compare
pianpwk
changed the title
|
@pytorchbot merge -f 'Landed internally' (Initiating merge automatically since Phabricator Diff has merged, using force because this PR might not pass merge_rules.json but landed internally) |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
The process for populating range_constraints follows separate methods for non-strict (`make_constraints`), and strict (`_process_constraints`). The strict method is somewhat more convoluted, and the analysis that Dynamo performs for strict is already present as part of the non-strict process in make_constraints (produce_guards(), running the export constraint solver). This PR kills _process_constraints() and replaces calls with make_constraints, without duplicating the work that Dynamo already does. Pull Request resolved: #123985 Approved by: https://github.com/avikchaudhuri
4 participants
The process for populating range_constraints follows separate methods for non-strict (
make_constraints), and strict (_process_constraints). The strict method is somewhat more convoluted, and the analysis that Dynamo performs for strict is already present as part of the non-strict process in make_constraints (produce_guards(), running the export constraint solver).This PR kills _process_constraints() and replaces calls with make_constraints, without duplicating the work that Dynamo already does.