-
Notifications
You must be signed in to change notification settings - Fork 60
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 Eigen3 as an optional alternative to Lapack #51
Open
phcerdan
wants to merge
15
commits into
albarji:master
Choose a base branch
from
phcerdan:use_eigen
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add dummy test checking if linking works properly. Add FindLAPACKE.cmake and export proxTVconfig.cmake. FindLAPACKE.cmake is used to handle lapacke installations with no export targets. Also complete the generation of proxTVconfig.cmake Add a dummy project to test usage of proxTV from external project. Add C Interface to Readme Remove Foo_FOUND in the case FOO is REQUIRED Update cmake required version to 3.5 This simplifies policy and version set up. Require a modern lapacke and remove FIND_LAPACKE This will look for a `lapacke-config.cmake`, that is generated since lapacke version 3.6. And allow us to remove the buggy FIND_LAPACKE. Add test for cmake in travis. Restore FindLAPACKE.cmake Because travis still uses ubuntu 14.04, with an old lapacke which does not provide config.cmake. Fix CMake Added FindLAPACK FindLAPACKE CONFIG is not populating LAPACK. From @jcfr branch: https://github.com/jcfr/proxTV/tree/cmake_support_squashed Co-authored-by: Jean-Christophe Fillion-Robin <[email protected]> WIP: Avoid deploy in .travis Also install cmake with pip (3.12) Also use find_package(LAPACKE NOCONFIG) for old librart versions Error: ``` +twine upload --repository-url https://test.pypi.org/legacy/ --username albarji dist/prox_tv-3.3.0-cp27-cp27mu-manylinux1_x86_64.whl Enter your password: No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself. Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received ```
Only two lapack functions are involved cmake: Add compile definition depending on LAPACK/EIGEN option: proxTV_USE_LAPACK (for consistency) compile definition: PROXTV_USE_LAPACK All instances of lapack calls have Eigen alternative Working!
BUG: Add source_dir to include_directories Fix cmake: remove installing config files in src/CMakeLists Handled in top CMakelists.txt
phcerdan
changed the title
Add Eigen3 alternatives to Lapacke
Add Eigen3 as an optional alternative to Lapacke
Jan 15, 2019
phcerdan
changed the title
Add Eigen3 as an optional alternative to Lapacke
Add Eigen3 as an optional alternative to Lapack
Jan 15, 2019
It uses the cmake_support branch introduced in #38 |
Triggered when wrapping ITK ```bash /usr/bin/ld: _deps/proxtv_fetch-build/src/libitkproxTV-5.0.a(lapackFunctionsWrap.cpp.o): relocation R_X86_64_PC32 against symbol `_ZTVSt9bad_alloc@@GLIBCXX_3.4' can not be used when making a shared object; recompile with -fPIC ```
Using CXX_CMAKE_FLAGS+="-Wall -Wextra" Plus Windows specific warnings
COMP: OpenMP_CXX.lib does not exist with Visual Studio
_deps\proxtv_fetch-src\src\TVL1opt_tautstring.cpp(336): warning C4244: '=': conversion from '__int64' to 'int', possible loss of data
Note: LAMBDA_STEPS_TVLP is a a preprocessor definition set to 1.
Windows warnings
This allows building proxTV with Eigen, when Eigen is an internal target, instead of only allowing find_package(Eigen) Closes InsightSoftwareConsortium/ITKTotalVariation#21
with variables: ```cmake proxTV_INSTALL_INCLUDE_DIR proxTV_INSTALL_LIB_DIR proxTV_INSTALL_BIN_DIR proxTV_INSTALL_CMAKE_DIR ``` These variables where ignored, and other CACHE variables were used. `proxTV_INSTALL_CMAKE_DIR` was added with a default value.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
proxTV depends on either Eigen3 of Lapack
Add Eigen3 functions as alternative to lapack.
This should aid packaging proxTV for windows.
Even though Eigen3 is a lightweight and fast c++ linear algebra library,
a performance drop should be expected when compared with lapack,
but hasn't been quantified.
Neither any non-default, performance-increased option in Eigen3 has been tested.
The test show that proxTV is fast enough for 3D imaging processing,
which might suffice for a the majority of cases.