Add inputs argument to autograd.backward()
#46855
Closed
Conversation
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
soulitzer
requested review from
albanD,
apaszke,
ebetica,
glaringlee,
goldsborough and
yf225
as code owners
facebook-github-bot
added
the
oncall: jit
💊 CI failures summary and remediationsAs of commit 30b3516 (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group. This comment has been revised 120 times. |
soulitzer
force-pushed
the
change_backward_api
branch
from
311cd4e to
fcf409b
Compare
albanD
reviewed
Oct 26, 2020
Codecov Report
@@ Coverage Diff @@
## master #46855 +/- ##
==========================================
- Coverage 68.87% 68.87% -0.01%
==========================================
Files 436 436
Lines 56368 56371 +3
==========================================
+ Hits 38823 38825 +2
- Misses 17545 17546 +1 |
albanD
reviewed
Oct 27, 2020
albanD
reviewed
Oct 28, 2020
soulitzer
changed the title
inputs argument to autograd.backward()inputs argument to autograd.backward()
albanD
reviewed
Oct 29, 2020
soulitzer
requested review from
mingzhe09088,
mrshenli,
pietern,
pritamdamania87,
rohan-varma and
zhaojuanmao
as code owners
…ge_backward_api
albanD
reviewed
Oct 29, 2020
There was a problem hiding this comment.
@soulitzer has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
zou3519
reviewed
Oct 30, 2020
| // The user either called autograd.backward(...) or autograd.grad(...) to get here | ||
| bool backward_api_called = inputs == nullptr; | ||
| bool backward_api_called = accumulate_grad; | ||
| TORCH_CHECK(!backward_api_called || at::impl::VmapMode::current_vmap_level() == 0, |
There was a problem hiding this comment.
What does accumulate_grad do?
| const variable_list& inputs, | ||
| bool keep_graph, | ||
| bool create_graph, | ||
| bool accumulate_grad, |
There was a problem hiding this comment.
Could we add a comment here for what accumulate_grad does, for future code readers?
There was a problem hiding this comment.
Creating a separate PR for this
Closed
|
@soulitzer merged this pull request in f5073b0. |
This was referenced Nov 3, 2020
facebook-github-bot
pushed a commit
that referenced
this pull request
Nov 3, 2020
Summary: Addressing a comment from a PR that has already been merged #46855 #46855 (comment) Pull Request resolved: #47266 Reviewed By: agolynski Differential Revision: D24709017 Pulled By: soulitzer fbshipit-source-id: 3c104c2fef90ffd75951ecef4ae9e938d4b12d8c
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Fixes #46373
As noted in #46373, there needs to be a flag passed into the engine that indicates whether it was executed through the backward api or grad api. Tentatively named the flag
accumulate_gradsince functionally, backward api accumulates grad into .grad while grad api captures the grad and returns it.Moving changes not necessary to the python api (cpp, torchscript) to a new PR.