Skip to content

Conversation

ydwu4
Copy link
Contributor

@ydwu4 ydwu4 commented Nov 16, 2023

Stack from ghstack (oldest at bottom):

This PR checks the tensor meta of the outputs of cond's branches. This helps us to identify several tests that return outputs that have different requires_grad. Also fix the error messages, which previously was in torch.ops.higher_order.cond now is raised in dynamo CondHigherOrder.

Test Plan:
Existing tests.

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @aakhundov @kadeng

@pytorch-bot
Copy link

pytorch-bot bot commented Nov 16, 2023

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/113900

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit f535862 with merge base 9cbee47 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

This PR checks the tensor meta of the outputs of cond's branches. This helps us to identify several tests that return outputs that have different requires_grad. Also fix the error messages, which previously was in torch.ops.higher_order.cond now is raised in dynamo CondHigherOrder.

Test Plan:
Existing tests.


cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng

[ghstack-poisoned]
ydwu4 added a commit that referenced this pull request Nov 17, 2023
ghstack-source-id: 51b18f7
Pull Request resolved: #113900
Comment on lines +531 to +535
# We check the meta data associated with meta["example_value"]
meta1 = _extract_tensor_metadata(var1.proxy.node.meta["example_value"])
meta2 = _extract_tensor_metadata(var2.proxy.node.meta["example_value"])
if meta1 != meta2:
all_diffs.append((f"pair{i}:", meta1, meta2))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this also do strides? This might make cond difficult to use, but we can cross that bridge when it happens (e.g. if strides mismatch we can always just return .contiguous())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It includes strides. Sounds good!

@ydwu4
Copy link
Contributor Author

ydwu4 commented Nov 21, 2023

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Nov 21, 2023
@pytorchmergebot
Copy link
Collaborator

Merge started

Your 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

Advanced Debugging
Check the merge workflow status
here

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: Command git -C /home/runner/work/pytorch/pytorch cherry-pick -x 59aa49173f454fd1f3c364539025f5001f2a5f17 returned non-zero exit code 1

Auto-merging test/dynamo/test_export.py
Auto-merging test/functorch/test_control_flow.py
Auto-merging torch/_dynamo/variables/builtin.py
CONFLICT (content): Merge conflict in torch/_dynamo/variables/builtin.py
Auto-merging torch/_dynamo/variables/higher_order_ops.py
error: could not apply 59aa49173f4... [HigherOrderOp] set should_flatten_output=True for cond
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
Details for Dev Infra team Raised by workflow job

This PR checks the tensor meta of the outputs of cond's branches. This helps us to identify several tests that return outputs that have different requires_grad. Also fix the error messages, which previously was in torch.ops.higher_order.cond now is raised in dynamo CondHigherOrder.

Test Plan:
Existing tests.


cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng

[ghstack-poisoned]
ydwu4 added a commit that referenced this pull request Nov 22, 2023
ghstack-source-id: 62106b0
Pull Request resolved: #113900
@ydwu4
Copy link
Contributor Author

ydwu4 commented Nov 22, 2023

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your 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

Advanced Debugging
Check the merge workflow status
here

@facebook-github-bot facebook-github-bot deleted the gh/ydwu4/45/head branch November 25, 2023 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants