This repository was archived by the owner on Jul 2, 2021. It is now read-only.

Add dilate option and MultiNodeBatchNormalization to Conv2DActiv and Conv2DBNActiv #494

Merged

yuyu2172 merged 37 commits into chainer:master from mitmul:add-conv2dmultinodebnrelu

Mar 25, 2018

Member

mitmul commented Dec 10, 2017 •

edited

Loading

#388 became a too large PR, then I split it into some small PRs.
First, Conv2DActiv and Conv2DBNActive should be able to take dilate option to use DilatedConvolution2D instead of Convolution2D.
Next, Conv2DBNActive should be able to take chainermn.links.MultiNodeBatchNormalization to use batch normalization layers correctly with multiple GPUs.
This PR introduces these two features.

mitmul added 15 commits

December 10, 2017 10:56


          Add dilate option to Conv2DActiv and Conv2DBNActiv, and add enable Mu…

76a529a

…ltiNodeBatchNormalization in Conv2DBNActiv when a communicator is given


          Add tests for dilate option


          Update travis.yml

f760ebf


          Fix .travis.yml

18f0779


          Skip a test if ChainerMN is not installed

3f7f158


          Fix .travis.yml

89bb008


          Fix .travis.yml

a7d8123


          Fix .travis.yml

875a216


          Fix .travis.yml

8f9f63f


          Fix .travis.yml

96857b8


          Fix .travis.yml

f0a8544


          Update tests

80be876


          Update tests

b68099d


          Update tests

14987d2


          Update tests

416839d

mitmul changed the title ~~[WIP] Add dilate option and MultiNodeBatchNormalization to Conv2DActiv and Conv2DBNActiv~~ Add dilate option and MultiNodeBatchNormalization to Conv2DActiv and Conv2DBNActiv

mitmul added 2 commits

December 10, 2017 14:06


          Update tests and travis settings

8fcba84


          Fix flake8 errors

eec207d

mitmul mentioned this pull request

Add PSPNet model #388

Closed

3 tasks

mitmul added 2 commits

December 10, 2017 14:45


          Fix tests

4909c12


          Fix Conv2DBNActiv

f878fb1

Hakuyume reviewed

View reviewed changes

chainercv/links/connection/conv_2d_bn_activ.py Outdated

-                  except for :obj:`activ` and :obj:`bn_kwargs`.
+                  except for :obj:`activ`, :obj:`bn_kwargs`, and :obj:`comm`.
+                  :obj:`comm` is a communicator of ChainerMN which is used for
+                  :obj:`MultiNodeBatchNormalization`. If :obj:`None` is given to the argument

Member

Hakuyume Dec 11, 2017

:class:`chainermn.links.MultiNodeBatchNormalization`

Member Author

mitmul Dec 11, 2017

Fixed

chainercv/links/connection/conv_2d_bn_activ.py Outdated

+                          If a ChainerMN communicator is given,
+                          :obj:`~chainermn.links.MultiNodeBatchNormalization` will be used
+                          for the batch normalization. If :obj:`None`,
+                          :obj:`~chainer.links.BatchNormalization` will be used.

Member

Hakuyume Dec 11, 2017

I prefer to take comm as an element of bn_kwargs because comm is used only for batchnorm.
If 'comm' in bn_kwargs, it use MultiNodeBatchNormalization.

Member Author

mitmul Dec 11, 2017

OK, I changed it


          Follow reviews

9604d3e

Hakuyume reviewed

View reviewed changes

chainercv/links/connection/conv_2d_bn_activ.py Outdated

    
                  :obj:`MultiNodeBatchNormalization`. If :obj:`None` is given to the argument

                  :obj:`comm`, :obj:`BatchNormalization` link from Chainer is used.

                  :class:`chainermn.links.MultiNodeBatchNormalization`. If

                  :obj:`None` is given to the argument :obj:`comm`, :obj:`BatchNormalization` link from Chainer is used.

Member

Hakuyume Dec 11, 2017

:class:`chainer.links.BatchNormalization`

chainercv/links/connection/conv_2d_bn_activ.py Outdated

                               self.bn = MultiNodeBatchNormalization(
-                                  out_channels, comm, **bn_kwargs)
+                                  out_channels, [**bn_kwargs)

Member

Hakuyume Dec 11, 2017

