Skip to content

Conversation

kshitij12345
Copy link
Collaborator

@kshitij12345 kshitij12345 commented Jul 22, 2022

Ref: #69991

Also fixes #82644

For CompositeCompliance, we can't use item to choose a special fast-path when Tensor is a Subclass. Instead we always dispatch to the slower but safer implementation.

@diff-train-skip-merge

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jul 22, 2022

🔗 Helpful links

✅ No Failures (0 Pending)

As of commit ce42074 (more details on the Dr. CI page):

Expand to see more

💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@kshitij12345 kshitij12345 requested a review from zou3519 August 2, 2022 15:53
@kshitij12345 kshitij12345 marked this pull request as ready for review August 2, 2022 15:53
@albanD albanD added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Aug 3, 2022
@kshitij12345
Copy link
Collaborator Author

Ping @zou3519

@pytorch-bot
Copy link

pytorch-bot bot commented Sep 14, 2022

🔗 Helpful Links

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

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

❌ 1 Failures

As of commit f94152e:

The following jobs have failed:

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

Copy link
Contributor

@zou3519 zou3519 left a comment

Choose a reason for hiding this comment

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

We should not regress support for input shapes like (3, 0). To do that, I think just calling grad * (result / input).conj(); in the zero-numel case works. Other than that, this LGTM, with some minor comments

Copy link
Contributor

@zou3519 zou3519 left a comment

Choose a reason for hiding this comment

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

Thanks!

@kshitij12345
Copy link
Collaborator Author

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a merge job. Check the current status here and land check progress here.
The merge job was triggered with the land checks (-l) flag. If you did not specify this flag yourself, you are likely enrolled in the land checks rollout. This means that your change will be merged once all checks on your PR and the land checks have passed (ETA 4 Hours). If you need to coordinate lands between different changes and cannot risk a land race, please add the ciflow/trunk label to your PR and wait for signal to complete, and then land your changes in proper order. Having trunk, pull, and Lint pre-run on a PR will bypass land checks and the ETA should be immediate. If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: The following mandatory check(s) failed (Rule superuser):

Dig deeper by viewing the failures on hud

If you believe this is an error, you can use the old behavior with @pytorchbot merge -g (optionally with the ciflow/trunk to get land checks) or use @pytorchbot merge -f "some reason here". For more information, see the bot wiki.

Please reach out to the PyTorch DevX Team with feedback or questions!

Details for Dev Infra team Raised by workflow job

@kshitij12345
Copy link
Collaborator Author

@pytorchbot merge -f "CI failure was flaky"

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a merge job. Check the current status here.
The merge job was triggered with the force (-f) flag. This means your change will be merged immediately, bypassing any CI checks (ETA: 1-5 minutes). If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

@github-actions
Copy link
Contributor

Hey @kshitij12345.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

@dagitses dagitses reopened this Sep 20, 2022
@dagitses dagitses closed this Sep 20, 2022
@facebook-github-bot
Copy link
Contributor

@dagitses has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@dagitses
Copy link
Collaborator

@pytorchbot revert --message='a PR this is based on got reverted, rebase and reland' --classification=ghfirst

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here.
Please reach out to the PyTorch DevX Team with feedback or questions!

@pytorchmergebot
Copy link
Collaborator

Reverting PR 81969 failed

Reason: Command git -C /home/runner/work/pytorch/pytorch revert --no-edit a4dca9822dfabcdbd1b36a12c013764f2af87613 returned non-zero exit code 1

Auto-merging functorch/test/test_ops.py
CONFLICT (content): Merge conflict in functorch/test/test_ops.py
Auto-merging torch/csrc/autograd/FunctionsManual.cpp
CONFLICT (content): Merge conflict in torch/csrc/autograd/FunctionsManual.cpp
Auto-merging torch/testing/_internal/common_methods_invocations.py
Auto-merging torch/testing/_internal/opinfo/definitions/_masked.py
error: could not revert a4dca9822d... [composite compliance] prod (#81969)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git revert --continue".
hint: You can instead skip this commit with "git revert --skip".
hint: To abort and get back to the state before "git revert",
hint: run "git revert --abort".
Details for Dev Infra team Raised by workflow job

@dagitses
Copy link
Collaborator

@pytorchbot revert --message='a PR this is based on got reverted, rebase and reland' --classification=ghfirst

@dagitses
Copy link
Collaborator

Might as well also include #85400 in the stack when you reland this.

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here.
Please reach out to the PyTorch DevX Team with feedback or questions!

@pytorchmergebot
Copy link
Collaborator

Reverting PR 81969 failed

Reason: Command git -C /home/runner/work/pytorch/pytorch revert --no-edit a4dca9822dfabcdbd1b36a12c013764f2af87613 returned non-zero exit code 1

Auto-merging torch/csrc/autograd/FunctionsManual.cpp
CONFLICT (content): Merge conflict in torch/csrc/autograd/FunctionsManual.cpp
Auto-merging torch/testing/_internal/common_methods_invocations.py
error: could not revert a4dca9822d... [composite compliance] prod (#81969)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git revert --continue".
hint: You can instead skip this commit with "git revert --skip".
hint: To abort and get back to the state before "git revert",
hint: run "git revert --abort".
Details for Dev Infra team Raised by workflow job

@kshitij12345
Copy link
Collaborator Author

@dagitses is there anything to be done from my end?

mehtanirav pushed a commit that referenced this pull request Oct 4, 2022
Ref: #69991

Also fixes #82644 (fix similar to #81617)

For CompositeCompliance, we can't use `item` to choose a special fast-path when Tensor is a Subclass. Instead we always dispatch to the slower but safer implementation.
Pull Request resolved: #81969
Approved by: https://github.com/zou3519
mehtanirav added a commit that referenced this pull request Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla signed Merged open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

torch.prod : backward errors when a tensor contains zero value

7 participants