Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

make the slice of sequential a sequential #10630

Merged
merged 3 commits into from
Apr 27, 2018
Merged

Conversation

szha
Copy link
Member

@szha szha commented Apr 20, 2018

Description

make the slice of sequential a sequential

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage
  • Code is well-documented:
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • make slices of sequential/container blocks a container block

Comments

  • If this change is a backward incompatible change, why must this change be made.
  • Interesting edge cases to note here

layers = list(self._children.values())[key]
if isinstance(layers, list):
with self.name_scope():
net = type(self)()
Copy link
Contributor

Choose a reason for hiding this comment

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

why type(self)? There is no guarantee that type(self)'s constructor accepts no arguments

Copy link
Member Author

Choose a reason for hiding this comment

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

This is so that any blocks that inherit sequential blocks have this default implementation. Child class that have other arguments can choose to override this.

return list(self._children.values())[key]
layers = list(self._children.values())[key]
if isinstance(layers, list):
with self.name_scope():
Copy link
Contributor

Choose a reason for hiding this comment

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

would it better to set prefix directly to be the same as self instead of using name_scope?

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm, good point. this way we can get rid of one namescope

@szha
Copy link
Member Author

szha commented Apr 20, 2018

Found a minor bug in using _brief_print_list. in py3, dict.keys() doesn't return a list. https://github.com/apache/incubator-mxnet/blob/master/python/mxnet/gluon/parameter.py#L779

@szha
Copy link
Member Author

szha commented Apr 24, 2018

@piiswrong

@piiswrong piiswrong merged commit 01e305b into apache:master Apr 27, 2018
@szha szha deleted the slice_seq branch April 27, 2018 18:00
cjolivier01 pushed a commit to cjolivier01/mxnet that referenced this pull request Apr 27, 2018
* make the slice of sequential a sequential

* update per comment

* py3 compat
rahul003 pushed a commit to rahul003/mxnet that referenced this pull request Jun 4, 2018
* make the slice of sequential a sequential

* update per comment

* py3 compat
zheng-da pushed a commit to zheng-da/incubator-mxnet that referenced this pull request Jun 28, 2018
* make the slice of sequential a sequential

* update per comment

* py3 compat
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants