Add sequential_feature_extractor #342
Conversation
yuyu2172
force-pushed
the
sequential-feature-extractor
branch
from
090940e to
3c031d4
Compare
|
|
||
| This class is a base class that can be used for an implementation of | ||
| a feature extractor model. | ||
| The link takes as an argument :obj:`layers` that specifies the computation |
| a feature extractor model. | ||
| The link takes as an argument :obj:`layers` that specifies the computation | ||
| conducted in :meth:`__call__`. | ||
| :obj:`layers` is a list or :obj:`collections.OrderedDict` of |
| conducted in :meth:`__call__`. | ||
| :obj:`layers` is a list or :obj:`collections.OrderedDict` of | ||
| callable objects called layers, which are going to be called sequentially | ||
| starting from the top to the end. |
There was a problem hiding this comment.
from the top to the end -> from the top to the bottom
| :obj:`layers` is a list or :obj:`collections.OrderedDict` of | ||
| callable objects called layers, which are going to be called sequentially | ||
| starting from the top to the end. | ||
| A :obj:`chainer.Link` object in the sequence will be added as |
| :meth:`__call__` returns single or multiple features that are picked up | ||
| through a stream of computation. | ||
| These features can be specified by :obj:`layer_names`, which contains | ||
| the names of the layers whose output is collected. |
| the names of the layers whose output is collected. | ||
| When :obj:`layer_names` is a string, single value is returned. | ||
| When :obj:`layer_names` is an iterable of strings, a tuple of values | ||
| will be returned. The order of the values is the same as the order of |
There was a problem hiding this comment.
will be -> is. You used is in L. 24.
| if not isinstance(layers, collections.OrderedDict): | ||
| layers = collections.OrderedDict( | ||
| [('{}_{}'.format(layer.__class__.__name__, i), layer) | ||
| for i, layer in enumerate(layers)]) |
There was a problem hiding this comment.
I don't like layer.__class__. Do we need to support iterables? (I mean OrderedDict is enough).
There was a problem hiding this comment.
Nice suggestion.
I dropped support for iterables.
| else: | ||
| return_tuple = False | ||
| layer_names = [layer_names] | ||
| if any([name not in self._layers for name in layer_names]): |
There was a problem hiding this comment.
any(name not in self._layers for name in layer_names)
| layer_names = list(self._layers.keys())[-1] | ||
|
|
||
| if (not isinstance(layer_names, str) and | ||
| all([isinstance(name, str) for name in layer_names])): |
There was a problem hiding this comment.
all(isinstance(name, str) for name in layer_names)
| # The biggest index among indices of the layers that are included | ||
| # in self._layer_names. | ||
| last_index = max([list(self._layers.keys()).index(name) for | ||
| name in self._layer_names]) |
There was a problem hiding this comment.
How about last_index = max(map(list(self._layers.keys()).index, self._layer_names)) ?
This version calls list(self._layers.keys()) only once.
There was a problem hiding this comment.
I personally find this more readable.
layers_keys = list(self._layers.keys())
last_index = max(
[layer_keys.index(name) for name in self._layer_names])There was a problem hiding this comment.
Making a temporal list layers_keys is good. Note that we can replace max([...]) with max(...).
|
|
||
| if self._return_tuple: | ||
| features = tuple( | ||
| [features[name] for name in self._layer_names]) |
| >>> feat1, feat2 = model(x) | ||
| >>> # The layer_names can be dynamically changed. | ||
| >>> model.layer_names = 'l3' | ||
| >>> # This is an output of layer l1. |
| >>> ('l3', L.Linear(None, 10))]) | ||
| >>> model = SequentialFeatureExtractor(layers, ['l2_relu', 'l1_relu']) | ||
| >>> # These are outputs of layer l2_relu and l1_relu. | ||
| >>> feat1, feat2 = model(x) |
There was a problem hiding this comment.
feat1, feat2 -> feat2, feat1? (feat1 sounds a feature from l1_relu)
| return features | ||
|
|
||
| def copy(self): | ||
| ret = super(SequentialFeatureExtractor, self).copy() |
There was a problem hiding this comment.
How about this?
ret = super(SequentialFeatureExtractor, self).copy()
for name in ret._layers.keys():
if hasattr(ret, name):
ret._layers[name] = ret[name]
return ret
| out = self.link(x) | ||
|
|
||
| self.assertEqual(len(out), 3) | ||
| self.assertIsInstance(out[0], chainer.Variable) |
|
|
||
| def check_copy(self): | ||
| copied = self.link.copy() | ||
| self.assertIs(copied.l1, copied.layers['l1']) |
…ure-extractor Use init_scope in SequentialFeatureExtractor
|
Related yuyu2172#2 |
|
|
||
| Examples: | ||
|
|
||
| >>> import collections |
There was a problem hiding this comment.
I forgot to remove this line. Could you remove this?
| A :class:`chainer.Link` object in the sequence will be added as | ||
| a child link of this link. | ||
|
|
||
| :meth:`__call__` returns single or multiple features that are picked up |
There was a problem hiding this comment.
I think that is OK.
It works like which.
|
How about using |
|
I think The word |
|
Yes, |
| layer_names (string or iterable of strings): | ||
| Names of layers whose outputs will be collected in | ||
| feature_names (string or iterable of strings): | ||
| Names of feature that are collected during |
Hakuyume
force-pushed
the
sequential-feature-extractor
branch
from
9a3fdee to
faa7f22
Compare
…ntial-feature-extractor Fix feature_names getter in SequentialFeatureExtractor
|
This needs to deal with a situation when feature_names contains the same strings. |
|
I mean add a test in that situation. |
You can add that test as follows. |
…u2172/chainercv into sequential-feature-extractor
I split PR from #265.