[ ?

chainercv/links/connection/conv_2d_bn_activ.py Outdated

-                          If a ChainerMN communicator is given,
+                          :class:`chainer.links.BatchNormalization`. If a ChainerMN
+                          communicator (:class:`~chainermn.communicators.CommunicatorBase)
+                          is given with the key :obj:`comm`,
                           :obj:`~chainermn.links.MultiNodeBatchNormalization` will be used

Member

Hakuyume Dec 11, 2017

:class:`chainermn.links.MultiNodeBatchNormalization`

mitmul added 3 commits

December 11, 2017 13:26


          Update

bc29554


          Follow review comments

0b0d28f


          Fix flake8 errors

2fe21b8

yuyu2172 reviewed

View reviewed changes

chainercv/links/connection/conv_2d_bn_activ.py Outdated

+              try:
+                  from chainermn.links import MultiNodeBatchNormalization
+                  _chainermn_available = True
+              except (ImportError, TypeError):

Member

yuyu2172 Mar 6, 2018

Does TypeError occur?
If not, we can remove it.

chainercv/links/connection/conv_2d_bn_activ.py Outdated

-                               stride=1, pad=0, nobias=True, initialW=None,
-                               initial_bias=None, activ=relu, bn_kwargs=dict()):
+                               stride=1, pad=0, dilate=1, nobias=True, initialW=None,
+                               initial_bias=None, activ=relu, bn_kwargs=dict(), comm=None):

Member

yuyu2172 Mar 6, 2018

Can you remove comm option?

tests/links_tests/connection_tests/test_conv_2d_bn_activ.py Outdated

+                          self.in_channels, self.out_channels, self.ksize, self.stride,
+                          self.pad, self.dilate, initialW=initialW,
+                          initial_bias=initial_bias, activ=activ, bn_kwargs=bn_kwargs,
+                          comm=comm)

Member

yuyu2172 Mar 6, 2018

Include comm in bn_kwards.

Member

yuyu2172 commented Mar 6, 2018

I am very sorry for late review.
Could you please resolve the conflict with master branch?

yuyu2172 reviewed

View reviewed changes

.travis.yml Outdated

                     conda env create -f environment.yml;
                     source activate chainercv;
+                    cd $HOME;
+                    wget https://github.com/chainer/chainermn/archive/v1.0.0.tar.gz -O chainermn.tar.gz;

Member

yuyu2172 Mar 6, 2018

Please update to the latest release.

Member

yuyu2172 commented Mar 6, 2018

ChainerMN seems to be not installed correctly.

It is raising an error error: option --no-nccl not recognized.

mitmul added 2 commits

March 7, 2018 10:07


          Fix tests and travis.yml

3585df6


          Fix tests

6b46c9b

Member

Hakuyume commented Mar 7, 2018

Why do you install ChainerMN from source? Can't we install it by modifying chainercv/environment.yml?


          Add chainermn to environment.yml

22dd5e1

Member

yuyu2172 commented Mar 7, 2018

Travis is failing because environment file is incorrect.
Perhaps, pip should be used inside the env file to install mn.


          Fix environment.yml

eddfb10

Member

yuyu2172 commented Mar 7, 2018

Conda installation is still failing.

Member

yuyu2172 commented Mar 12, 2018

You need - before pip.

The env file below worked.

name: chainercv
channels:
- !!python/unicode
  'menpo'
- !!python/unicode
  'mpi4py'
- !!python/unicode
  'defaults'
dependencies:
  - Cython
  - opencv3=3.2.0
  - matplotlib
  - numpy
  - Pillow
  - openmpi
  - pip:
    - chainermn==1.2.0


          Fix environment.yml

Member Author

mitmul commented Mar 15, 2018

Thanks. I updated it.

Member

yuyu2172 commented Mar 16, 2018

Tests are failing when dilate == 2.
I think you forgot to modify test_conv_2d_activ.py. It is fine for test_conv_2d_bn_activ.py.

mitmul added 2 commits

March 16, 2018 16:39


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

024d00e

…multinodebnrelu


          Fix tests in test_conv_2d_activ.py

59ce661

Member Author

mitmul commented Mar 16, 2018

@yuyu2172 Sorry, I updated the test too.


          Fix a typo

0132e33

Member Author

mitmul commented Mar 16, 2018

All tests passed.

Member

yuyu2172 commented Mar 19, 2018 •

edited

Loading

I checked the test log and it seems that ChainerMN is not installed properly.

The log says that mpi4py fails to get installed. https://travis-ci.org/chainer/chainercv/jobs/354200398#L751
Could you fix this?

  _configtest.c:2:17: fatal error: mpi.h: No such file or directory
   #include <mpi.h>
                   ^
  compilation terminated.
  failure.
  removing: _configtest.c _configtest.o
  error: Cannot compile MPI programs. Check your configuration!!!
  
  ----------------------------------------
�[31m  Failed building wheel for mpi4py�[0m
�[?25h  Running setup.py clean for mpi4py
  Running setup.py bdist_wheel for pycparser ... �[?25l-� �\� �done
�[?25h  Stored in directory: /home/travis/.cache/pip/wheels/95/14/9a/5e7b9024459d2a6600aaa64e0ba485325aff7a9ac7489db1b6
  Running setup.py bdist_wheel for filelock ... �[?25l-� �done
�[?25h  Stored in directory: /home/travis/.cache/pip/wheels/5f/5e/8a/9f1eb481ffbfff95d5f550570c1dbeff3c1785c8383c12c62b
Successfully built chainermn chainer pycparser filelock
Failed to build mpi4py

As a consequence, the tests related to ChainerMN are skipped.
https://travis-ci.org/chainer/chainercv/jobs/354200398#L1077

Member Author

mitmul commented Mar 19, 2018

I see. Thanks for catching it. I'll fix that.


          Install mpi4py through conda

aa858ea

Member Author

mitmul commented Mar 22, 2018

@yuyu2172 Fixed the environment.yml to install mpi4py from conda. I think it fixed the installation of mpi4py.

mitmul force-pushed the add-conv2dmultinodebnrelu branch from 63a1f02 to 96cd29a Compare

March 22, 2018 09:37


          Update .travis.yml to export LD_LIBRARY_PATH

a9e908d

mitmul force-pushed the add-conv2dmultinodebnrelu branch from 96cd29a to a9e908d Compare

March 22, 2018 09:46

yuyu2172 approved these changes

View reviewed changes

Member

yuyu2172 left a comment

LGTM

yuyu2172 merged commit aee70e6 into chainer:master

yuyu2172 added this to the v0.9 milestone

yuyu2172 added the feature label

yuyu2172 self-assigned this

mitmul deleted the add-conv2dmultinodebnrelu branch

May 18, 2018 09:22

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels