Skip to content
This repository has been archived by the owner on Jul 2, 2021. It is now read-only.

Sequential feature extractor remove_unused #348

Merged

Conversation

yuyu2172
Copy link
Member

Merge after #347 .

I found that it is beneficial to have a method that chops off unnecessary part from a feature extractor.
For instance, an optimizer hook is called for all parameters even if they are not used in a computational graph.

@yuyu2172 yuyu2172 force-pushed the sequential-feature-extractor-getitem branch from c56f021 to 2173223 Compare July 20, 2017 15:04
@Hakuyume
Copy link
Member

I merged #347. Could you merge master branch to this PR?

@yuyu2172
Copy link
Member Author

Merge after #349

@yuyu2172 yuyu2172 added this to the v0.7 milestone Jul 21, 2017
@yuyu2172 yuyu2172 force-pushed the sequential-feature-extractor-getitem branch from 63cd9da to 982f2f6 Compare July 21, 2017 01:54
@yuyu2172 yuyu2172 force-pushed the sequential-feature-extractor-getitem branch from a518f21 to b4fc033 Compare July 21, 2017 05:28
@yuyu2172 yuyu2172 changed the title Sequential feature extractor getitem Sequential feature extractor remove_unused Jul 21, 2017

"""
if self._feature_names is None:
feature_names = (self.all_feature_names[-1],)
Copy link
Member

Choose a reason for hiding this comment

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

If _feature_names is None, we can simply use return to skip this method.

delattr(ret, name)
return ret
def remove_unused(self):
"""Delete all features that are not needed for the forward pass.
Copy link
Member

Choose a reason for hiding this comment

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

This function removes layers (function or link). I prefer Delete all layers.

Copy link
Member

@Hakuyume Hakuyume left a comment

Choose a reason for hiding this comment

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

LGTM

@Hakuyume Hakuyume merged commit 89d18b3 into chainer:master Jul 23, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants