-
Notifications
You must be signed in to change notification settings - Fork 25.7k
Further improvements of nn.container docs #17731
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
Conversation
| or another :class:`~torch.nn.ParameterDict` (the argument to | ||
| :meth:`~torch.nn.ParameterDict.update`). | ||
| Note that :meth:`~torch.nn.ParameterDict.update` with other unordered mapping |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be added in a note-block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sort of goes together with the previous paragraph, so I didn't make it a .. note::
torch/nn/modules/container.py
Outdated
| :meth:`~torch.nn.ParameterDict.update`). | ||
| Note that :meth:`~torch.nn.ParameterDict.update` with other unordered mapping | ||
| types (e.g., Python's plain ``dict``) doesn't not preserve order of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't not -> does not
|
|
||
| if isinstance(parameters, container_abcs.Mapping): | ||
| if isinstance(parameters, OrderedDict): | ||
| if isinstance(parameters, (OrderedDict, ParameterDict)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, didn't know you could do this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah it takes in a tuple :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment on something that looks like a typo. Otherwise, looks good to me
|
@zou3519 Thanks! I've fixed the typo. Hopefully lint passes as I edited on github UI directly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@soumith is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
No description provided.