add FCIS ResNet101 #568
add FCIS ResNet101 #568
Conversation
knorth55
force-pushed
the
experimental-fcis
branch
from
0232a41 to
0bb0cef
Compare
knorth55
force-pushed
the
experimental-fcis
branch
from
0bb0cef to
bb7e2d4
Compare
|
|
||
| class ResNet101Extractor(chainer.Chain): | ||
|
|
||
| def __init__(self): |
There was a problem hiding this comment.
Can you change the feature extractor to take initialW as an argument, and pass the zero-initializer when a pretrained model is specified?
|
|
||
| def __call__(self, x): | ||
| with chainer.using_config('train', False): | ||
| with chainer.function.no_backprop_mode(): |
There was a problem hiding this comment.
I prefer setting related backpropagation in the training code.
Can you remove with chainer.function.no_backprop_mode from here?
There was a problem hiding this comment.
Hmm, in that case, I cannot reuse this model in training code...
There was a problem hiding this comment.
You can use disable_update to not update weights.
There was a problem hiding this comment.
So, you prefer setting disable_update() by hand in training code?
OK, i will do that.
In fact, no_backprop_mode and disable_update are confusing me many times.
| self.psroi_conv2 = L.Convolution2D( | ||
| 1024, group_size * group_size * n_class * 2, | ||
| 1, 1, 0, initialW=initialW) | ||
| self.psroi_conv3 = L.Convolution2D( |
There was a problem hiding this comment.
How about changing the name of the links to conv1, cls_seg and ag_loc?
This naming convention looks as if three convolutions are applied sequentially to the input.
Alternatively, psroi_conv1, psroi_cls_seg, psroi_ag_loc is OK (I prefer the first choice for simplicity).
| def __call__(self, x, rois, roi_indices, img_size): | ||
| h = F.relu(self.psroi_conv1(x)) | ||
| h_cls_seg = self.psroi_conv2(h) | ||
| h_ag_locs = self.psroi_conv3(h) |
There was a problem hiding this comment.
How about h_ag_loc instead of h_ag_locs.
We usually do not put s with variables starting with h_ (e.g.., not hs).
Also, this looks more consistent with h_cls_seg, which you did not put s.
| initialW = chainer.initializers.Normal(0.01) | ||
| with self.init_scope(): | ||
| self.psroi_conv1 = L.Convolution2D( | ||
| 2048, 1024, 1, 1, 0, initialW=initialW) |
There was a problem hiding this comment.
hmmm, i'm don't see any merits of using Conv2DActiv...
It is easy to use default one for me.
There was a problem hiding this comment.
OK. I think this is just a matter of preference. Please leave this as is.
| roi_ag_locs = roi_ag_locs.reshape((n_roi, 2, 4)) | ||
|
|
||
| # Mask Regression | ||
| # shape: (n_rois, n_class, 2, roi_size, roi_size) |
| # shape: (n_rois, n_class, 2, roi_size, roi_size) | ||
| # Group Pick by Score | ||
| max_cls_indices = roi_cls_scores.array.argmax(axis=1) | ||
| # shape: (n_rois, 2, roi_size, roi_size) |
|
|
||
|
|
||
| def _global_average_pooling_2d(x): | ||
| n_rois, n_channel, H, W = x.array.shape |
| # Group Max | ||
| # shape: (n_rois, n_class, roi_size, roi_size) | ||
| h_cls = pool_cls_seg.transpose((0, 1, 3, 4, 2)) | ||
| h_cls = F.max(h_cls, axis=4) |
There was a problem hiding this comment.
We can remvoe transpose by using the right axis when taking maximum.
| max_cls_indices = roi_cls_scores.array.argmax(axis=1) | ||
| # shape: (n_rois, 2, roi_size, roi_size) | ||
| roi_seg_scores = pool_cls_seg[ | ||
| np.arange(len(max_cls_indices)), max_cls_indices] |
There was a problem hiding this comment.
Use self.xp.arange.
This is faster because np.arange need to transfer data from CPU to GPU.
knorth55
force-pushed
the
experimental-fcis
branch
from
017b6b5 to
a860287
Compare
| self, h_cls_seg, h_ag_locs, rois, roi_indices): | ||
| # PSROI Pooling | ||
| # shape: (n_rois, n_class*2, roi_size, roi_size) | ||
| pool_cls_seg = psroi_pooling_2d( |
There was a problem hiding this comment.
May I suggest to use roi_seg_cls_scores instead of pool_cls_seg?
roi_seg_scores is (n_roi, 2, R, R) and roi_cls_scores is(n_roi, n_class).
For a variable with shape (n_roi, n_class, 2, R, R), I think roi_seg_cls_scores is the appropriate name.
Basically, _cls_ indicates that we have extra dimension that corresponds to categories.
| pool_cls_seg = pool_cls_seg.reshape( | ||
| (-1, self.n_class, 2, self.roi_size, self.roi_size)) | ||
| # shape: (n_rois, 2*4, roi_size, roi_size) | ||
| pool_ag_locs = psroi_pooling_2d( |
There was a problem hiding this comment.
May I suggest to name (n_roi, 2 * 4, R, R) as roi_seg_ag_locs instead of pool_ag_locs?
This seems like an consistent usage of _seg_, which is used for all variables with (n_roi, *, R, R).
|
|
||
| _models = { | ||
| 'sbd': { | ||
| 'n_fg_class': 20, |
There was a problem hiding this comment.
Is setting the name of the pretrained weight voc inappropriate?
There was a problem hiding this comment.
VOC images with SBD annotations, so I prefer SBD.
Also, the model is trained with SBD training image list, which differs from VOC train image list.
| 'sbd': { | ||
| 'n_fg_class': 20, | ||
| 'url': 'https://github.com/yuyu2172/share-weights/releases/' | ||
| 'download/0.0.6/fcis_resnet101_2018_04_14.npz' |
There was a problem hiding this comment.
Could you name the weight in the following protocol?
fcis_resnet101_DATASETNAME_trained_DATE.npz?
(e.g., faster_rcnn_vgg16_voc07_trained_2017_08_06.npz).
knorth55
force-pushed
the
experimental-fcis
branch
3 times, most recently
from
e8b2522 to
5899fdd
Compare
|
You haven't followed the comments? #568 (comment) #568 (comment) |
|
|
||
|
|
||
| def mask_voting( | ||
| mask_score, bbox, score, size, |
There was a problem hiding this comment.
I have two comments on the names of variables.
First, if the range of the value is [0, 1], use _prob instead of _score.
Second, the name mask_score can be misunderstood as an array with shape (R, H, W). How about naming a variable (R, RH, RW) as roi_mask_prob. (depending on the range of its values). This name is consistent with names in fcis.py.
Also, how about renaming bbox to roi so that it is clear that roi_mask_prob is defined in the region of roi.
When iterating over roi, either for bb in roi: or for r in roi is OK.
In addition to that, score can be renamed to roi_prob to make its shape more precise (this modification is only optional).
There was a problem hiding this comment.
You actually do use something similar to what I suggested in test_mask_voting.py.
There was a problem hiding this comment.
First, if the range of the value is [0, 1], use _prob instead of _score.
I look in some chainercv codes, and score and prob are mixed.
For example, object_detection returns scores, which is probability.
In order to clean up, we first set the rule first.
Second, the name mask_score can be misunderstood as an array with shape (R, H, W). How about naming a variable (R, RH, RW) as roi_mask_prob. (depending on the range of its values). This name is consistent with names in fcis.py.
Also, how about renaming bbox to roi so that it is clear that roi_mask_prob is defined in the region of roi.
OK, i will fix it.
There was a problem hiding this comment.
I look in some chainercv codes, and score and prob are mixed.
For example, object_detection returns scores, which is probability.
In order to clean up, we first set the rule first.
I missed this comment. sorry.
There was a problem hiding this comment.
We used scores because we wanted to relax the interface of detection models.
Hmm... I totally forgot this...
You can leave this part in the way you like for now. We can later discuss the naming conventions. I personally think prob is a lot more readable (that is probably why Faster R-CNN sometimes uses *_prob).
There was a problem hiding this comment.
I totally agree that prob is easier to understand.
But, for API, score is consistent to object detection API, so I will use scores :(.
There was a problem hiding this comment.
You can consistently use prob up until the end of prediction.
In the end, you do scores.append(prob).
There was a problem hiding this comment.
OK, that sounds good
| cls_score.append(score_l) | ||
|
|
||
| sorted_score = np.sort(np.concatenate(cls_score))[::-1] | ||
| keep_n = min(len(sorted_score), limit) |
|
|
||
| roi_seg_score = roi_seg_scores.array | ||
| roi_score = roi_scores.array | ||
| bbox = rois / scale |
There was a problem hiding this comment.
How about not introducing bbox.
I mean use roi = rois / scale.
The rationale to prefer roi is explained in the second suggestion of #568 (comment).
There was a problem hiding this comment.
roi and bbox is complicated, too.
In this case, rois = bboxes so it doesn't matter.
What's wrong with it?
There was a problem hiding this comment.
By definition, rois and bboxes are different (bboxes is a list of bbox).
But, as you said, in the case when batchsize=1, rois = bboxes.
Having said that, I think that continuing to use rois in predict is better because it explicitly indicates that roi_mask_prob is defined relative to rois.
This is the same reason why we use rois elsewhere, such as __call__.
There was a problem hiding this comment.
I don't see the difference of bboxes and rois.
In my understanding, bboxes and rois have same shape (R, 4).
In FCIS, we don't use roi_ag_locs (because it is already applied in __call__ with iter2=True) so that roi is same as bbox.
For the return output, the predict should return bbox as output not roi, so that I rename here.
There was a problem hiding this comment.
No. bboxes is a list of (R, 4), whereas rois is (R', 4).
You mean bbox is after non maximum suppression and other processing?
In that case, this rois are already bboxes.
There was a problem hiding this comment.
roi_mask are complete different from other roi_*.
What you want is removing other roi_* variables. Maybe i understand.
In that case, the rule below sounds good to me
roi_mask -> roi_mask (no change)
rois -> bboxes
roi_seg_scores -> seg_scores
roi_scores -> scores
There was a problem hiding this comment.
What I was saying is this.
roi_mask -> roimask
rois -> bboxes
roi_seg_scores -> seg_scores
roi_scores -> scores
There was a problem hiding this comment.
You mean bbox is after non maximum suppression and other processing?
In that case, this rois are already bboxes.
No no. The types are different.
There was a problem hiding this comment.
No no. The types are different.
Types are different? Can you tell me more detailed information?
roi_mask -> roimask
And I prefer roi_mask to roimask because it is easy to read.
| bbox[:, 0::2] = self.xp.clip(bbox[:, 0::2], 0, size[0]) | ||
| bbox[:, 1::2] = self.xp.clip(bbox[:, 1::2], 0, size[1]) | ||
|
|
||
| roi_seg_prob = F.softmax(roi_seg_score).array |
There was a problem hiding this comment.
How about roi_seg_prob = F.softmax(roi_seg_score).array[:, 1, :, :] so that the naming convention is consistent with mask_voting.py.
| roi_prob = chainer.cuda.to_cpu(roi_prob) | ||
| bbox = chainer.cuda.to_cpu(bbox) | ||
|
|
||
| roi_mask_score, bbox, label, score = mask_voting( |
There was a problem hiding this comment.
Isn't the returned mask a probability?
In that case, can you rename roi_mask_score to roi_mask_prob.
| # concat 1st and 2nd iteration results | ||
| rois = self.xp.concatenate((rois, rois2)) | ||
| roi_indices = self.xp.concatenate((roi_indices, roi_indices)) | ||
| roi_seg_scores = F.concat( |
There was a problem hiding this comment.
Do you have any particular reason to prefer using *_seg_* for names?
I think it is the same as *_mask_*, so how about using only one convention.
Since mask is used for the output, how about choosing *_mask_*.
| roi_ag_locs = roi_ag_locs.array | ||
| mean = self.xp.array(self.loc_normalize_mean) | ||
| std = self.xp.array(self.loc_normalize_std) | ||
| roi_loc = roi_ag_locs[:, 1, :] |
| scale = img_var.shape[3] / size[1] | ||
| roi_seg_scores, _, roi_scores, rois, _ = \ | ||
| self.__call__(img_var, scale) | ||
|
|
There was a problem hiding this comment.
| 2. **Region Proposal Networks**: Given the feature maps calculated in \ | ||
| the previous stage, produce set of RoIs around objects. | ||
| 3. **Localization, Segmentation and Classification Heads**: Using feature \ | ||
| maps that belong to the proposed RoIs, segment region of the objects, \ |
There was a problem hiding this comment.
segment region --> segment regions
| :class:`chainer.Chain` objects :obj:`feature`, :obj:`rpn` and :obj:`head`. | ||
| There are two functions :meth:`predict` and :meth:`__call__` to conduct | ||
| instance segmentation. | ||
| :meth:`predict` takes images and returns masks, object label and its score. |
There was a problem hiding this comment.
object labels and their scores
| Please refer to the documentation found there. | ||
| head (callable Chain): A callable that takes a BCHW array, | ||
| RoIs and batch indices for RoIs. | ||
| This returns class dependent segmentation scores, class-agnostic |
There was a problem hiding this comment.
class agnostic segmentation scores ?
| indices for RoIs. | ||
| mean (numpy.ndarray): A value to be subtracted from an image | ||
| in :meth:`prepare`. | ||
| min_size (int): A preprocessing paramter for :meth:`prepare`. Please |
| in :meth:`prepare`. | ||
| min_size (int): A preprocessing paramter for :meth:`prepare`. Please | ||
| refer to a docstring found for :meth:`prepare`. | ||
| max_size (int): A preprocessing paramter for :meth:`prepare`. |
|
|
||
| Args: | ||
| mask_scores (array): A mask score array whose shape is | ||
| :math:`(R, RH, RW)`. |
There was a problem hiding this comment.
forgetting the second argument
| * :math:`RW` is the height of pooled image. | ||
|
|
||
| Args: | ||
| mask_scores (array): A mask score array whose shape is |
| Args: | ||
| mask_scores (array): A mask score array whose shape is | ||
| :math:`(R, RH, RW)`. | ||
| scores (array): A class score array whose shape is |
| for mask merging. | ||
| binary_thresh (float): A threshold value of mask score | ||
| for mask merging. | ||
| limit (int): A limit number of outputs. |
There was a problem hiding this comment.
A limit number of outputs --> The maximum number of outputs.
| binary_thresh (float): A threshold value of mask score | ||
| for mask merging. | ||
| limit (int): A limit number of outputs. | ||
| bg_label (int): A background label. |
There was a problem hiding this comment.
The id of the background label.
|
|
||
|
|
||
| class TestFCISResNet101Pretrained(unittest.TestCase): | ||
|
|
There was a problem hiding this comment.
Please add @attr.disk like other dataset tests.
There was a problem hiding this comment.
The same test of FasterRCNNVGG16 and SSDVGG16 does not have @attr.disk.
Is it needed?
https://github.com/chainer/chainercv/blob/master/tests/links_tests/model_tests/faster_rcnn_tests/test_faster_rcnn_vgg.py#L17
https://github.com/chainer/chainercv/blob/master/tests/links_tests/model_tests/ssd_tests/test_ssd_vgg16.py#L56
There was a problem hiding this comment.
That is because they don't use pretrained models.
There was a problem hiding this comment.
SSD uses pretrained model!
| def test_pretrained(self): | ||
| link = FCISResNet101(pretrained_model='sbd') | ||
| self.assertIsInstance(link, FCIS) | ||
|
|
Because I found better solution.
it is nonsense to add
|
This variable has shape Also, this is not
OK. I am fine with how you order |
i still cannot understand why we need
This variable is a kind of scores.
Maybe you misunderstand something, but |
OK. Maybe I am misunderstanding something. Having said that, what do you think about stop using |
|
At this point, I want to clarify the difference of |
|
And also, another question. |
|
knorth55
force-pushed
the
experimental-fcis
branch
2 times, most recently
from
11af374 to
0db468d
Compare
|
I updated Variable names as below. |
knorth55
force-pushed
the
experimental-fcis
branch
from
0db468d to
d849730
Compare
|
@knorth55 I think we don't need |
|
I consider pretrained weight to require a huge disk space (but the notion of huge disk is subjective). |
Assume that we have two images, and two sets of bounding boxes are assigned to them. |
For Inside |
Yes. The definition of |
|
|
||
| """ | ||
| h = F.relu(self.conv1(x)) | ||
| h_cls_seg = self.cls_seg(h) |
There was a problem hiding this comment.
cls_seg --> seg_score and h_cls_seg --> h_seg_score
| return roi_mask_scores, ag_locs, cls_scores, rois, roi_indices | ||
|
|
||
| def _pool( | ||
| self, h_cls_seg, h_ag_loc, rois, roi_indices): |
I see. |
Thinking of training code, we should define the variable name now. |
More precisely, |
|
Edit: |
| if len(y_indices) == 0 or len(x_indices) == 0: | ||
| return None, None | ||
| else: | ||
| y_max = y_indices.max() + 1 |
| assert bbox.shape[0] == len(mask_prob) | ||
| assert bbox.shape[0] == mask_weight.shape[0] | ||
|
|
||
| mask = np.zeros(size, dtype=np.float32) |
| x_max = x_indices.max() + 1 | ||
| x_min = x_indices.min() | ||
|
|
||
| c_bbox = np.array( |
| mask_weight = mask_weight / mask_weight.sum() | ||
| mask_prob_i = roi_mask_prob[keep_indices] | ||
| bbox_i = bbox[keep_indices] | ||
| c_mask, c_bbox = _mask_aggregation( |
|
This PR is pretty big, and implementation is good except for some naming conventions. Related: |
Merge after #560
mask_votingFCISFCISResNet101cd chainercv/examples/fcis wget https://raw.githubusercontent.com/knorth55/chainer-fcis/master/examples/voc/images/SBD_test2008_000090.jpg python demo.py SBD_test2008_000090.jpg --gpu 0