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

Make HDF5 components optional for configurable build #2619

Closed
wants to merge 1 commit into from

Conversation

sh1r0
Copy link
Contributor

@sh1r0 sh1r0 commented Jun 18, 2015

This PR simply intends to make HDF5 as an optional dependency.

@bhack
Copy link
Contributor

bhack commented Jun 18, 2015

/cc @Nerei

@flx42
Copy link
Contributor

flx42 commented Jun 18, 2015

I'm not sure if it's a good idea to start making all external libraries optional, it will clutter the code with a lot of #ifdef. Do you really need to do that for glog and gflags?

Also, switching to C++11 should be a separate patch.
Adding Eigen support should be a separate patch too.

@bhack
Copy link
Contributor

bhack commented Jun 18, 2015

@flx42 You are right about separating PRs. But this needs #2523 and #2537.
The only thing he can do to separate this is to base/rebase this PR on the same commits (without squashing) of that pull requests and contribute commit to that PR for their competences. When that will be merged commits diffs will be automatically normalized.

@sh1r0
Copy link
Contributor Author

sh1r0 commented Jun 18, 2015

I agree with @bhack, this is to show the current progress for having caffe (w/ cmake) work on Android. I'll rebase commits after those PRs are merged. For Eigen, I'm not quite sure should we support it for Caffe, it's a workaround so far.

@bhack
Copy link
Contributor

bhack commented Jun 18, 2015

@sh1r0 I've already commented at OpenMathLib/OpenBLAS#363 to see if we could recover Openblas again.

@flx42
Copy link
Contributor

flx42 commented Jun 18, 2015

Oh, so it's just WIP then. So the point of the PR is to show current progress for Android? Not to submit an actual PR for making glog/gflags optional?

I thought you actually wanted to include Eigen support. In this situation, I would have created a separate patch for Eigen above this patch and kept the Eigen patch out of the PR.

@sh1r0
Copy link
Contributor Author

sh1r0 commented Jun 18, 2015

@flx42 Sorry for that, I just want these to be discussed, as I need helps and comments. In addition, this PR is still in progress and not ready to be merged, that's what I mean. Sorry again for confusing words.

@sh1r0
Copy link
Contributor Author

sh1r0 commented Jun 18, 2015

Again, for Eigen part, this is only a workaround patch for Android deployment as I'm not able to make OpenBLAS work on Android. I appreciate any helps about OpenBLAS. Besides, the Eigen patch failed in test cases related to backward (i.e. vanishing gradients) as I mentioned, and thus the patch is imperfect. That's why I did not make a independent PR for Eigen. But if someone is willing to, I appreciate for his help. (The Eigen is possible to be removed in the future.)

@bhack
Copy link
Contributor

bhack commented Jun 19, 2015

/cc @futurely @saudet

@bhack
Copy link
Contributor

bhack commented Jun 20, 2015

@sh1r0 this seems interesting OpenMathLib/OpenBLAS@a11555c

@xianyi
Copy link

xianyi commented Jun 20, 2015

Hi all,

I think OpenBLAS develop branch should support Android NDK cross compiler. However, OpenBLAS only support hard floating point ABI (-mfloat-abi=hard).

Xianyi

@bhack
Copy link
Contributor

bhack commented Jun 20, 2015

@xianyi Are you sure that softfp is not enough with the right -mfpu flag? See discussion at opencv/opencv#4103. What kind of instruction set do you really use to accelerate on arm?

@sh1r0
Copy link
Contributor Author

sh1r0 commented Jun 21, 2015

I tried to use openblas-v0.2.8-android-rc1.tar.gz, an older prebuilt version OpenBLAS for Android, to build my android libs and test on my simple demo app caffe-android-demo with success. However, it's much slower than that of Eigen and the possible reason is that the prebuilt OpenBLAS lib is for Android armv5tel ( single thread and without lapack ) as documented.
I'll have a try later to see if I can directly use the latest prebuilt OpenBLAS-v0.2.14-armv7a.tar.gz. Or, taking a look at OpenMathLib/OpenBLAS@a11555c to build OpenBLAS for android on my own, but this approach might need some tricks if https://github.com/taka-no-me/android-cmake is used for compilation.

@bhack
Copy link
Contributor

bhack commented Jun 21, 2015

@sh1r0 Probably we could evaluate to use Cmake external project feature. Official documentation here

@bhack
Copy link
Contributor

bhack commented Jun 21, 2015

@sh1r0
Copy link
Contributor Author

sh1r0 commented Jun 22, 2015

I tried to follow OpenMathLib/OpenBLAS@a11555c to build OpenBLAS for Android and link it to caffe libs without errors. However, I got the following error message during runtime.

** On entry to SGEMM parameter number 8 had an illegal value

Any ideas?

@bhack
Copy link
Contributor

bhack commented Jun 22, 2015

What revision are you using? How can i reproduce this crash?

@xianyi
Copy link

xianyi commented Jun 23, 2015

@bhack , we didn't use neon for ARM 32-bit. Thus, we just -mfpu=vfpv3 or -mfpu=vfpv4.

The problem is the function calling convention. The OpenBLAS kernel is assembly code, and we use hard floating point ABI (-mfloat-abi=hard). If you want soft floating point ABI (default mode), we must modify our assembly codes.

@bhack
Copy link
Contributor

bhack commented Jun 25, 2015

@sh1r0 How we can reproduce the SGEMM error? @xianyi Any hint?

@sh1r0
Copy link
Contributor Author

sh1r0 commented Jun 25, 2015

