Skip to content

Add buffers(), named_buffers() methods. #10554

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
Closed
jma127 wants to merge 1 commit into from
Closed

Add buffers(), named_buffers() methods. #10554

jma127 wants to merge 1 commit into from

Conversation

@jma127
Copy link
Contributor

@jma127 jma127 commented Aug 15, 2018

This commit adds the buffers() and named_buffers() methods as
analogues of parameters() and named_parameters().

@ezyang
Copy link
Contributor

ezyang commented Aug 15, 2018

don't forget tests!

@jma127
Copy link
Contributor Author

jma127 commented Aug 15, 2018

Yep, adding as we speak :)

@jma127 jma127 changed the title [WIP] Add buffers(), named_buffers() methods. Add buffers(), named_buffers() methods. Aug 16, 2018
@jma127
Copy link
Contributor Author

jma127 commented Aug 16, 2018

Ready for review. cc @ezyang @ssnl @apaszke

@jma127 jma127 mentioned this pull request Aug 16, 2018
Closed
ezyang
ezyang reviewed Aug 16, 2018
View reviewed changes
torch/nn/modules/module.py 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

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

This comment was marked as off-topic.

Sign in to view

ezyang
ezyang reviewed Aug 16, 2018
View reviewed changes
torch/nn/modules/module.py Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

apaszke
apaszke approved these changes Aug 16, 2018
View reviewed changes
torch/nn/modules/module.py Outdated

This comment was marked as off-topic.

Sign in to view

torch/nn/modules/module.py Outdated

This comment was marked as off-topic.

Sign in to view

@apaszke
Copy link
Contributor

apaszke commented Aug 16, 2018

LGTM, but please let's remove the stuff that you kept for backwards compatibility. All those functions were implementation details, and we shouldn't keep them around for the sake of code clarity.

@jma127
Copy link
Contributor Author

jma127 commented Aug 16, 2018

Got it, will do. memo made me feel real icky, so glad to get rid of it 😄

@jma127
Add buffers(), named_buffers() methods.
4bf968b 
This commit adds the ``buffers()`` and ``named_buffers()`` methods as
analogues of ``parameters()`` and ``named_parameters()``.
@jma127
Copy link
Contributor Author

jma127 commented Aug 16, 2018

Ready to merge, cc @apaszke / @ezyang as mergers.

The one CI failure looks spurious.

facebook-github-bot
facebook-github-bot reviewed Aug 16, 2018
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

SsnL has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@jma127
Copy link
Contributor Author

jma127 commented Aug 16, 2018

@pytorchbot retest this please

@benbellheron benbellheron mentioned this pull request Oct 12, 2018
Closed
@ezyang ezyang added the merged label Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ezyang ezyang ezyang left review comments

@ssnl ssnl ssnl left review comments

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@colesbury colesbury Awaiting requested review from colesbury

@gchanan gchanan Awaiting requested review from gchanan

@soumith soumith Awaiting requested review from soumith

@zdevito zdevito Awaiting requested review from zdevito

+1 more reviewer

@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.

5 participants

@jma127 @ezyang @apaszke @ssnl @facebook-github-bot