OpenMS / contrib

Automated CMake build scripts for the OpenMS contrib libraries
Other
0 stars 21 forks source link

update eigen to 3.3.4 #55

Closed timosachsenberg closed 6 years ago

timosachsenberg commented 6 years ago

binaries at: http://bitbucket.org/eigen/eigen/get/3.3.4.tar.gz

jpfeuffer commented 6 years ago

Looks right on the first glance. Wondering how the C11 switch will work out ;)

jpfeuffer commented 6 years ago

Did it solve your problems?

timosachsenberg commented 6 years ago

hmm builds passed but the archive is not yet on the ftp... is this correct?

timosachsenberg commented 6 years ago

yes it solved the problems. BinnedSpectrum implementation is now 4x as fast

timosachsenberg commented 6 years ago

I think the C11 switch is not required but I thought it might be a good idea to also enforce it here

jpfeuffer commented 6 years ago

Hmm the failed CMake call did not cause an error in the script.

jpfeuffer commented 6 years ago

Archive is on the FTP now.

jpfeuffer commented 6 years ago

Ok let's rebuild

hroest commented 6 years ago

seems like it didnt work on Windows (which is not applied under Linux):

-- Call was: C:/jenkins/ws/openms_contrib_jobs/openms_contrib_CI/c2e226b2/contrib_build/src/eigen-eigen-5a0156e40feb: C:/Program Files (x86)/Git/usr/bin/patch.exe  -p0 -b -N -i "C:/jenkins/ws/openms_contrib_jobs/openms_contrib_CI/c2e226b2/source/patches//eigen/EigenDetermineVSServicePack.cmake.patch"
CMake Error at macros.cmake:242 (message):
  (Stripping trailing CRs from patch use --binary to disable.)

  patching file cmake/EigenDetermineVSServicePack.cmake

  Hunk #1 FAILED at 4.

  1 out of 1 hunk FAILED -- saving rejects to file
  cmake/EigenDetermineVSServicePack.cmake.rej
Call Stack (most recent call first):
  libraries.cmake/eigen.cmake:21 (OPENMS_PATCH)
  CMakeLists.txt:539 (OPENMS_CONTRIB_BUILD_EIGEN)

-- Configuring incomplete, errors occurred!
See also "C:/jenkins/ws/openms_contrib_jobs/openms_contrib_CI/c2e226b2/contrib_build/CMakeFiles/CMakeOutput.log".

C:\jenkins\ws\openms_contrib_jobs\openms_contrib_CI\c2e226b2\contrib_build>tar: Removing leading `C:/' from member names
Archiving artifacts
Finished: SUCCESS

seems like you need to update the patch: https://github.com/OpenMS/contrib/blob/master/patches/eigen/EigenDetermineVSServicePack.cmake.patch

jpfeuffer commented 6 years ago

Fails on MacOS (elcapitan):

CMake Warning at cmake/FindBLAS.cmake:1371 (message):
  BLA_VENDOR has been set to All but blas libraries could not be found or
  check of symbols failed.

  Please indicate where to find blas libraries.  You have three options:

  - Option 1: Provide the installation directory of BLAS library with cmake
  option: -DBLAS_DIR=your/path/to/blas

  - Option 2: Provide the directory where to find BLAS libraries with cmake
  option: -DBLAS_LIBDIR=your/path/to/blas/libs

  - Option 3: Update your environment variable (Linux: LD_LIBRARY_PATH,
  Windows: LIB, Mac: DYLD_LIBRARY_PATH)

  To follow libraries detection more precisely you can activate a verbose
  mode with -DBLAS_VERBOSE=ON at cmake configure.

  You could also specify a BLAS vendor to look for by setting
  -DBLA_VENDOR=blas_vendor_name.

  List of possible BLAS vendor: Goto, ATLAS PhiPACK, CXML, DXML, SunPerf,
  SCSL, SGIMATH, IBMESSL, Intel10_32 (intel mkl v10 32 bit),Intel10_64lp
  (intel mkl v10 64 bit, lp thread model, lp64 model), Intel10_64lp_seq
  (intel mkl v10 64 bit, sequential code, lp64 model),Intel( older versions
  of mkl 32 and 64 bit), ACML, ACML_MP, ACML_GPU, Apple, NAS, Generic
Call Stack (most recent call first):
  cmake/FindLAPACK.cmake:138 (find_package)
  test/CMakeLists.txt:30 (find_package)

CMake Error: CMAKE_Fortran_COMPILER not set, after EnableLanguage
CMake Error at /usr/local/Cellar/cmake/3.8.2/share/cmake/Modules/CheckFortranFunctionExists.cmake:45 (try_compile):
  Failed to configure test project build system.
Call Stack (most recent call first):
  cmake/FindBLAS.cmake:297 (check_fortran_function_exists)
  cmake/FindBLAS.cmake:1297 (check_fortran_libraries)
  cmake/FindBLASEXT.cmake:53 (find_package)
  cmake/FindBLASEXT.cmake:86 (find_package_blas)
  cmake/FindPASTIX.cmake:214 (find_package)
  test/CMakeLists.txt:83 (find_package)
hroest commented 6 years ago

@jpfeuffer something is wrong here - why does it report success when it actually failed?

-- Configuring incomplete, errors occurred!
See also "C:/jenkins/ws/openms_contrib_jobs/openms_contrib_CI/c2e226b2/contrib_build/CMakeFiles/CMakeOutput.log".

C:\jenkins\ws\openms_contrib_jobs\openms_contrib_CI\c2e226b2\contrib_build>tar: Removing leading `C:/' from member names
Archiving artifacts
Finished: SUCCESS
jpfeuffer commented 6 years ago

I think the patch is only applied on Windows since it is adressing a VS issue. Probably the line numbers do not match anymore.

jpfeuffer commented 6 years ago

And yes. Still some errors with getting the correct exit code, though.

hroest commented 6 years ago

yes, but now the patch does not work any more on the new eigen version, it seems like they changed the line if(NOT DETERMINED_VS_SERVICE_PACK OR NOT ${_pack})

@timosachsenberg can you fix this and try the new patch on Linux to make sure it works?

timosachsenberg commented 6 years ago

They seem to have incorporated the patch in the recent version so I was able to remove it. Let's see if it works on windows

jpfeuffer commented 6 years ago

I would try to disable build_testing with EIGEN. It seems to require all this optional BLAS/LAPACK stuff. https://github.com/tensorflow/tensorflow/pull/7586/files

jpfeuffer commented 6 years ago

Btw.. Building Eigen seems to work but takes ages on Windows.

jpfeuffer commented 6 years ago

Maybe disabling build_testing also helps in this.

jpfeuffer commented 6 years ago

I guess the switch was introduced later/on another branch/removed. Maybe we have to do stuff like others and patch the CMakeLists by commenting out the include of the test directories. https://github.com/kartikmohta/meta-km/blob/master/recipes-extended/eigen/eigen-3.1.3/eigen-disable-tests.patch

timosachsenberg commented 6 years ago

cool. thanks for checking!

jpfeuffer commented 6 years ago

It's weird. Did you build it on win before? Seems to get stuck and I don't see anything in the logs.

jpfeuffer commented 6 years ago

Finally. It just works after a restart of the machine.

jpfeuffer commented 6 years ago

Although it should be no problem to merge now, I would suggest to do it after the release to avoid confusion.

timosachsenberg commented 6 years ago

Yeah that is totally fine. Thanks!

timosachsenberg commented 6 years ago

will be addressed in https://github.com/OpenMS/contrib/pull/57

don't ask why ;)