format on latest dev

[dcslin]

No description provided.

[dcslin]

command used on ubuntu container
yapf 0.29.0
clang-format version 3.8.0-2ubuntu4 (tags/RELEASE_380/final)

# format cpp code by clang-format
find src/api/ \
    src/core/ \
    src/proto/ \
    src/utils/ \
    include/singa/core/ \
    include/singa/utils/ \
    test/singa/ -iname *.h -o -iname *.cc | xargs clang-format -i

# format python code by yapf
find python/ \
    examples/autograd \
    test/python/ -iname *.py | xargs yapf -i

[dcslin]

testing on other OS..

[chrishkchris]

command used on ubuntu container
yapf 0.29.0
clang-format version 3.8.0-2ubuntu4 (tags/RELEASE_380/final)

format cpp code by clang-format

find src/api/
src/core/
src/proto/
src/utils/
include/singa/core/
include/singa/utils/
test/singa/ -iname *.h -o -iname *.cc | xargs clang-format -i

format python code by yapf

find python/
examples/autograd
test/python/ -iname *.py | xargs yapf -i

@shicong this commands can detect the project root config file which indicates google style?

[dcslin]

No ready for merge

[dcslin]

indeed there are difference between versions on different os

[chrishkchris]

indeed there are difference between versions on different os

I guess maybe because the formatter are installed by pip, and that the versions of the formatters are different in different os?

[chrishkchris]

