Skip to content
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

libnd4j: Link with MKL-DNN and OpenBLAS when available from Maven #6204

Merged
merged 7 commits into from Sep 5, 2018

Conversation

@saudet
Copy link
Member

commented Aug 18, 2018

I plan to add support for MKL-DNN to libnd4j by basically using Maven as our repository for native libraries as well. That'll probably eventually include BLAS and LAPACK too, so I've also added OpenBLAS here to test as a fallback for platforms that don't have MKL.

In the case of Windows, Intel doesn't test MKL-DNN with GCC, not sure if it would be a good idea to build it with it, but if we're stuck with MSVC, this means we might have to switch libnd4j to MSVC as well, without good OpenMP support, or switch to the Intel C++ Compiler...

Anyway, for now I've just added a couple of #include in NativeOps.h to test that it works, and then it's just about using them like any other native dependency.

@saudet saudet requested review from sshepel and raver119 Aug 18, 2018

@raver119
Copy link
Contributor

left a comment

<3

@saudet saudet force-pushed the sa_maven branch 3 times, most recently from 3c36a72 to f49659c Aug 23, 2018

@saudet saudet force-pushed the sa_maven branch 2 times, most recently from 52bccae to bfcb363 Aug 31, 2018

@saudet

This comment has been minimized.

Copy link
Member Author

commented Sep 3, 2018

Ok, this is building and running fine for me on Android, Linux, Mac, Windows, so I think it's good to merge!

@sshepel Jenkins is not picking up the updated parent pom.xml. Could you fix that so we can have the CI server test it a bit more?

@sshepel

This comment has been minimized.

Copy link
Contributor

commented Sep 3, 2018

@saudet sure, let me check that.

@saudet

This comment has been minimized.

Copy link
Member Author

commented Sep 3, 2018

Actually, the issue seems to be that it's using an old version of nd4j-native to build the rest of the modules...

@saudet saudet force-pushed the sa_maven branch from ee0029e to 21a0ff3 Sep 5, 2018

@sshepel

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2018

@saudet oh, I can agree with that, because we had some problems with failing builds for master branch, but now it's ok, so lets fire a new build.

@saudet

This comment has been minimized.

Copy link
Member Author

commented Sep 5, 2018

@sshepel It's failing here and there in pretty much the same way, but I don't think I'm seeing any failures related to this PR, so I'm merging this.

@saudet saudet merged commit a300b32 into master Sep 5, 2018

0 of 2 checks passed

codeclimate Code Climate encountered an error attempting to analyze this pull request.
Details
continuous-integration/jenkins/pr-head This commit cannot be built
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.