Add PSPNet model #388

mitmul · 2017-08-10T12:24:51Z

Please merge #494 and #392 first.
This PR uses Conv2DBNActiv with a ChainerMN communicator and CityscapesSemanticSegmentationDataset.

This PR adds PSPNet model definition and it's inference demo code.
This also includes convert.py to convert the official caffemodel to Chainer model.

Ensure the convert.py correctly works
Add tests
Add evaluation scripts to ensure the conversion correctness

…-infer

…yuyu2172/chainercv into add-pspnet-infer

… add-pspnet-infer

…-infer

yuyu2172

Initial comment.

yuyu2172 · 2017-10-08T09:06:52Z

chainercv/links/model/pspnet/pspnet.py

+
+        with self.init_scope():
+            self.input_size = input_size
+            self.trunk = DilatedFCN(n_blocks=n_blocks)


Please use extractor instead of trunk.
This is consistent with links such as FasterRCNN.

OK, I changed it.

yuyu2172 · 2017-10-08T09:13:47Z

chainercv/links/model/pspnet/pspnet.py

+        mid_stride = chainer.config.mid_stride
+        super(BottleneckConv, self).__init__()
+        with self.init_scope():
+            self.cbr1 = ConvBNReLU(


I want to make the attribute name of ConvBN* consistent across the library.
We have Conv2DBNActiv, which will be used by ResNet implementations.

Since cbr is specific to Conv->BN->ReLU, I want to avoid the name.
How about conv1, conv2, ... ?

Thanks, I changed the attribute name to conv1, conv2, ...

yuyu2172 · 2017-10-08T09:15:10Z

chainercv/links/model/pspnet/pspnet.py

+        return F.relu(h + x)
+
+
+class ResBlock(chainer.ChainList):


Can you merge ResBlock and DilatedResBlock?

yuyu2172 · 2017-10-08T09:18:07Z

chainercv/links/model/pspnet/pspnet.py

+        h = F.max_pooling_2d(h, 3, 2, 1)  # 1/4
+        h = self.res2(h)
+        h = self.res3(h)  # 1/8
+        if chainer.config.train:


Why not always return h1 and h2?

h1 is only for calculating auxiliary loss. That is completely useless during inference.

predict should be called during inference and not __call__. There is not much inconvenience returning it.

In general, I want to avoid using a global variable to configure behavior if possible.

OK, I changed this part to return both h1 and h2 always.

yuyu2172 · 2017-10-08T09:21:12Z

chainercv/links/model/pspnet/pspnet.py

+        if initialW is None:
+            chainer.config.initialW = chainer.initializers.HeNormal()
+        else:
+            chainer.config.initialW = initialW


I prefer not using global variables.

I replaced all such part to use a local variable given by the argument to the class.

yuyu2172 · 2017-10-08T09:24:18Z

chainercv/links/model/pspnet/pspnet.py

+
+            # To calculate auxirally loss
+            if chainer.config.train:
+                self.cbr_aux = ConvBNReLU(None, 512, 3, 1, 1)


I do not like to change the configuration of a model using global variable.
It would not be much problem to instantiate these links. Why not always instantiate them?

OK, let's instantiate them always.

yuyu2172 · 2017-10-08T09:27:19Z

chainercv/links/model/pspnet/pspnet.py

+                constructor. ``H, W`` is the input image size.
+
+        """
+        if chainer.config.train:


Why not add compute_aux option instead of changing behavior using chainer.config.train?
There can be a situation when a user wants to compute aux with chainer.config.train=False.

Right. I'll change it.

yuyu2172 · 2017-10-08T09:30:23Z

chainercv/links/model/pspnet/pspnet.py

+            img -= self.mean[:, None, None]
+            img = img.astype(np.float32, copy=False)
+            if self._use_pretrained_model:
+                # Pre-trained model is trained for BGR images


Can you flip the order of in_channels of the weights in conversion code?
By doing so, this if statement can be deleted.
I want to make pspnet.py to be as simple as possible at the expense of more complex conversion code because people would rarely see the conversion code.

OK, I'll try to do it.

yuyu2172 · 2017-10-08T09:32:17Z

chainercv/links/model/pspnet/pspnet.py

+        }
+    }
+
+    def __init__(self, n_class=None, input_size=None, n_blocks=None,


How about making PSPNet50 and PSPNet101, which inherit from PSPNet.
In my idea, PSPNet.__init__ would take extractor as input.

I have the designs of FasterRCNN and FasterRCNNVGG16 in my mind.

yuyu2172 · 2017-10-08T09:38:42Z

chainercv/links/model/pspnet/pspnet.py

+
+class ConvBNReLU(chainer.Chain):
+
+    def __init__(self, in_ch, out_ch, ksize, stride=1, pad=1, dilation=1):


Can you make input variables/variable names similar to Conv2DBNActiv?

yuyu2172 · 2017-10-08T09:46:49Z

chainercv/links/model/pspnet/pspnet.py

+try:
+    from chainermn.links import MultiNodeBatchNormalization
+except Exception:
+    warnings.warn('To perform batch normalization with multiple GPUs or '


Instead of raising a warning during import, it should be raised when PSPNet is instantiated.

Please fix this. Please make sure that the error is raised only when comm is specified.

Also, we should install the latest release.

yuyu2172 · 2017-10-09T04:51:21Z

I ran predict and an error was raised.
I used Python2.

  File "/home/yuyu2172/projects/chainercv/chainercv/links/model/pspnet/pspnet.py", line 420, in _tile_predict           
    for yy in six.moves.xrange(hh):                                                                                     
TypeError: integer argument expected, got float

yuyu2172 · 2017-10-09T05:27:11Z

Can you make sure that predict does not alter the value of input images?
Currently, prepare changes the value because the input is not copied.

yuyu2172 · 2018-03-06T03:45:16Z

examples/pspnet/demo.py

+
+if __name__ == '__main__':
+    parser = argparse.ArgumentParser()
+    parser.add_argument('--img_fn', '-f', type=str)


Can you use
parser.add_argument('image')
for parsing image path?
This is the style used by other demos.

yuyu2172 · 2018-03-06T03:46:00Z

examples/pspnet/demo.py

+    parser.add_argument('--img_fn', '-f', type=str)
+    parser.add_argument('--gpu', '-g', type=int, default=-1)
+    parser.add_argument('--scales', '-s', type=float, nargs='*', default=None)
+    parser.add_argument('--out_dir', '-o', type=str, default='.')


Can you change the demo to plot.show instead of plot.savefig?
As a consequence, could you remove this option?

yuyu2172 · 2018-03-06T03:47:04Z

examples/pspnet/demo.py

+    parser.add_argument('--gpu', '-g', type=int, default=-1)
+    parser.add_argument('--scales', '-s', type=float, nargs='*', default=None)
+    parser.add_argument('--out_dir', '-o', type=str, default='.')
+    parser.add_argument('--model', '-m', type=str,


We use model option when mentioning architectures of networks (e.g., SSD512 and SSD300).
Could you use pretrained_model as the name of option?

yuyu2172 · 2018-03-06T03:47:19Z

examples/pspnet/demo.py

+from chainercv.links import PSPNet
+from chainercv.utils import read_image
+from chainercv.visualizations import vis_image
+from chainercv.visualizations import vis_label


The name of the function has been modified to vis_semantic_segmentation.

yuyu2172 · 2018-03-06T03:48:00Z

examples/pspnet/demo.py

+
+    if args.model == 'voc2012':
+        model = PSPNet(pretrained_model='voc2012')
+        labels = voc_semantic_segmentation_label_names


Could you change the name to label_names?
labels is (B, H, W) array in this context.

yuyu2172 · 2018-03-06T03:50:23Z

examples/pspnet/predict.py

+    return pred
+
+
+if __name__ == '__main__':


How about changing the name of the file to predict_cityscapes.

yuyu2172 · 2018-03-06T03:52:20Z

.pep8

@@ -1,3 +1,3 @@
 [pep8]


We should merge .pep8 to setup.cfg because this is how Chainer does it.

yuyu2172 · 2018-03-06T03:52:37Z

setup.cfg

@@ -0,0 +1,3 @@
+[flake8]
+exclude = .eggs,*.egg,build,caffe_pb2.py,caffe_pb3.py,docs,.git
+


Please delete empty line.

yuyu2172 · 2018-03-06T04:00:45Z

chainercv/links/model/pspnet/pspnet.py

+try:
+    from chainermn.links import MultiNodeBatchNormalization
+except Exception:
+    warnings.warn('To perform batch normalization with multiple GPUs or '


Please fix this. Please make sure that the error is raised only when comm is specified.

Also, we should install the latest release.

yuyu2172 · 2018-03-06T04:13:48Z

@Hakuyume @mitmul

What do you think about reusing ResBlock for resnet for PSPNet?
This can be possible once we merge #494.

https://github.com/chainer/chainercv/blob/master/chainercv/links/model/resnet/resblock.py

Hakuyume · 2018-03-06T05:00:29Z

👍 for reusing ResBlock in PSPNet if possible.

mitmul · 2018-05-18T08:00:34Z

@yuyu2172 @Hakuyume Can I restart working on this? What's the situation around this PR?
Just updating this PR by following your reviews is fine?

…-infer

yuyu2172 · 2018-05-21T02:15:18Z

I was actually working on it, but had not yet completed implementations.
I can finish the inference-part by this week. Is that OK?

https://github.com/yuyu2172/chainercv/tree/add-pspnet-infer-major-change

yuyu2172 · 2018-05-22T08:36:56Z

Reopening PR #610

mitmul · 2018-05-22T08:43:44Z

@yuyu2172 Thanks, that's fine. I think it's better to close this PR. Thank you for handing over this.

mitmul added 9 commits August 10, 2017 21:23

Add PSPNet model

0ee2a76

Update PSPNet model

d31b837

Fix convert.py

8c835e3

Fix URLs

005c0ae

Merge branch 'add-cityscapes' into add-pspnet-infer

97f3340

Revert model around auxiliary loss branch

7787729

Update demo.py

163ea83

Update demo.py

13a5ace

Merge branch 'master' of github.com:chainer/chainercv into add-pspnet…

7489085

…-infer

yuyu2172 added the feature label Sep 29, 2017

mitmul added 9 commits October 4, 2017 21:38

Add predict.py

4017ba7

update readme

55a4f0d

Remove download.sh

dbe3c2b

Update readme

07c8594

Merge branch 'cityscapes-label-variable-names' of https://github.com/…

e4b5ea0

…yuyu2172/chainercv into add-pspnet-infer

Merge branch 'add-ade20k' of https://github.com/mitmul/chainercv into…

d40e24f

… add-pspnet-infer

Fix

78ad097

Fix import

fbb8606

flake8

891c68d

mitmul changed the title ~~[WIP] Add PSPNet model~~ Add PSPNet model Oct 4, 2017

mitmul added 4 commits October 5, 2017 00:03

Add setup.cfg

f567ebc

Fix conver.py

1f1c382

Fix flake8

0278ce5

Merge branch 'master' of github.com:chainer/chainercv into add-pspnet…

3f4a0b0

…-infer

yuyu2172 reviewed Oct 8, 2017

View reviewed changes

Use conv1, conv2, ... for the attribute name of ConvBNReLU link

d104b06

mitmul added 3 commits November 16, 2017 19:10

Merge DilatedResBlock and ResBlock

09e196f

Always return h1 and h2 in DilatedFCN's __call__

9dd625b

Follow conventions

a941993

mitmul mentioned this pull request Dec 10, 2017

Add dilate option and MultiNodeBatchNormalization to Conv2DActiv and Conv2DBNActiv #494

Merged

yuyu2172 reviewed Mar 6, 2018

View reviewed changes

yuyu2172 mentioned this pull request Mar 27, 2018

Add dilate and bn_kwargs to Bottleneck and ResBlock #549

Merged

Hakuyume added this to Stall in ChainerCV May 18, 2018

Merge branch 'master' of github.com:chainer/chainercv into add-pspnet…

9a8d630

…-infer

yuyu2172 mentioned this pull request May 22, 2018

Add inference part of PSPNet #610

Merged

mitmul closed this May 22, 2018

yuyu2172 removed this from Stall in ChainerCV May 22, 2018


		class ConvBNReLU(chainer.Chain):

		def __init__(self, in_ch, out_ch, ksize, stride=1, pad=1, dilation=1):

		@@ -0,0 +1,3 @@
		[flake8]
		exclude = .eggs,*.egg,build,caffe_pb2.py,caffe_pb3.py,docs,.git

Add PSPNet model #388

Add PSPNet model #388

Conversation

mitmul commented Aug 10, 2017 • edited Loading

yuyu2172 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuyu2172 commented Oct 9, 2017 • edited Loading

yuyu2172 commented Oct 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuyu2172 commented Mar 6, 2018

Hakuyume commented Mar 6, 2018

mitmul commented May 18, 2018 • edited Loading

yuyu2172 commented May 21, 2018

yuyu2172 commented May 22, 2018

mitmul commented May 22, 2018

mitmul commented Aug 10, 2017 •

edited

Loading

yuyu2172 commented Oct 9, 2017 •

edited

Loading

mitmul commented May 18, 2018 •

edited

Loading