@dcslin Oh, the license headers are updated in this PR. Last time when I updated the format of the distributed module the license header was not updated (PR #595), so my formatter may also be different as you.

[dcslin]

Hi After sync-ing all the versions between different OS

# yapf --version
yapf 0.29.0
# clang-format --version
clang-format version 9.0.0 (https://github.com/mgevaert/clang-format-wheel d3a647c305759f37f30257352e621579f53d0073)

the result is almost constistent except one small code snippet, yapf on ubuntu gives left result, while osx and windows give right result. (same yapf version and config)
4739c6e

Kindly let me know any comment and it is helpful to test at your side. thanks

[chrishkchris]

@dcslin Oh, the license headers are updated in this PR. Last time when I updated the format of the distributed module the license header was not updated (PR #595), so my formatter may also be different as you.

Oh, I see, my formator version was:

C:\Users\dcsysh>yapf --version
yapf 0.29.0

C:\Users\dcsysh>clang-format --version
clang-format version 3.7.0 (tags/RELEASE_370/final)

I think I will need to upgrade my formatter to 9.0

[dcslin]

@chrishkchris
FYI
https://prereleases.llvm.org/win-snapshots/LLVM-11.0.0-2663a25f-win64.exe
https://llvm.org/builds/

[chrishkchris]

Hi After sync-ing all the versions between different OS

yapf --version

yapf 0.29.0

clang-format --version

clang-format version 9.0.0 (https://github.com/mgevaert/clang-format-wheel d3a647c305759f37f30257352e621579f53d0073)

the result is almost constistent except one small code snippet, yapf on ubuntu gives left result, while osx and windows give right result. (same yapf version and config)
4739c6e
Kindly let me know any comment and it is helpful to test at your side. thanks

@joddiy I think your formatter should be upgraded to this version, so all the versions we use will be the same

[chrishkchris]

@chrishkchris
FYI
https://prereleases.llvm.org/win-snapshots/LLVM-11.0.0-2663a25f-win64.exe
https://llvm.org/builds/

@dcslin Thanks for your information.
I have upgraded the version of LLVM to 9.0, and now the version of clang-formatter raised to 9.0
download page for LLVM 9.0: http://releases.llvm.org/download.html#9.0.1

C:\Users\dcsysh>clang-format --version
clang-format version 9.0.0 (tags/RELEASE_900/final)

C:\Users\dcsysh>yapf --version
yapf 0.29.0

@joddiy @nudles I think we may use the same version as shicong did to get the same format, otherwise will lead to lots of github conflicts when everyone format differently

[nudles]

agree!
Does this PR fix this version issue?

[chrishkchris]

agree!
Does this PR fix this version issue?

@nudles
The version of clang-formatter depends on the version of LLVM we installed in our OS, So all the developer should have the same version of LLVM installed on their computer to make the version of clang-formatter the same. This PR only use the clang-formatter 9.0 to format the current singa code.
@dcslin please correct me if I am wrong

[chrishkchris]

agree!
Does this PR fix this version issue?

@nudles
The version of clang-formatter depends on the version of LLVM we installed in our OS, So all the developer should have the same version of LLVM installed on their computer to make the version of clang-formatter the same. This PR only use the clang-formatter 9.0 to format the current singa code.
@dcslin please correct me if I am wrong

download link of LLVM 9.0 to provide clang-format version 9.0.0
http://releases.llvm.org/download.html#9.0.1

[chrishkchris]

agree!
Does this PR fix this version issue?
@nudles
The version of clang-formatter depends on the version of LLVM we installed in our OS, So all the developer should have the same version of LLVM installed on their computer to make the version of clang-formatter the same. This PR only use the clang-formatter 9.0 to format the current singa code.
@dcslin please correct me if I am wrong

download link of LLVM 9.0 to provide clang-format version 9.0.0
http://releases.llvm.org/download.html#9.0.1

so to fix the formatting issue I suggest we may update the instruction in sing-doc how to reproduce the same formatting

[nudles]

Seems this PR does not need any modification?
so, let's all use version 9.0.
and update the instructions to indicate llvm 9.0.

[chrishkchris]

@dcslin The nccl communicator is inside src/io , why it is not included for formatting?

[nudles]

then pls add the nccl related files.
any other files missed?

[chrishkchris]

@dcslin @nudles
Also, in additional to nccl, the src/model/operation needed to be included which contains conv, pool, and batchnorm used by autograd. Meanwhile, I assume those other folders such as src/model/layer are the old optimizer API (not autograd) and not to be reformatted

[chrishkchris]

Hi After sync-ing all the versions between different OS

yapf --version

yapf 0.29.0

clang-format --version

clang-format version 9.0.0 (https://github.com/mgevaert/clang-format-wheel d3a647c305759f37f30257352e621579f53d0073)

the result is almost constistent except one small code snippet, yapf on ubuntu gives left result, while osx and windows give right result. (same yapf version and config)
4739c6e
Kindly let me know any comment and it is helpful to test at your side. thanks

@dcslin Thanks a lot!
You mean even the same python code formatting yapf version gives different result in ubuntu and osx/window? So which OS this PR is based on? I think this should be the last point we should make the same when we update the instruction in singa-doc. Personally I prefer window/osx as I do not have the ubuntu os to be installed with vscode.

[dcslin]

Hi After sync-ing all the versions between different OS

yapf --version

yapf 0.29.0

clang-format --version

clang-format version 9.0.0 (https://github.com/mgevaert/clang-format-wheel d3a647c305759f37f30257352e621579f53d0073)
the result is almost constistent except one small code snippet, yapf on ubuntu gives left result, while osx and windows give right result. (same yapf version and config)
4739c6e
Kindly let me know any comment and it is helpful to test at your side. thanks

@dcslin Thanks a lot!
You mean even the same python code formatting yapf version gives different result in ubuntu and osx/window? So which OS this PR is based on? I think this should be the last point we should make the same when we update the instruction in singa-doc. Personally I prefer window/osx as I do not have the ubuntu os to be installed with vscode.

Among hunderds of files in Singa, there are only a few lines of mis-match, which is showed in 4739c6e

Currently the code format result is on OSX/Wins. I agree with you on the preference on OSX/wins.

[nudles]

I am going to merge this PR if there is no more probem.

[dcslin]

I am going to merge this PR if there is no more probem.

adding files from chris comment. please wait a second

[dcslin]

Added NCCL files mentioned by @chrishkchris
Added model/operation also
command updated:

# format cpp code by clang-format
find src/api/ \
    src/core/ \
    src/proto/ \
    src/utils/ \
    include/singa/core/ \
    include/singa/utils/ \
    src/model/operation/ \
    include/singa/io/communicator.h \
    src/io/communicator.cc \
    test/singa/ -iname *.h -o -iname *.cc | xargs clang-format -i

# format python code by yapf
find python/ \
    examples/autograd \
    test/python/ -iname *.py | xargs yapf -i

This is ready for merge.

[dcslin]

Current code is up to date comparing singa:dev:

upstream	https://github.com/apache/singa.git (fetch)
upstream	https://github.com/apache/singa.git (push)
[11:15 AM] dcslin0:singa sc$ git merge upstream/dev
Already up to date.