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.
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
Add mxnet python bindings #5667
Conversation
The tests are being triggered in jenkins. |
+1 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
mxnet-predict.spec
Outdated
## INITENV +PATH PYTHON27PATH %{i}/lib64/python`echo $PYTHON_VERSION | cut -d. -f 1,2`/site-packages | ||
## INITENV +PATH PYTHON3PATH %{i}/lib64/python`echo $PYTHON3_VERSION | cut -d. -f 1,2`/site-packages | ||
## INITENV +PATH LD_LIBRARY_PATH %{i}/lib64 | ||
|
||
%define tag 337cf1b54cc02bde94f459c89863a18187b0aada | ||
%define branch 1.5.0-cms-mod | ||
%define github_user cms-externals | ||
Source: git+https://github.com/%{github_user}/incubator-mxnet.git?obj=%{branch}/%{tag}&export=%{n}-%{realversion}&submodules=1&output=/%{n}-%{realversion}-%{tag}.tgz | ||
|
||
BuildRequires: cmake ninja ccache |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrodozov , please drop this dependency and use our gcc compiler
mxnet-predict.spec
Outdated
@@ -17,6 +21,8 @@ rm -rf ../build; mkdir ../build; cd ../build | |||
# use LAPACK functions in OpenBLAS: | |||
# manually set MXNET_USE_LAPACK=1 and turn off USE_LAPACK in cmake | |||
export CFLAGS="-DMXNET_USE_LAPACK=1 -DMXNET_THREAD_LOCAL_ENGINE=1" | |||
export PYTHONV=$(echo $PYTHON_VERSION | cut -f1,2 -d.) | |||
export PYTHON3V=$(echo $PYTHON3_VERSION | cut -f1,2 -d.) | |||
|
|||
cmake ../%{n}-%{realversion} -GNinja \ | |||
-DCMAKE_CUDA_COMPILER_LAUNCHER=ccache \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrodozov , we also need to clean this up, we should use our compiler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok let me see about that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to understand what is ccache doing as a "compiler launcher". we don't use cuda with mxnet so I'll guess I have to change it to gcc (I did it and it's building), but also because we don't use cuda do we need this flag ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just drop ccache dependency and remove the 3 LAUNCHER lines from cmake command and you should be fine.
@mrodozov , can you please also add a unit test in cmssw to text mxnet ( https://mxnet.apache.org/get_started/validate_mxnet#python ) ? |
sure. I'll get the test as it is |
Pull request #5667 was updated. |
@mrodozov , please add mxnet-predict in to python_tools.spec otherwise |
please test |
mxnet-predict.spec
Outdated
@@ -1,12 +1,16 @@ | |||
### RPM external mxnet-predict 1.5.0 | |||
## INITENV +PATH PYTHON27PATH %{i}/lib64/python`echo $PYTHON_VERSION | cut -d. -f 1,2`/site-packages | |||
## INITENV +PATH PYTHON3PATH %{i}/lib64/python`echo $PYTHON3_VERSION | cut -d. -f 1,2`/site-packages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrodozov , please use PYTHON3_LIB_SITE_PACKAGES here
mxnet-predict.spec
Outdated
@@ -1,12 +1,16 @@ | |||
### RPM external mxnet-predict 1.5.0 | |||
## INITENV +PATH PYTHON27PATH %{i}/lib64/python`echo $PYTHON_VERSION | cut -d. -f 1,2`/site-packages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use PYTHON_LIB_SITE_PACKAGES
|
||
cmake ../%{n}-%{realversion} -GNinja \ | ||
-DCMAKE_CUDA_COMPILER_LAUNCHER=ccache \ | ||
-DCMAKE_C_COMPILER_LAUNCHER=ccache \ | ||
-DCMAKE_CXX_COMPILER_LAUNCHER=ccache \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pass -DCMAKE_INSTALL_LIBDIR=lib
to get lib instead of lib64. fix -toolfile
to pointto lib
mxnet-predict.spec
Outdated
rm %{i}/lib64/*.a | ||
|
||
rm %{i}/lib64/*.a %{i}/*.so | ||
mv %{i}/python* %{i}/lib64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
once cmake is updated to use lib, then fix this one too
Pull request #5667 was updated. |
please test |
The tests are being triggered in jenkins. |
+1 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+externals |
This pull request is fully signed and it will be integrated in one of the next IB/CMSSW_11_1_X/master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @silviodonato, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
please test
both python 2 and 3