fix bernoulli_ for functionalization by making it a primitive wrt torch_dispatch#82771
Closed
bdhirsh wants to merge 9 commits intogh/bdhirsh/292/basefrom
Closed
fix bernoulli_ for functionalization by making it a primitive wrt torch_dispatch#82771bdhirsh wants to merge 9 commits intogh/bdhirsh/292/basefrom
bdhirsh wants to merge 9 commits intogh/bdhirsh/292/basefrom
Conversation
…ch_dispatch [ghstack-poisoned]
This was referenced Aug 3, 2022
Closed
Contributor
🔗 Helpful links
❌ 14 New FailuresAs of commit 740192c (more details on the Dr. CI page): Expand to see more
🕵️ 14 new failures recognized by patternsThe following CI failures do not appear to be due to upstream breakages
|
…ive wrt torch_dispatch" Context: `bernoulli_.float` is currently a primitive op, but it's functional counterpart `bernoulli.p` is `CompositeImplicitAutograd`. This is a problem when we're using functionalization on top of python tracing: functionalization will convert `bernoulli_.float` into `bernoulli.p`, but the decomposition for `bernoulli.p` will run before hitting python, and happily convert back into `bernoulli_.float`. This doesn't seems to come up very often, but I think the right fix is to just make `bernoulli.p` a primitive w.r.t. the Python key too, by making it `CompositeExplicitAutograd`. [ghstack-poisoned]
zou3519
reviewed
Aug 4, 2022
| # This op needs to be a primitive w.r.t. Python, | ||
| # because functionalization wants to convert bernoulli_.float -> bernoulli.p. | ||
| # If we leave it as CompositeImplicitAutograd, then we'll run this kernel | ||
| # If we leave it as CompositeImplicitAutograd, then we'll run this kernel |
Contributor
There was a problem hiding this comment.
duplicated line in comment?
zou3519
reviewed
Aug 4, 2022
tools/autograd/derivatives.yaml
Outdated
Comment on lines
+336
to
+338
| - name: bernoulli.p(Tensor self, float p, *, Generator? generator=None) -> Tensor | ||
| self: zeros_like(grad) | ||
| result: auto_element_wise |
Contributor
There was a problem hiding this comment.
The behavior today is that bernoulli.p returns a Tensor without requires_grad. Does this PR change the behavior by giving it a backwards formula?
In [6]: x = torch.randn(3, 3, requires_grad=True)
In [7]: y = torch.bernoulli(x, 0.5)
In [8]: y
Out[8]:
tensor([[1., 1., 0.],
[0., 1., 0.],
[1., 0., 0.]])
Collaborator
There was a problem hiding this comment.
Correct, please mark output_differentiability = [False] here to preserve that behavior
…ive wrt torch_dispatch" Context: `bernoulli_.float` is currently a primitive op, but it's functional counterpart `bernoulli.p` is `CompositeImplicitAutograd`. This is a problem when we're using functionalization on top of python tracing: functionalization will convert `bernoulli_.float` into `bernoulli.p`, but the decomposition for `bernoulli.p` will run before hitting python, and happily convert back into `bernoulli_.float`. This doesn't seems to come up very often, but I think the right fix is to just make `bernoulli.p` a primitive w.r.t. the Python key too, by making it `CompositeExplicitAutograd`. [ghstack-poisoned]
albanD
approved these changes
Aug 11, 2022
Collaborator
albanD
left a comment
There was a problem hiding this comment.
Duplicate comments needs to be removed. But good otherwise.
…ive wrt torch_dispatch" Context: `bernoulli_.float` is currently a primitive op, but it's functional counterpart `bernoulli.p` is `CompositeImplicitAutograd`. This is a problem when we're using functionalization on top of python tracing: functionalization will convert `bernoulli_.float` into `bernoulli.p`, but the decomposition for `bernoulli.p` will run before hitting python, and happily convert back into `bernoulli_.float`. This doesn't seems to come up very often, but I think the right fix is to just make `bernoulli.p` a primitive w.r.t. the Python key too, by making it `CompositeExplicitAutograd`. [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/82771
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 Failures, 1 PendingAs of commit 3ba4219: The following jobs have failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
…ive wrt torch_dispatch" Context: `bernoulli_.float` is currently a primitive op, but it's functional counterpart `bernoulli.p` is `CompositeImplicitAutograd`. This is a problem when we're using functionalization on top of python tracing: functionalization will convert `bernoulli_.float` into `bernoulli.p`, but the decomposition for `bernoulli.p` will run before hitting python, and happily convert back into `bernoulli_.float`. This doesn't seems to come up very often, but I think the right fix is to just make `bernoulli.p` a primitive w.r.t. the Python key too, by making it `CompositeExplicitAutograd`. [ghstack-poisoned]
…ive wrt torch_dispatch" Context: `bernoulli_.float` is currently a primitive op, but it's functional counterpart `bernoulli.p` is `CompositeImplicitAutograd`. This is a problem when we're using functionalization on top of python tracing: functionalization will convert `bernoulli_.float` into `bernoulli.p`, but the decomposition for `bernoulli.p` will run before hitting python, and happily convert back into `bernoulli_.float`. This doesn't seems to come up very often, but I think the right fix is to just make `bernoulli.p` a primitive w.r.t. the Python key too, by making it `CompositeExplicitAutograd`. [ghstack-poisoned]
…ive wrt torch_dispatch" Context: `bernoulli_.float` is currently a primitive op, but it's functional counterpart `bernoulli.p` is `CompositeImplicitAutograd`. This is a problem when we're using functionalization on top of python tracing: functionalization will convert `bernoulli_.float` into `bernoulli.p`, but the decomposition for `bernoulli.p` will run before hitting python, and happily convert back into `bernoulli_.float`. This doesn't seems to come up very often, but I think the right fix is to just make `bernoulli.p` a primitive w.r.t. the Python key too, by making it `CompositeExplicitAutograd`. [ghstack-poisoned]
…ive wrt torch_dispatch" Context: `bernoulli_.float` is currently a primitive op, but it's functional counterpart `bernoulli.p` is `CompositeImplicitAutograd`. This is a problem when we're using functionalization on top of python tracing: functionalization will convert `bernoulli_.float` into `bernoulli.p`, but the decomposition for `bernoulli.p` will run before hitting python, and happily convert back into `bernoulli_.float`. This doesn't seems to come up very often, but I think the right fix is to just make `bernoulli.p` a primitive w.r.t. the Python key too, by making it `CompositeExplicitAutograd`. [ghstack-poisoned]
Closed
…ive wrt torch_dispatch" Context: `bernoulli_.float` is currently a primitive op, but it's functional counterpart `bernoulli.p` is `CompositeImplicitAutograd`. This is a problem when we're using functionalization on top of python tracing: functionalization will convert `bernoulli_.float` into `bernoulli.p`, but the decomposition for `bernoulli.p` will run before hitting python, and happily convert back into `bernoulli_.float`. This doesn't seems to come up very often, but I think the right fix is to just make `bernoulli.p` a primitive w.r.t. the Python key too, by making it `CompositeExplicitAutograd`. [ghstack-poisoned]
Contributor
|
/easycla As part of the transition to the PyTorch Foundation, this project now requires contributions be covered under the new CLA. See #85559 for additional details. This comment will trigger a new check of this PR. If you are already covered, you will simply see a new "EasyCLA" check that passes. If you are not covered, a bot will leave a new comment with a link to sign. |
Contributor
|
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
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
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.
Context:
bernoulli_.floatis currently a primitive op, but it's functional counterpartbernoulli.pisCompositeImplicitAutograd.This is a problem when we're using functionalization on top of python tracing: functionalization will convert
bernoulli_.floatintobernoulli.p, but the decomposition forbernoulli.pwill run before hitting python, and happily convert back intobernoulli_.float.This doesn't seems to come up very often, but I think the right fix is to just make
bernoulli.pa primitive w.r.t. the Python key too, by making itCompositeExplicitAutograd.Stack from ghstack: