Skip to content

[C++ API] Add backward() to Tensor and Variable #7750

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.

Sign up for GitHub

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

Jump to bottom
Merged
goldsborough merged 2 commits into from
May 22, 2018
Merged

[C++ API] Add backward() to Tensor and Variable #7750

goldsborough merged 2 commits into from
May 22, 2018

Conversation

@goldsborough
Copy link
Contributor

@goldsborough goldsborough commented May 22, 2018 •
edited
Loading

A sort of small and uncomplicated change, but nice for the C++ world: adds backward() to the "autograd API" of at::Tensor and implements it for VariableImpl to execute the default autograd engine on the variable.

Main code is in autograd/variable.cpp.

I've removed the equivalent free function from autogradpp.

@zdevito @colesbury @apaszke @ebetica

@goldsborough goldsborough changed the title Backward [C++ API] Add backward() to Tensor and Variable May 22, 2018
@goldsborough
Copy link
Contributor Author

goldsborough commented May 22, 2018 •
edited
Loading

windows CI be like:
image

@goldsborough
Copy link
Contributor Author

goldsborough commented May 22, 2018

@pytorchbot retest this please

apaszke
apaszke approved these changes May 22, 2018
View reviewed changes
Copy link
Contributor

@apaszke apaszke left a comment

Choose a reason for hiding this comment

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

LGTM

Splitting namespaces into multiple lines is really unnecessary. Such changes don't add any value to our code, and really make it less consistent. Please don't include such changes in the future, unless we establish a code format policy, and will do a full codebase refactor.

torch/csrc/autograd/variable.cpp Outdated

This comment was marked as off-topic.

Sign in to view

torch/csrc/autograd/variable.h Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

ebetica
ebetica reviewed May 22, 2018
View reviewed changes
torch/csrc/autograd/variable.cpp Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

@goldsborough goldsborough merged commit 1e27627 into pytorch:master May 22, 2018
@goldsborough goldsborough deleted the backward branch May 22, 2018 17:43
@ezyang
Copy link
Contributor

ezyang commented May 22, 2018

FYI, according to GitHub this was merged without CI going green. Even after the subsequent fix, Windows is still failing (#7762).

yf225 added a commit that referenced this pull request May 22, 2018
@yf225
Revert "[C++ API] Add backward() to Tensor and Variable (#7750)"
fbeadc6 
This reverts commit 1e27627.
yf225 pushed a commit to yf225/pytorch that referenced this pull request May 22, 2018
Revert "[C++ API] Add backward() to Tensor and Variable (pytorch#7750)"
a376150 
This reverts commit 1e27627.
@goldsborough
Copy link
Contributor Author

goldsborough commented May 22, 2018

Yeah I pushed a quick two line formatting fix after everything was already green, and merged it immediately. The formatting fix had the missing braces

@yf225 yf225 mentioned this pull request May 22, 2018
Merged
goldsborough pushed a commit that referenced this pull request May 22, 2018
@yf225 @goldsborough
Revert #7750 and #7762 to fix Windows CI on master (#7772)
60745b3 
* Revert "Add missing brace (#7762)"

This reverts commit ea27c5a.

* Revert "[C++ API] Add backward() to Tensor and Variable  (#7750)"

This reverts commit 1e27627.
@goldsborough goldsborough mentioned this pull request May 22, 2018
Merged
petrex pushed a commit to petrex/pytorch that referenced this pull request May 23, 2018
Merge branch 'caffe2_core_hip' of github.com:petrex/pytorch into caff…
e7a80cd 
…e2_core_hip

* 'caffe2_core_hip' of github.com:petrex/pytorch: (24 commits)
  Allow empty storage for the 'Edge' class. (pytorch#7595)
  Process group base class and Gloo implementation (pytorch#7628)
  _LRSchedulers getstate include optimizer info (pytorch#7757)
  [PyTorch] [gradcheck] change backward() to grad() (pytorch#7710)
  Update test_nn.py (pytorch#7787)
  Define general default scheduler for TBB and fix ppc64le bug (pytorch#7761)
  Add support for accepting Tensor as input in clip_grad_*  functions. (pytorch#7769)
  [Easy] Remove unused code (pytorch#7782)
  Update tbb (pytorch#7734)
  Add @generated annotation (pytorch#7780)
  fix legacy comment after variable tensor merge (pytorch#7771)
  Revert pytorch#7750 and pytorch#7762 to fix Windows CI on master (pytorch#7772)
  Temporarily disable build env check (pytorch#7768)
  Add missing brace (pytorch#7762)
  [C++ API] Add backward() to Tensor and Variable  (pytorch#7750)
  [auto] Update onnx to d43b550 - Fix .gitignore and add missing files (onnx/onnx#1005) onnx/onnx@d43b550
  [auto] Update onnx to ea1aa13 - add tests for reduce ops (onnx/onnx#675) onnx/onnx@ea1aa13
  include cudnn_h (pytorch#7749)
  [C++ API] Using new registration mechanism (pytorch#7663)
  [auto] Update onnx to 5dd68e6 - Add a util function: polish_model (onnx/onnx#1000) onnx/onnx@5dd68e6
  ...
weiyangfb pushed a commit to weiyangfb/pytorch that referenced this pull request Jun 11, 2018
@goldsborough @weiyangfb
[C++ API] Add backward() to Tensor and Variable (pytorch#7750)
bf977a9 
* Add backward() to Tensor and Variable

* Added a couple tests
weiyangfb pushed a commit to weiyangfb/pytorch that referenced this pull request Jun 11, 2018
@yf225 @weiyangfb
Revert pytorch#7750 and pytorch#7762 to fix Windows CI on master (pyt…
12370bc 
…orch#7772)

* Revert "Add missing brace (pytorch#7762)"

This reverts commit ea27c5a.

* Revert "[C++ API] Add backward() to Tensor and Variable  (pytorch#7750)"

This reverts commit 1e27627.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@colesbury colesbury Awaiting requested review from colesbury

@ezyang ezyang Awaiting requested review from ezyang

@gchanan gchanan Awaiting requested review from gchanan

@soumith soumith Awaiting requested review from soumith

@zdevito zdevito Awaiting requested review from zdevito

2 more reviewers

@ebetica ebetica ebetica left review comments

@apaszke apaszke apaszke approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@goldsborough @ezyang @ebetica @apaszke