-
Notifications
You must be signed in to change notification settings - Fork 306
Conversation
I did a quick survey on various architectures that are called ResNet.
NamesI think naming the ResNet class as
|
ed0f51c
to
7b86cea
Compare
The third model is called |
Thanks. I added that information to the summary. |
Can you briefly take a look at Note that the uploaded pretrained weight would not work with the current organization of weights. |
|
@Hakuyume |
@yuyu2172 OK, I'll check. Please fix the coding style first. |
Oh. Sorry about that. |
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.
First comments
nobias=True) | ||
self.conv3 = Conv2DBNActiv(mid_channels, out_channels, 1, 1, 0, | ||
initialW=initialW, nobias=True, | ||
activ=lambda x: x) |
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.
Calling Caonv2DBActiv
without activ looks tricky. How about adding Conv2DBN
? (It is ok to make it a private class)
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.
Hmm...
I think the current version is fine, but alternatively I can explicitly use conv
and bn
.
I do not like the idea of adding a private class. This usage comes up a lot.
I am less hesitant on the idea of adding Conv2DBN
, but personally it looks redundant given that this is just a Conv2DBNActiv
with no activation....
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.
If you don't like adding a private class, using Conv2D
+ BatchNormalization
is better.
Another solution is adding active='no'
option. to Conv2DBNActiv
.
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.
OK. I like the second idea.
How about setting the name of the special string to identity
?
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.
Since the default value of active
is chainer.functions.relu
(https://github.com/chainer/chainercv/blob/master/chainercv/links/connection/conv_2d_bn_activ.py#L65), we can use active=None
for no activation.
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.
OK
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.
updated
stride (int or tuple of ints): Stride of filter application. | ||
initialW (4-D array): Initial weight value used in | ||
the convolutional layers. | ||
conv_shortcut (bool): If :obj:`True`, apply a 1x1 convolution |
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.
How about residual_conv
? I prefer residual
to shortcut
. Using both residual
and shortcut
is confusing and residual
is more common.
|
||
class ResNet(PickableSequentialChain): | ||
|
||
"""Base class for ResNet Network. |
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.
ResNet architecture
is better because ResNet Network
is Residual Network Network
.
|
||
"""Base class for ResNet Network. | ||
|
||
This is a feature extraction link. |
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.
pickable sequential link
? We have not defined feature extraction link
officially.
This is only supported when :obj:`arch=='he'`. | ||
|
||
Args: | ||
model_name (str): Name of the resnet model to instantiate. |
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.
How about n_layer
? It takes one of 50, 101, 152
as integer. model_name
is difficult to understand.
the mean value used to train the pretrained model is used. | ||
Otherwise, the mean value calculated from ILSVRC 2012 dataset | ||
is used. | ||
initialW (callable): Initializer for the weights. |
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.
weights for convolution kernels
?
docs/source/reference/links.rst
Outdated
@@ -20,6 +20,7 @@ Feature extraction links extract feature(s) from given images. | |||
.. toctree:: | |||
|
|||
links/vgg | |||
links/resnet |
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.
alphabetical order
"""A bottleneck layer. | ||
|
||
Args: | ||
in_channels (int): The number of channels of input arrays. |
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.
input arrays
-> (the) input array
? From my understanding, this link takes only one array.
Args: | ||
in_channels (int): The number of channels of input arrays. | ||
mid_channels (int): The number of channels of intermediate arrays. | ||
out_channels (int): The number of channels of output arrays. |
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.
ditto.
examples/classification/README.md
Outdated
| VGG16 | 27.1 % | | | ||
| ResNet50 | 23.0 % | 22.9 % [2] | | ||
| ResNet101 |21.8 % | 21.8 % [2] | | ||
| ResNet152 |21.4 % | 21.4 % [2] | |
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.
| 21.4 %
(space after |
).
Thanks. |
for name in self._forward: | ||
l = getattr(self, name) | ||
x = l(x) | ||
return x |
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.
I prefer using PickableSequentialChain
to managing _forward
manually. Although the pickable feature is not used, it will be more simple.
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.
LGTM
@yuyu2172 I have a question. Some repositories like mxnet and https://github.com/KaimingHe/deep-residual-networks, https://github.com/KaimingHe/deep-residual-networks/blob/master/prototxt/ResNet-101-deploy.prototxt#L18 |
Merge after #265 (DONE)
EDIT:
merge after #427 (DONE)