New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Caffe2 building scripts #41

Closed

ArutyunovG wants to merge 7 commits into peterjc123:master from ArutyunovG:caffe2_building

Contributor

ArutyunovG commented Nov 7, 2018

Hi @peterjc123,

Finished building Caffe2 with Visual Studio 2015 and the script is ready.
At the moment pytorch is cloned from my fork, where the build fully runs. When everything with adapting my pytorch PR will be agreed, of course will switch it to the main repo.

The only exception found at the moment is CUDA 8 in Debug mode. In Release though, it still is able to build.
Successfully tried CUDA 9.0, 9.1, python 2.7 and 3.5, cudnn 7.* in both Debug and Release.

Contributor Author

ArutyunovG commented Nov 7, 2018

Have to merge image_input_op.h to resolve conflict with the main branch.

ArutyunovG closed this

Contributor Author

ArutyunovG commented Nov 8, 2018

Ok, it runs after mege.
I have some 18 CI issues that don't know hot to resolve though. Probably you can tell me, what's wrong there? They don't seem related to build.

ArutyunovG reopened this

peterjc123 reviewed

View reviewed changes

Owner

peterjc123 left a comment

Thanks for your PR. These are the problems that need to be resolved before it can be merged. Anyway, I'll add it to the CI to test its feasibility.

caffe2_builders/build_windows_shared_v140.bat Outdated

+                  msbuild /p:Configuration=Release /p:Platform=x64 /m:%NUM_BUILD_PROC% INSTALL.vcxproj /p:PreferredToolArchitecture=x64
+              )
+              cd %ORIGINAL_DIR%

Owner

peterjc123 Nov 8, 2018

nit: newline here

caffe2_builders/build_windows_shared_v140.bat Outdated