@bhack As @xianyi said, I used -Wl,--no-warn-mismatch flag to make OpenBLAS linked without error, but caffe and other dependencies were not built with hard float support. Currently, android cmake, which OpenCV for Android is mainly dependent on, does not support hardware float.

@bhack
Copy link
Contributor

bhack commented Jun 25, 2015

@sh1r0 Do you refer to this taka-no-me/android-cmake#30? Are you sure that it was only cause old NDK version didn't support hard float abi?

@sh1r0
Copy link
Contributor Author

sh1r0 commented Jun 25, 2015

@bhack

Are you sure that it was only cause old NDK version didn't support hard float abi?

I did not say that, my point is that android-cmake does not support hard float so far. Actually, I tried to make android-cmake support armeabi-v7a-hard but failed. Perhaps, you or someone could have a try. :p

@bhack
Copy link
Contributor

bhack commented Jun 25, 2015

@sh1r0 Yes I can try. How can I build this PR? With the standard android-cmake procedure?

@sh1r0
Copy link
Contributor Author

sh1r0 commented Jun 25, 2015

I use sh1r0/android-cmake@ebc8089 and the following script to build.

#!/usr/bin/env sh
export ANDROID_LIB_ROOT=<path to your library root>
export OPENCV_ROOT=$ANDROID_LIB_ROOT/OpenCV-2.4.9-android-sdk/sdk/native/jni
export PROTOBUF_ROOT=$ANDROID_LIB_ROOT/protobuf-2.5.0
export OpenBLAS_HOME=$ANDROID_LIB_ROOT/OpenBLAS-Android
cmake -DCMAKE_TOOLCHAIN_FILE=~/android-cmake/android.toolchain.cmake \
      -DANDROID_NDK=~/android-ndk-r10d/ \
      -DCMAKE_BUILD_TYPE=Release \
      -DANDROID_ABI="armeabi-v7a with NEON" \
      -DANDROID_NATIVE_API_LEVEL=14 \
      -DANDROID_TOOLCHAIN_NAME=arm-linux-androideabi-4.9 \
      -DADDITIONAL_FIND_PATH=$ANDROID_LIB_ROOT \
      -DBUILD_python=OFF \
      -DBUILD_docs=OFF \
      -DCPU_ONLY=ON \
      -DUSE_BOOST=OFF \
      -DUSE_GLOG=OFF \
      -DUSE_GFLAGS=OFF \
      -DUSE_LMDB=OFF \
      -DUSE_LEVELDB=OFF \
      -DUSE_HDF5=OFF \
      -DUSE_SNAPPY=OFF \
      -DBLAS=open \
      -DOpenCV_DIR=$OPENCV_ROOT \
      -DPROTOBUF_INCLUDE_DIR=$PROTOBUF_ROOT/include \
      -DPROTOBUF_LIBRARY=$PROTOBUF_ROOT/lib/libprotobuf.a \
      ..

@bhack
Copy link
Contributor

bhack commented Jun 25, 2015

Ok probably we cannot workaround opencv android SDK softfp without recompiling it with hard abi.

@bhack
Copy link
Contributor

bhack commented Jun 29, 2015

@mshabunin Are there any plan to have an ABI hard distribution of opencv for android SDK?

@sh1r0
Copy link
Contributor Author

sh1r0 commented Sep 26, 2015

@bhack Updated, please refer to the first comment.

@bhack
Copy link
Contributor

bhack commented Sep 26, 2015

@sh1r0 Nice progression! At some point I think we can substitute java-cpp caffe-preset build script that seems doesn't support android. This will let to have a full caffe java interface. @saudet @cypof what do you think?

@bhack
Copy link
Contributor

bhack commented Sep 26, 2015

@cypof Now I understand your interest in java-cpp caffe preset.

@bhack
Copy link
Contributor

bhack commented Sep 26, 2015

@cypof I really hope that we can unify (here) upstream java and android builds!

@saudet
Copy link

saudet commented Sep 26, 2015

@bhack An updated build script that supports Android would be great indeed. If someone gets something working, please send a PR, thanks!

@bhack
Copy link
Contributor

bhack commented Nov 11, 2015

@sh1r0 Any update from this. Could be already reviewed?

@sh1r0 sh1r0 force-pushed the cmake branch 2 times, most recently from 1060d3e to 76be3d0 Compare November 11, 2015 16:20
@sh1r0
Copy link
Contributor Author

sh1r0 commented Nov 11, 2015

@bhack I just rebased to master. But currently, there are only glog and hdf5 switches. Any further directions?

@sh1r0 sh1r0 changed the title Separate boost, glog and gflags dependencies for configurable installation Separate glog and hdf5 dependencies for configurable installation Nov 11, 2015
@bhack
Copy link
Contributor

bhack commented Nov 11, 2015

Seems any maintainer commented here.

@d4nst
Copy link

d4nst commented Dec 14, 2015

@sh1r0 what's the status of this? Is your caffe for android port still using Eigen, or you can finally use OpenBLAS? Also, have you tried to run it on iOS? I believe that it should work fine with Eigen, but I'm not sure about OpenBLAS.

@sh1r0
Copy link
Contributor Author

sh1r0 commented Dec 15, 2015

@danst18 It's fine with either Eigen or OpenBLAS for building caffe executables by Android NDK. However, OpenBLAS is limited to be built with hard float support, and thus built jni libs has runtime problems which I still cannot solve (probably related to this). BTW, I did not have any try on iOS so far. But I believe this PR should work for providing a simple way to build caffe with kind of customization.

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

Successfully merging this pull request may close these issues.