Skip to content

[Ready] Add ModuleDict and ParameterDict containers #8463

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
karandwivedi42 wants to merge 32 commits into from
Closed

[Ready] Add ModuleDict and ParameterDict containers #8463

karandwivedi42 wants to merge 32 commits into from

Conversation

@karandwivedi42
Copy link
Contributor

@karandwivedi42 karandwivedi42 commented Jun 13, 2018

Addresses:

#4048 and #5297 (comment)

@ezyang
Copy link
Contributor

ezyang commented Jun 14, 2018

@pytorchbot retest this please

apaszke
apaszke reviewed Jun 14, 2018
View reviewed changes
torch/nn/modules/container.py Outdated

This comment was marked as off-topic.

Sign in to view

torch/nn/modules/container.py Outdated

This comment was marked as off-topic.

Sign in to view

@karandwivedi42 karandwivedi42 changed the title [WIP] Add ModuleDict and ParameterDict containers [Ready] Add ModuleDict and ParameterDict containers Jun 18, 2018
@karandwivedi42
Copy link
Contributor Author

karandwivedi42 commented Jun 20, 2018

@apaszke made the changes. Is this good to merge?

apaszke
apaszke reviewed Jun 21, 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.

Mostly LGTM. Some small comments to fix, and it should be good to merge

torch/nn/modules/container.py Outdated

This comment was marked as off-topic.

Sign in to view

torch/nn/modules/container.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

torch/nn/modules/container.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

torch/nn/modules/container.py Outdated

This comment was marked as off-topic.

Sign in to view

torch/nn/modules/container.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

ssnl
ssnl requested changes Jun 26, 2018
View reviewed changes
torch/nn/modules/container.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

torch/nn/modules/container.py Outdated

This comment was marked as off-topic.

Sign in to view

@karandwivedi42 karandwivedi42 mentioned this pull request Jun 27, 2018
Closed
facebook-github-bot
facebook-github-bot reviewed Jun 29, 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.

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

facebook-github-bot
facebook-github-bot reviewed Jun 29, 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.

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

ezyang
ezyang reviewed Jun 29, 2018
View reviewed changes
torch/nn/modules/container.py Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

ezyang
ezyang requested changes Jun 29, 2018
View reviewed changes
Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

Unicode is a problem.

facebook-github-bot pushed a commit that referenced this pull request Jun 29, 2018
@karandwivedi42 @facebook-github-bot
Better print of nn Containers (#8939)
37e526e 
Summary:
Fix #8900

Waiting on #8463

1. Remove extra Line
2. ...
Closes #8939

Reviewed By: soumith

Differential Revision: D8687730

Pulled By: ezyang

fbshipit-source-id: 81c57a03683875704d537cb4585b11838f70df56
apaszke
apaszke reviewed Jul 2, 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.

Just had an idea that could potentially deduplicate the implementation

torch/nn/modules/container.py
return tmpstr


class ParameterDict(Module):

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

facebook-github-bot
facebook-github-bot reviewed Jul 3, 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.

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

@karandwivedi42
Copy link
Contributor Author

karandwivedi42 commented Jul 9, 2018

@ezyang removed the unicode characters. Is it good to go?

@ezyang
Copy link
Contributor

ezyang commented Jul 14, 2018

Yes. I'll import it soon.

facebook-github-bot
facebook-github-bot reviewed Jul 14, 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.

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

petrex pushed a commit to petrex/pytorch that referenced this pull request Jul 16, 2018
Merge remote-tracking branch 'upstream/master' into hipify_c2
986eeb7 
* upstream/master: (24 commits)
  Implement tensor weak references (pytorch#9363)
  Nuke TestCollectEnv (pytorch#9459)
  Add test case for segmentation fault fix in grad_fn (pytorch#9457)
  Add peephole optimization for type_as operators. (pytorch#9316)
  Fix out-of-range error for test_neg (pytorch#9431)
  add depthwise conv support for mkldnn (pytorch#8782)
  Refactor `_log_sum_exp` (pytorch#9173)
  Add ModuleDict and ParameterDict containers (pytorch#8463)
  Introduce SupervisedPtr, delete THAllocator and THCDeviceAllocator (pytorch#9358)
  Introducing IsInf (pytorch#9169)
  add device to CUDAEvent (pytorch#9415)
  Make localScalar error message more intuitive (pytorch#9443)
  Only accept continguous tensors in TopK for cuda (pytorch#9441)
  Add support for .norm() pytorch onnx export and ReduceL1/ReduceL2 caffe2 operators (pytorch#9299)
  Only view() rhs of index_put if we need to (pytorch#9424)
  Add BatchBucketizeOp in caffe2 (pytorch#9385)
  Implementation of Wngrad optimizer caffe2 python wrapper and unit test on least square regression (pytorch#9001)
  Implementation and operator test for Wngrad optimizer (pytorch#8999)
  Fix segmentation fault in grad_fn (pytorch#9292)
  update docs (pytorch#9423)
  ...
@vishwakftw vishwakftw mentioned this pull request Jul 17, 2018
Closed
goldsborough pushed a commit to goldsborough/pytorch that referenced this pull request Jul 20, 2018
@karandwivedi42 @goldsborough
Add ModuleDict and ParameterDict containers (pytorch#8463)
eec2a94 
Summary:
Addresses:

pytorch#4048 and pytorch#5297 (comment)
Pull Request resolved: pytorch#8463

Reviewed By: SsnL

Differential Revision: D8689291

Pulled By: ezyang

fbshipit-source-id: 47e67d9bae1b64ec10771a2c00c56229463b1598
jramseyer pushed a commit to jramseyer/pytorch that referenced this pull request Jul 30, 2018
@karandwivedi42 @jramseyer
Add ModuleDict and ParameterDict containers (pytorch#8463)
8e3aef5 
Summary:
Addresses:

pytorch#4048 and pytorch#5297 (comment)
Pull Request resolved: pytorch#8463

Reviewed By: SsnL

Differential Revision: D8689291

Pulled By: ezyang

fbshipit-source-id: 47e67d9bae1b64ec10771a2c00c56229463b1598
goodlux pushed a commit to goodlux/pytorch that referenced this pull request Aug 15, 2018
@karandwivedi42
Better print of nn Containers (pytorch#8939)
9d2349d 
Summary:
Fix pytorch#8900

Waiting on pytorch#8463

1. Remove extra Line
2. ...
Closes pytorch#8939

Reviewed By: soumith

Differential Revision: D8687730

Pulled By: ezyang

fbshipit-source-id: 81c57a03683875704d537cb4585b11838f70df56
goodlux pushed a commit to goodlux/pytorch that referenced this pull request Aug 15, 2018
@karandwivedi42
Add ModuleDict and ParameterDict containers (pytorch#8463)
6f7204f 
Summary:
Addresses:

pytorch#4048 and pytorch#5297 (comment)
Pull Request resolved: pytorch#8463

Reviewed By: SsnL

Differential Revision: D8689291

Pulled By: ezyang

fbshipit-source-id: 47e67d9bae1b64ec10771a2c00c56229463b1598
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ssnl ssnl ssnl requested changes

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

@ezyang ezyang ezyang requested changes

@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

@zou3519 zou3519 Awaiting requested review from zou3519

+1 more reviewer

@apaszke apaszke apaszke approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

open source

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

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