+              )
+              rem Building Debug in %CAFFE2_ROOT%\build\Debug
+              if %BUILD_DEBUG% EQU 1 (

Owner

peterjc123 Nov 8, 2018

What about a variable like CONFIG and if "%CONFIG%"=="Debug"?

Owner

peterjc123 Nov 8, 2018

In this way, your code can be reused.

caffe2_builders/build_windows_shared_v140.bat Outdated

+                  exit
+              )
+              if %CMAKE_LINKER% EQU "" (

Owner

peterjc123 Nov 8, 2018

Why do we want to override the default linker?

Contributor Author

ArutyunovG Nov 8, 2018 •

edited

I explicitly specify x64 toolset to build caffe2_gpu.dll in Debug with pdbs.

caffe2_builders/build_windows_shared_v140.bat Outdated

+                  exit
+              )
+              if %CMAKE_C_COMPILER% EQU "" (

Owner

peterjc123 Nov 8, 2018

Use CC and CXX for unification.

caffe2_builders/build_windows_shared_v140.bat Outdated

+              )
+              rem Now checking, that everything is defined for build...
+              if %CMAKE_CXX_COMPILER% EQU "" (

Owner

peterjc123 Nov 8, 2018

Use CC and CXX for unification.

caffe2_builders/build_windows_shared_v140.bat Outdated

+              rem msbuild /m: option value
+              if NOT DEFINED NUM_BUILD_PROC (
+                  set NUM_BUILD_PROC=6

Owner

peterjc123 Nov 8, 2018

Use MAX_JOBS for unification.

caffe2_builders/build_windows_shared_v140.bat Outdated

+              )
+              rem Should build folder be deleted and build start from scratch?
+              if NOT DEFINED CLEAR_BUILD_FOLDER_AND_START_AGAIN (

Owner

peterjc123 Nov 8, 2018

The name is so long, what about changing it into REBUILD?

caffe2_builders/build_windows_shared_v140.bat Outdated

+              set CAFFE2_ROOT=%~dp0%\pytorch
+              rem Cloning repository if it doesn't exist
+              if not exist pytorch (

Owner

peterjc123 Nov 8, 2018

The script internal\clone.bat can be used here.

peterjc123 reviewed

View reviewed changes

caffe2_builders/v140/auto.bat Outdated

+              rem Building Debug in %CAFFE2_ROOT%\build\Debug
+              if %BUILD_DEBUG% EQU 1 (
+                  set CONFIG=Debug
+                  call msbuild.bat

Owner

peterjc123 Nov 8, 2018

%cd% may not exactly equal to %~dp0.

peterjc123 reviewed

View reviewed changes

caffe2_builders/v140/auto.bat Outdated

+              rem Building Release in %CAFFE2_ROOT%\build\Release
+              if %BUILD_RELEASE% EQU 1 (
+                  set CONFIG=Release
+                  call msbuild.bat

Owner

peterjc123 Nov 8, 2018

And here.

peterjc123 reviewed

View reviewed changes

caffe2_builders/v140/auto.bat

+                  exit
+              )
+              if %CMAKE_LINKER% EQU "" (

Owner

peterjc123 Nov 8, 2018

Are you changing the default linker? It's interesting,

peterjc123 reviewed

View reviewed changes

caffe2_builders/v140/auto.bat Outdated

+              rem msbuild /m: option value
+              if NOT DEFINED MAX_JOBS (
+                  set MAX_JOBS=6

Owner

peterjc123 Nov 8, 2018

Consider using %NUMBER_OF_PROCESSORS% instead?

Contributor Author

ArutyunovG Nov 8, 2018

As far as I know, it is rather number of processes, not devices.
https://www.hanselman.com/blog/FasterBuildsWithMSBuildUsingParallelBuildsAndMulticoreCPUs.aspx
Switch it to NUMBER_OF_PROCESSES?

Owner

peterjc123 Nov 9, 2018

%NUMBER_OF_PROCESSORS% stores the number of logical processors. For example, I have an i7-7700hq and then this env var is set to 8 by the OS. It's not good to pin the MAX_JOBS to a constant number.

peterjc123 reviewed

View reviewed changes

caffe2_builders/v140/msbuild.bat Outdated

+                  exit
+              )
+              msbuild /p:Configuration=%CONFIG% /p:Platform=x64 /m:%MAX_JOBS% INSTALL.vcxproj /p:PreferredToolArchitecture=x64

Owner

peterjc123 Nov 8, 2018

nit: newline here

peterjc123 reviewed

View reviewed changes

caffe2_builders/v140/auto.bat Outdated

+              if %REBUILD% EQU 1 (
+                  if exist %CAFFE2_ROOT%\build (
+                      rmdir %CAFFE2_ROOT%\build /s /q

Owner

peterjc123 Nov 8, 2018

Consider using python setup.py clean?

Contributor Author

ArutyunovG Nov 8, 2018

There is no setup.py here, everything is installed as CMake install rules specify.
Explain please, what do you want setup.py to do?

Owner

peterjc123 Nov 9, 2018

%CAFFE2_ROOT% is the path of the pytorch repo, then there should be setup.py under that directory. Run python setup.py clean will clean everything that is not tracked in the repo.

peterjc123 reviewed

View reviewed changes

caffe2_builders/v140/auto.bat

+              )
+              rem Debug build enabled by default
+              if NOT DEFINED BUILD_DEBUG (

Owner

peterjc123 Nov 8, 2018

So if both BUILD_DEBUG and BUILD_RELEASE are not specified, DEBUG will be used. I think it should default to Release instead.

Contributor Author

ArutyunovG Nov 8, 2018

It must build both, Debug and Release.

Contributor Author

ArutyunovG Nov 8, 2018

First it builds debug, then release.

ArutyunovG mentioned this pull request

Windows shared build pytorch/pytorch#13550

Closed

Owner

peterjc123 commented Nov 9, 2018

@ArutyunovG Could you rebase this PR against the master branch? I want to test it through CI.

ArutyunovG force-pushed the caffe2_building branch from 6c71633 to e8327d8 Compare

November 9, 2018 10:03

Contributor Author

ArutyunovG commented Nov 9, 2018

@peterjc123
Rebased, check if everything is correct with the rebase, please.

ArutyunovG and others added 6 commits

November 10, 2018 13:19


          Added Caffe2 building script for Visual Studio 2015

203859c


          Build DEBUG as well.

9b4b8d6


          caffe2 v140 build system refactoring.

40301f3


          Newline fixes and ~%dp0.


          ORIGINAL_DIR must be %cd%.

364a602


          MAX_JOBS=%NUMBER_OF_PROCESSORS%;

e79b878

python setup.py clean instead of rmdir

peterjc123 force-pushed the caffe2_building branch from e8327d8 to e79b878 Compare

November 10, 2018 05:20

peterjc123 previously approved these changes

View reviewed changes

Owner

peterjc123 left a comment

I've rebased your changes again due to the changes in the CI side. This PR will be merged automatically if all things go right.

peterjc123 closed this

peterjc123 reopened this


          Test through CI

cf57eb9

peterjc123 dismissed their stale review via

cf57eb9

November 10, 2018 05:30

Owner

peterjc123 commented Nov 10, 2018

Oh no, CI seems to be stuck in this PR. Would you please open another PR?

peterjc123 changed the title ~~Caffe2 building~~ Add Caffe2 building scripts

Owner

peterjc123 commented Nov 10, 2018

BTW, the CI didn't pass. https://peter-jc.visualstudio.com/peter_jiachen/_build/results?buildId=166&view=logs

Contributor Author

ArutyunovG commented Nov 10, 2018 •

edited

@peterjc123
Could you please explain what needs to be done on my side?
What PR to open and what are the problems with CI?
BTW edit:
This is my Skype for faster communication on technical issues, feel free to add if there is a need at some point: the_last_inspiration.

Contributor Author

ArutyunovG commented Nov 10, 2018

On my PC the build runs about 5 hours. Not sure what happened, that made you think it is stuck though.
Waiting for your answer.

Owner

peterjc123 commented Nov 10, 2018

@ArutyunovG Due to an unknown reason, the build of the CI cannot be triggered in this PR. What you could do is to do the following things:

Close this PR
Create a new branch that points to the current master
Cherry-pick every commit in this PR
Open a new PR with that new branch

peterjc123 reviewed

View reviewed changes

caffe2_builders/v140/msbuild.bat

+                                           -DINCLUDE_EXPERIMENTAL_C10_OPS=OFF ^
+                                           -DBUILD_SHARED_LIBS=ON ^
+                                           -DBUILD_BINARY=ON

Owner

peterjc123 Nov 10, 2018

We'll have to catch errors here, otherwise, the script will complete with exit code 0 even if there are actually errors. Consider using something like if errorlevel 1 exit /b 1

peterjc123 reviewed

View reviewed changes

caffe2_builders/v140/msbuild.bat

		)

		msbuild /p:Configuration=%CONFIG% /p:Platform=x64 /m:%MAX_JOBS% INSTALL.vcxproj /p:PreferredToolArchitecture=x64

Owner

peterjc123 Nov 10, 2018

and here.

peterjc123 reviewed

View reviewed changes

caffe2_builders/v140/auto.bat

+              rem Building Release in %CAFFE2_ROOT%\build\Release
+              if %BUILD_RELEASE% EQU 1 (
+                  set CONFIG=Release
+                  call %~dp0%msbuild.bat

Owner

peterjc123 Nov 10, 2018

and here

peterjc123 reviewed

View reviewed changes

caffe2_builders/v140/auto.bat

+              rem Building Debug in %CAFFE2_ROOT%\build\Debug
+              if %BUILD_DEBUG% EQU 1 (
+                  set CONFIG=Debug
+                  call %~dp0%msbuild.bat

Owner

peterjc123 Nov 10, 2018

and here

ArutyunovG closed this

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