Skip to content
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.

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

Added params_require_grad arg to make_functional* #701

Merged
merged 4 commits into from
Apr 25, 2022

Conversation

vfdev-5
Copy link
Contributor

@vfdev-5 vfdev-5 commented Apr 14, 2022

Related to #639 (comment)

Description:

  • Added params_require_grad argument to make_functional and make_functional_with_buffers
  • Added tests

@samdow let me know if something else I'm missing for this PR. Thanks

@zou3519 zou3519 requested a review from samdow April 14, 2022 15:52
@vfdev-5 vfdev-5 force-pushed the add-make_functional-grad-arg branch from 4d55614 to cb1273a Compare April 19, 2022 13:42
@vfdev-5 vfdev-5 requested a review from samdow April 19, 2022 13:43
@zou3519 zou3519 self-requested a review April 21, 2022 15:44
functorch/_src/make_functional.py Outdated Show resolved Hide resolved
functorch/_src/make_functional.py Outdated Show resolved Hide resolved
@vfdev-5 vfdev-5 force-pushed the add-make_functional-grad-arg branch from db662b1 to a8830cf Compare April 22, 2022 15:36
@vfdev-5
Copy link
Contributor Author

vfdev-5 commented Apr 22, 2022

@zou3519 @samdow I updated the name and the docstring (and skipped the CI) to wait for your approval. If everything is fine, then I can restart the CI, we'll be able to merge.

Copy link
Contributor

@samdow samdow left a comment

Choose a reason for hiding this comment

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

LGTM! Just had the small nit that I missed earlier. Thanks for the patience with the doc + naming comments 😄

@vfdev-5 vfdev-5 force-pushed the add-make_functional-grad-arg branch from a8830cf to 1340fb0 Compare April 25, 2022 14:24
@samdow
Copy link
Contributor

samdow commented Apr 25, 2022

Merging since all of the failures look to be from upstream. Thanks for all your work on this @vfdev-5!

@samdow samdow merged commit 26a4e83 into pytorch:main Apr 25, 2022
@vfdev-5 vfdev-5 deleted the add-make_functional-grad-arg branch April 25, 2022 20:12
zou3519 pushed a commit to zou3519/pytorch that referenced this pull request Jul 20, 2022
…h/functorch#701)

* Added params_require_grad arg to make_functional*

* Renamed arg to disable_params_grad

* [skip ci] Updated docstring according to the review

* Updated docstring func signature
bigfootjon pushed a commit to pytorch/pytorch that referenced this pull request Jul 21, 2022
…h/functorch#701)

* Added params_require_grad arg to make_functional*

* Renamed arg to disable_params_grad

* [skip ci] Updated docstring according to the review

* Updated docstring func signature
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.

4 participants