-
Notifications
You must be signed in to change notification settings - Fork 22.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[dynamo] Change dimension constraint summary to log.info #101584
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/101584
Note: Links to docs will display an error until the docs builds have been completed. ✅ 2 Unrelated FailuresAs of commit 3f3428e: BROKEN TRUNK - The following jobs failed but were present on the merge base 964e61e:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please update docs to reflect the change?
"Summary of dimension constraints:%s", | ||
dim_constraints.prettify_results(inspect.signature(f)), | ||
) | ||
|
||
# Inline constraints added by users correspond to unbacked symbols in shape_env, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just move these lines for inline constraints up and delete the repeated if branch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following lines are for adding "inline_constraints" to new_graph.meta which doesn't happen until after the aten_graph=True call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then, we probably can move the lines down instead of moving lines up. Can we do that? Another reason is that fake_mode is checked to be not None in L904, we shouldn't do the getattr(fake_mode...) before L904.
Concretely, I mean delete the branch testing in L883-L896, and add the if constraint_violation_error check in L1027.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure moving it down is going to work, because we want the message to be attached to any constraint violation error, which is thrown above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I got your point. I just feel it's kind of redundant to do the checking again. The other alternative is to throw the exception later but it seems to be not so desirable as well..
@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 |
Merge failedReason: 2 jobs have failed, first few of them are: inductor / cuda11.8-py3.10-gcc7-sm86 / test (inductor_torchbench, 1, 1, linux.g5.4xlarge.nvidia.gpu), inductor / cuda11.8-py3.10-gcc7-sm86 / test (inductor_torchbench_dynamic, 1, 1, linux.g5.4xlarge.nvidia.gpu) Details for Dev Infra teamRaised by workflow job |
@pytorchbot merge -f "2 unrelated failures" |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Running with `TORCH_LOGS=dynamo` will have the dimension constraint summary pop up again. Pull Request resolved: #101584 Approved by: https://github.com/avikchaudhuri
Running with
TORCH_LOGS=dynamo
will have the dimension constraint summary pop up again.cc @voznesenskym @penguinwu @anijain2305 @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @desertfire