-
Notifications
You must be signed in to change notification settings - Fork 304
Conversation
self.fc7 = L.Linear(4096, 4096, **kwargs) | ||
self.fc8 = L.Linear(4096, 1000, **kwargs) | ||
|
||
self.functions = collections.OrderedDict([ |
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 do not recommend this style of network definition. It might cause problems when an instance of this class is copied.
(See chainer/chainer#2810 .)
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 didn't know that! Thank you!
I am trying to reproduce the evaluation score reported here. Currently, my implementation scored 32%, and I am going to investigate the reason why it scored below the reported score. EDIT: EDIT2: NOTE: NOTE: |
cfcccc7
to
2d8eafc
Compare
2d8eafc
to
d29172e
Compare
Merged master |
@@ -74,7 +73,8 @@ class FasterRCNNVGG16(FasterRCNN): | |||
'voc07': { | |||
'n_fg_class': 20, | |||
'url': 'https://github.com/yuyu2172/share-weights/releases/' | |||
'download/0.0.3/faster_rcnn_vgg16_voc07_2017_06_06.npz' | |||
'download/0.0.4/' | |||
'faster_rcnn_vgg16_voc07_trained_2017_08_06_trial_4.npz' |
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.
Why do you use trial_4
? Is this the best model?
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 is the model that is converted from faster_rcnn_vgg16_voc07_2017_06_06.npz
.
It performs the same with the previously distributed model.
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.
Perhaps, faster_rcnn_vgg16_voc07_2017_08_06.npz
is better. User will think "what is trial_4?" like me.
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. Thanks for you feedback.
chainercv/links/model/vgg/vgg16.py
Outdated
|
||
class VGG16(SequentialFeatureExtractor): | ||
|
||
"""VGG16 Network for classification and feature extraction. |
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.
VGG16
-> VGG-16
chainercv/links/model/vgg/vgg16.py
Outdated
"""VGG16 Network for classification and feature extraction. | ||
|
||
This is a feature extraction model. | ||
The network can choose to output features from set of all |
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.
to output features
-> features to output
or output features
?
if pretrained_model in self._models: | ||
mean = self._models[pretrained_model]['mean'] | ||
else: | ||
mean = _imagenet_mean |
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 adding a note about these behaviours? The default values of n_class
and mean
.
chainercv/links/model/vgg/vgg16.py
Outdated
>>> prob = model(imgs) | ||
|
||
>>> model.feature_names = 'conv5_3' | ||
# This is feature conv5_3. |
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 # This is feature conv5_3 (after ReLU).
?
chainercv/links/model/vgg/vgg16.py
Outdated
Examples: | ||
|
||
>>> model = VGG16() | ||
# By default, VGG16.__call__ returns a probability score. |
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 a probability score (after Softmax)
?
chainercv/links/model/vgg/vgg16.py
Outdated
|
||
>>> model.feature_names = ['conv5_3', 'fc6'] | ||
>>> # These are features conv5_3 and fc6. | ||
>>> feat5_3, feat6 = model(imgs) |
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.
# These are features conv5_3 (after ReLU) and fc6 (before ReLU).
docs/source/reference/links.rst
Outdated
|
||
Feature Extraction | ||
~~~~~~~~~~~~~~~~~~ | ||
Feature extraction models can be used to extract feature(s) given images. |
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.
extract feature(s) from given images.
?
examples/classification/README.md
Outdated
|
||
2. Extract the training data: | ||
```bash | ||
mkdir train && mv ILSVRC2012_img_train.tar train/ && cd train |
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.
$ mkdir ...
@@ -0,0 +1,37 @@ | |||
import chainer |
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.
Considering consistency with other examples, this file should be put under examples/vgg/
.
And it is better to convert from .caffemodel
.
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 think that it is OK to leave conversion code under the current directory. This is because unlike other examples, VGG does not have its own
demo.py
ortrain.py
. I think it better to keepexamples/
with small number of subdirectories. - OK. I will prepare write a script to convert directly from
.caffemodel
.
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.
Putting a model-specific utility under task-specific directory seems against your intention. The conversion code for VGG-16 can be used for other task than classification.
This is because unlike other examples, VGG does not have its own demo.py or train.py
You won't add any training codes for VGG or other classification networks?
Anyway, we should keep consistency with other examples. If you want to reduce the number of subdirectories, how about putting detection models under detection
?
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.
Putting a model-specific utility under task-specific directory seems against your intention. The conversion code for VGG-16 can be used for other task than classification.
OK. Your suggestion seems easier for users who are not interested in classification to access the conversion code.
If you want to reduce the number of subdirectories, how about putting detection models under detection?
The current layout of directories are easier to explore than the nested directories.
I think it is still manageable.
If things start to get out of control, we can write a README under examples/
.
examples/vgg/caffee2npz_vgg_16.py
Outdated
@@ -0,0 +1,47 @@ | |||
import argparse |
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.
You can make this code more general. Please check SSD's conversion code.
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.
What do you mean more general?
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 can take other VGG model (e.g. VGG-19).
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.
Is there a caffemodel for VGG-19?
examples/vgg/caffee2npz_vgg_16.py
Outdated
# The pretrained weights are trained to accept BGR images. | ||
# Convert weights so that they accept RGB images. | ||
model.conv1_1.conv.W.data[:] = model.conv1_1.conv.W.data[:, ::-1] | ||
model.conv1_2.conv.copyparams(caffemodel.conv1_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.
These copyparams
can be reduced.
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?
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.
We can save CaffeFunction
directly. Why do you copy the paramerters from CaffeFunction
to VGG16
?
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.
Because VGG16
uses Conv2DActiv
, which is not used in the caffemodel
.
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 this yuyu2172#4 ?
9d1e8ee
to
0dfddfe
Compare
Improve conversion code of VGG
This PR is no-compat because the child links of the FasterRCNN has changed. |
examples/vgg/README.md
Outdated
Convert `*.caffemodel` to `*.npz`. | ||
|
||
``` | ||
$ python caff2npz_vgg_16.py <source>.caffemodel <target>.npz |
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.
Sorry, I forgot to change this line. Please update caff2npz_vgg_16.py
-> caffe2npz.py
.
examples/vgg/README.md
Outdated
@@ -6,7 +6,7 @@ For evaluation, please go to [`examples/classification`](https://github.com/chai | |||
Convert `*.caffemodel` to `*.npz`. | |||
|
|||
``` | |||
$ python caff2npz_vgg_16.py <source>.caffemodel <target>.npz | |||
$ python caff2npz.py <source>.caffemodel <target>.npz |
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.
Typo. caff
-> caffe
.
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
Merge after #271.
This PR adds a VGG16Layers, which will have APIs consistent with rest of the links in ChainerCV.
numpy.array
.__init__
(users can manually select to stop using random initializer)__call__
does not havelayers
option. Also, the return value is not a dictionary but achainer.Variable
.__init__
takesfeature
option, which selects the feature that is going to be returned by__call__
.feature
option, layers that are unnecessary will automatically be deleted.These changes are made for the following concrete scenarios in mind.