[SyneRBI/SIRF-SuperBuild] Build gadgetron master (#438)
136 views
Skip to first unread message
Edoardo Pasca
Oct 6, 2020, 9:53:28 AM10/6/20
to SyneRBI/SIRF-SuperBuild, Subscribed
This changes should allow the build of Gadgetron master.
closes #378
closes #384
You can view, comment on, or merge this pull request online at:
https://github.com/SyneRBI/SIRF-SuperBuild/pull/438
Commit Summary
- allows user to pass CBLAS_LIBRARY and CBLAS_INCLUDE_DIR
- Merge remote-tracking branch 'origin' into build_Gadgetron_master
File Changes
Patch Links:
- https://github.com/SyneRBI/SIRF-SuperBuild/pull/438.patch
- https://github.com/SyneRBI/SIRF-SuperBuild/pull/438.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Edoardo Pasca
Oct 6, 2020, 9:59:46 AM10/6/20
to SyneRBI/SIRF-SuperBuild, Subscribed
It'd be good to decide which commit tag to use for Gadgetron in version_config.cmake
This one is the minimum one @johannesmayer would require to build his RPE encoding branch
Kris Thielemans
Oct 6, 2020, 5:01:31 PM10/6/20
to SyneRBI/SIRF-SuperBuild, Subscribed
@KrisThielemans approved this pull request.
looks fine to me.
Edoardo Pasca
Oct 7, 2020, 12:28:11 AM10/7/20
to SyneRBI/SIRF-SuperBuild, Subscribed
But Gadgetron on my VM doesn't seem to build! I had checked but there may have been some caching mechanisms that made it work then.
The culprit is rather simple, really, CMake is giving the wrong directory for includes to Gadgetron:
cd /home/sirfuser/devel/buildVM/builds/Gadgetron/build/core && /usr/bin/c++
-DARMA_64BIT_WORD -DARMA_DONT_USE_WRAPPER
-DARMA_USE_CXX11 -DFORCE_LIMIT_OPENBLAS_NUM_THREADS -DUSE_OMP -Dgadgetron_core_EXPORTS
-I/home/sirfuser/devel/buildVM/builds/Gadgetron/build/include
-I/home/sirfuser/devel/buildVM/sources/Gadgetron/core
-I/home/sirfuser/devel/buildVM/sources/Gadgetron/toolboxes/core
-I/home/sirfuser/devel/buildVM/builds/Gadgetron/build/toolboxes/core
-I/home/sirfuser/devel/buildVM/sources/Gadgetron/toolboxes/core/cpu
-I/home/sirfuser/devel/buildVM/sources/Gadgetron/toolboxes/core/cpu/image
-I/home/sirfuser/devel/buildVM/sources/Gadgetron/toolboxes/core/cpu/algorithm
-I/home/sirfuser/devel/buildVM/sources/Gadgetron/toolboxes/core/cpu/hostutils
-I/home/sirfuser/devel/buildVM/sources/Gadgetron/toolboxes/core/cpu/math
-I/home/sirfuser/devel/buildVM/sources/Gadgetron/apps/gadgetron
-I/home/sirfuser/devel/buildVM/sources/Gadgetron/toolboxes/log
-isystem /home/sirfuser/devel/install/include/ismrmrd
-DBOOST_ERROR_CODE_HEADER_ONLY -DBOOST_SYSTEM_NO_DEPRECATED
-fopenmp
-Werror=return-type -Werror=terminate -Werror=non-virtual-dtor
-O3 -DNDEBUG -fPIC -std=gnu++1z
-o CMakeFiles/gadgetron_core.dir/Channel.cpp.o
-c /home/sirfuser/devel/buildVM/sources/Gadgetron/core/Channel.cpp
I passes the include directory of our install, which is good but it does it with -isystem and appending ismrmrd, which is wrong.
The CMake line that does reads:
target_include_directories(gadgetron_core PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}> $<INSTALL_INTERFACE:include> )
I don't understand very well this generator expression, but it doesn't look to me that it should be wrong. But it's difficult to debug.
Edoardo Pasca
Oct 7, 2020, 12:42:51 AM10/7/20
to SyneRBI/SIRF-SuperBuild, Subscribed
If I understand correctly, we are trying to pass Gadgetron the include directory here as
-DCMAKE_INCLUDE_PATH:PATH=${SUPERBUILD_INSTALL_DIR}/includeHowever, CMAKE_INCLUDE_PATH provides a search path for the find_file() and find_path() commands which is not what we want? We want to add to the standard include directories of Gadgetron.
Kris Thielemans
Oct 7, 2020, 3:28:55 AM10/7/20
to SyneRBI/SIRF-SuperBuild, Subscribed
However, CMAKE_INCLUDE_PATH provides a search path for the
find_file()andfind_path()commands which is not what we want? We want to add to the standard include directories of Gadgetron.
That's correct. A lot of the External* files have lines setting CMAKE_* variables, but they shouldn't be there. Even CMAKE_LIBRARY_PATH shouldn't be set as it should find it from the PREFIX or some other default path.
Kris Thielemans
Oct 7, 2020, 4:33:03 AM10/7/20
to SyneRBI/SIRF-SuperBuild, Subscribed
-isystem /home/sirfuser/devel/install/include/ismrmrd
this isn't something that we do. It has to come from how Gadgetron depends on ISMRMRD. Gadgetron master uses dependencies on ISMRMRD::ISMRMRD (see e.g.here, so that implies it comes from ISMRMRDConfig.cmake.
That uses include here. It uses to be include/ismrmrd but that was fixed 2 years ago ismrmrd/ismrmrd#109 which seems to be between ISMRMRD 1.4.0 and 1.4.1. I think we're using the latter, so I don't understand. In any case, you'd want to update ISMRMD version if you use Gadgetron master. Most recent seems 1.4.2.1
Edoardo Pasca
Dec 10, 2020, 9:28:59 AM12/10/20
to SyneRBI/SIRF-SuperBuild, Subscribed
Built ISMRMRD 1.4.2.1, now Gadgetron fails with
/home/sirfuser/devel/buildVM/sources/Gadgetron/toolboxes/core/cpu/math/hoNDArray_reductions.cpp:3:10: fatal error: range/v3/core.hpp: No such file or directory
#include <range/v3/core.hpp>
^~~~~~~~~~~~~~~~~~~
compilation terminated.
It seems now Gadgetron depends on range-v3
Edoardo Pasca
Dec 11, 2020, 8:04:39 AM12/11/20
to SyneRBI/SIRF-SuperBuild, Subscribed
I'll add the range-v3 to the superbuild (it's a header only library!).
Next problem
In file included from /home/sirfuser/devel/buildVM/sources/Gadgetron/toolboxes/klt/cpu/hoNDKLT.cpp:6:0:
/home/sirfuser/devel/buildVM/sources/Gadgetron/toolboxes/core/cpu/hoNDArray_utils.h: In function ‘auto Gadgetron::concat_along_dimension(const COLL&, size_t)’:
/home/sirfuser/devel/buildVM/sources/Gadgetron/toolboxes/core/cpu/hoNDArray_utils.h:1015:121: error: cannot deduce template arguments for ‘logical_and’ from ()
umulate(arrays | view::transform(dimensions_valid), true, std::logical_and() );
^
/home/sirfuser/devel/buildVM/sources/Gadgetron/toolboxes/core/cpu/hoNDArray_utils.h:1021:81: error: cannot deduce template arguments for ‘multiplies’ from ()
size_t(1), std::multiplies());
Kris Thielemans
Dec 11, 2020, 8:20:09 AM12/11/20
to SyneRBI/SIRF-SuperBuild, Subscribed
not sure. sorry. You'll need to raise an issue on Gadgetron. More than likely they'll say that you need to have a more recent version of gcc (and potentially C++), but as their CMake doesn't enforce it, they should know about this.
Edoardo Pasca
Jan 20, 2021, 5:39:37 AM1/20/21
to SyneRBI/SIRF-SuperBuild, Subscribed
Opened an issue in gadgetron/gadgetron#925
Edoardo Pasca
Jan 23, 2021, 6:20:00 PM1/23/21
to SyneRBI/SIRF-SuperBuild, Subscribed
Apparently Gadgetron requires C++17 which means GCC 9. gadgetron/gadgetron#925 (comment)
Edoardo Pasca
Jan 25, 2021, 1:21:50 PM1/25/21
to SyneRBI/SIRF-SuperBuild, Subscribed
This also requires CUDA SDK >= 10.2 to build Gadgetron with CUDA gadgets, otherwise the build fails with this error:
nvcc fatal : Value 'c++17' is not defined for option 'std'
Edoardo Pasca
Jan 26, 2021, 2:59:47 AM1/26/21
to SyneRBI/SIRF-SuperBuild, Subscribed
currently Gadgetron test build fails:
/usr/include/c++/9/tuple:1394:33: error: no match for ‘operator==’ (operand types are ‘std::__tuple_element_t<0, std::tuple<ISMRMRD::AcquisitionHeader, Gadgetron::hoNDArray<std::complex<float> >, std::optional<Gadgetron::hoNDArray<float> > > >’ {aka ‘const ISMRMRD::AcquisitionHeader’} and ‘std::__tuple_element_t<0, std::tuple<ISMRMRD::AcquisitionHeader, Gadgetron::hoNDArray<std::complex<float> >, std::optional<Gadgetron::hoNDArray<float> > > >’ {aka ‘const ISMRMRD::AcquisitionHeader’})
1394 | return bool(std::get<__i>(__t) == std::get<__i>(__u))
| ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~
Edoardo Pasca
Jan 26, 2021, 3:13:53 AM1/26/21
to SyneRBI/SIRF-SuperBuild, Subscribed
Using GCC 9 for building Gadgetron adds another layer of difficulty, due to some changes in behaviour of GCC 9 wrt older versions.
In particular the OpenMP clause #pragma omp parallel for default(none) leads to some variable not being declared. Unfortunately, this happens with NIFTYREG.
In function ‘void GetDiscretisedValueSSD_core3D_2(nifti_image*, float*, int, int, nifti_image*, nifti_image*, int*) [with DTYPE = float]’:
/home/ofn77899/devel/build_Gadgetron/sources/NIFTYREG/reg-lib/cpu/_reg_ssd.cpp:850:35: error: ‘voxelBlockNumber_t’ not specified in enclosing ‘parallel’
850 | for(idBlock=0;idBlock<voxelBlockNumber_t;idBlock++) {
|
Edoardo Pasca
Jan 26, 2021, 11:04:24 AM1/26/21
to SyneRBI/SIRF-SuperBuild, Subscribed
NiftyPET won't build with GCC 9
[ 4%] Building NVCC (Device) object nipet/CMakeFiles/mmr_auxe.dir/src/mmr_auxe_generated_scanner_0.cu.o
In file included from /usr/local/cuda/include/cuda_runtime.h:83,
from <command-line>:
/usr/local/cuda/include/crt/host_config.h:129:2: error: #error -- unsupported GNU version! gcc versions later than 8 are not supported!
Kris Thielemans
Jan 26, 2021, 3:20:42 PM1/26/21
to SyneRBI/SIRF-SuperBuild, Subscribed
In particular the OpenMP clause #pragma omp parallel for default(none) leads to some variable not being declared. Unfortunately, this happens with NIFTYREG.
Yes, noted at #413 (comment), which points to a NiftyReg PR
Kris Thielemans
Jan 26, 2021, 3:23:40 PM1/26/21
to SyneRBI/SIRF-SuperBuild, Subscribed
I suggest to keep comments here restricted to Gadgetron. Problems with gcc9 other dependencies belong in #413
Kris Thielemans
Jan 26, 2021, 3:26:44 PM1/26/21
to SyneRBI/SIRF-SuperBuild, Subscribed
@KrisThielemans requested changes on this pull request.
> set(DEFAULT_ACE_URL https://github.com/paskino/libace-conda) set(DEFAULT_ACE_TAG origin/master) + # range-v3 + set(DEFAULT_JSON_TAG origin/master)
JSON_TAG doesn't look ok
> @@ -169,6 +169,10 @@ set(DEFAULT_CCPi-FrameworkPlugins_TAG ${CIL_VERSION})
set(DEFAULT_CCPi-Astra_URL https://github.com/vais-ral/CCPi-Astra.git)
set(DEFAULT_CCPi-Astra_TAG ${CIL_VERSION})
+# range-v3
+set(DEFAULT_JSON_URL https://github.com/ericniebler/range-v3.git )
+set(DEFAULT_JSON_TAG 0.11.0)
JSON_TAG doesn't look ok
Kris Thielemans
Jan 26, 2021, 3:27:07 PM1/26/21
to SyneRBI/SIRF-SuperBuild, Subscribed
Does this PR build if asking for Gadgetron only?
Edoardo Pasca
Jan 27, 2021, 3:17:27 AM1/27/21
to SyneRBI/SIRF-SuperBuild, Push
@paskino pushed 4 commits.
- c635bd4 creates env_ccppetmr.* symbolic links
- adb7019 Merge branch 'master' into build_Gadgetron_master
- e002ca7 pass SIRF include directory to Gadgetron from CPATH env variable
- 5bcdc05 updates to build Gadgetron master
—
You are receiving this because you are subscribed to this thread.
Edoardo Pasca
Jan 27, 2021, 3:18:09 AM1/27/21
to SyneRBI/SIRF-SuperBuild, Subscribed
@paskino commented on this pull request.
> set(DEFAULT_ACE_URL https://github.com/paskino/libace-conda) set(DEFAULT_ACE_TAG origin/master) + # range-v3 + set(DEFAULT_JSON_TAG origin/master)
you are right, I fixed it but not pushed
Edoardo Pasca
Jan 27, 2021, 3:18:19 AM1/27/21
to SyneRBI/SIRF-SuperBuild, Subscribed
> @@ -169,6 +169,10 @@ set(DEFAULT_CCPi-FrameworkPlugins_TAG ${CIL_VERSION})
set(DEFAULT_CCPi-Astra_URL https://github.com/vais-ral/CCPi-Astra.git)
set(DEFAULT_CCPi-Astra_TAG ${CIL_VERSION})
+# range-v3
+set(DEFAULT_JSON_URL https://github.com/ericniebler/range-v3.git )
+set(DEFAULT_JSON_TAG 0.11.0)
you are right, I fixed it but not pushed
—
Kris Thielemans
Jan 27, 2021, 3:23:00 AM1/27/21
to SyneRBI/SIRF-SuperBuild, Subscribed
currently Gadgetron test build fails:
This just seems a Gadgetron bug in their current master which presumably will be gone tomorrow.
I feel chasing their master is just giving you extra headaches. I suppose we need @johannesmayer 's gadgetron/gadgetron#794 and gadgetron/gadgetron#816 . We should therefore be fine with the merge commit of the latter, which is gadgetron/gadgetron@0033296. @johannesmayer @ckolbPTB could you confirm if this would be ok?
Johannes Mayer
Jan 27, 2021, 4:11:28 AM1/27/21
to SyneRBI/SIRF-SuperBuild, Subscribed
There has also been the fix of an include statement that was required, so I would need gadgetron/gadgetron@7d7a31c
ckolbPTB
Jan 27, 2021, 7:41:42 AM1/27/21
to SyneRBI/SIRF-SuperBuild, Subscribed
I leave this one to Johannes - he is the Gadgetron build expert
Kris Thielemans
Jan 27, 2021, 3:29:48 PM1/27/21
to SyneRBI/SIRF-SuperBuild, Subscribed
@KrisThielemans requested changes on this pull request.
In SuperBuild/External_Gadgetron.cmake:
> @@ -107,12 +116,13 @@ if(NOT ( DEFINED "USE_SYSTEM_${externalProjName}" AND "${USE_SYSTEM_${externalPr
${PYTHONLIBS_CMAKE_ARGS}
-DGTEST_ROOT:PATH=${GTEST_ROOT}
${HDF5_CMAKE_ARGS}
+ -DHDF5_FOUND:BOOL=ON
this makes zero sense to me
In SuperBuild/External_Gadgetron.cmake:
> +if (NOT HDF5_CMAKE_ARGS)
+ message( FATAL_ERROR "HDF5_CMAKE_ARGS not found: ${HDF5_CMAKE_ARGS}")
+else()
+ message( STATUS "HDF5_CMAKE_ARGS found: ${HDF5_CMAKE_ARGS}")
+endif()
+
can't see why you need it. The project depends on HDF5. It should set this appropriately (or not, possibly, if we USE_SYSTEM_HDF5=ON)
> @@ -115,7 +115,7 @@ set(DEFAULT_glog_TAG v0.3.5) ## ITK set(DEFAULT_ITK_URL https://github.com/InsightSoftwareConsortium/ITK.git) -set(DEFAULT_ITK_TAG v4.13.1) +set(DEFAULT_ITK_TAG v4.13.3)
don't do this in this PR
In SuperBuild/External_Gadgetron.cmake:
> @@ -24,12 +24,21 @@ #This needs to be unique globally set(proj Gadgetron) +if ((NOT CC_9) OR (NOT CXX_9))
could try and find it of course
—
You are receiving this because you are subscribed to this thread.
Edoardo Pasca
Nov 5, 2021, 12:01:57 PM11/5/21
to SyneRBI/SIRF-SuperBuild, Subscribed
I updated Boost minimal version to 1.71.0, but installing 1.72.0 as the former doesn't exist!
The build seems to complain that I haven't built boost.coroutine https://github.com/SyneRBI/SIRF-SuperBuild/runs/4118695695?check_suite_focus=true#step:6:17209
How do I require that in
https://github.com/SyneRBI/SIRF-SuperBuild/blob/053bfabcdfd0967873aaaf68252e7e0ef497cc26/SuperBuild/External_Boost_configureboost.cmake#L27
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Edoardo Pasca
Nov 5, 2021, 12:02:51 PM11/5/21
to SyneRBI/SIRF-SuperBuild, Subscribed
@paskino commented on this pull request.
> @@ -115,7 +115,7 @@ set(DEFAULT_glog_TAG v0.3.5) ## ITK set(DEFAULT_ITK_URL https://github.com/InsightSoftwareConsortium/ITK.git) -set(DEFAULT_ITK_TAG v4.13.1) +set(DEFAULT_ITK_TAG v4.13.3)
removed!
Edoardo Pasca
Nov 5, 2021, 12:03:43 PM11/5/21
to SyneRBI/SIRF-SuperBuild, Push
@paskino pushed 1 commit.
- d493c2f removed bump of ITK version [ci skip]
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or unsubscribe.
Edoardo Pasca
Nov 5, 2021, 12:04:03 PM11/5/21
to SyneRBI/SIRF-SuperBuild, Subscribed
@paskino commented on this pull request.
In SuperBuild/External_Gadgetron.cmake:
> +if (NOT HDF5_CMAKE_ARGS)
+ message( FATAL_ERROR "HDF5_CMAKE_ARGS not found: ${HDF5_CMAKE_ARGS}")
+else()
+ message( STATUS "HDF5_CMAKE_ARGS found: ${HDF5_CMAKE_ARGS}")
+endif()
+
removed
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Edoardo Pasca
Nov 5, 2021, 12:04:32 PM11/5/21
to SyneRBI/SIRF-SuperBuild, Subscribed
> @@ -107,12 +116,13 @@ if(NOT ( DEFINED "USE_SYSTEM_${externalProjName}" AND "${USE_SYSTEM_${externalPr
${PYTHONLIBS_CMAKE_ARGS}
-DGTEST_ROOT:PATH=${GTEST_ROOT}
${HDF5_CMAKE_ARGS}
+ -DHDF5_FOUND:BOOL=ON
removed in fact
Kris Thielemans
Nov 6, 2021, 2:09:14 PM11/6/21
to SyneRBI/SIRF-SuperBuild, Subscribed
@KrisThielemans commented on this pull request.
In docker/build_gadgetron-ubuntu.sh:
> +apt-get upgrade -y && \ +apt-get dist-upgrade -y && \
I really don't like this...
Kris Thielemans
Nov 6, 2021, 2:41:43 PM11/6/21
to SyneRBI/SIRF-SuperBuild, Subscribed
Not sure why Gadgetron requires coroutine. I've asked gadgetron/gadgetron#1019.
Edoardo Pasca
Nov 6, 2021, 7:24:00 PM11/6/21
to SyneRBI/SIRF-SuperBuild, Push
@paskino pushed 1 commit.
- f88187a add coroutine
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or unsubscribe.
Edoardo Pasca
Nov 6, 2021, 7:26:21 PM11/6/21
to SyneRBI/SIRF-SuperBuild, Subscribed
@paskino commented on this pull request.
removed
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Kris Thielemans
Nov 7, 2021, 2:45:20 AM11/7/21
to SyneRBI/SIRF-SuperBuild, Subscribed
ok, past coroutine. Now we need RockDB apparently.
One example script is here https://gist.github.com/srimaln91/bea81d8c5ba36a64b0cd1b3b5324f687
Edoardo Pasca
Nov 7, 2021, 9:08:16 AM11/7/21
to SyneRBI/SIRF-SuperBuild, Push
@paskino pushed 3 commits.
- 1c03393 simplify gadgetron dependencies
- 0e7fae3 add dependency for rocksdb
- 4ab0a97 add librocksdb-dev
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or unsubscribe.
Edoardo Pasca
Nov 7, 2021, 9:11:09 AM11/7/21
to SyneRBI/SIRF-SuperBuild, Subscribed
I'm trying with RocksDB from apt. Do you think we should add it to the SuperBuild?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Edoardo Pasca
Nov 7, 2021, 9:20:53 AM11/7/21
to SyneRBI/SIRF-SuperBuild, Push
@paskino pushed 2 commits.
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or unsubscribe.
Edoardo Pasca
Nov 7, 2021, 11:15:08 AM11/7/21
to SyneRBI/SIRF-SuperBuild, Subscribed
An error which has to do with ISMRMRD .
error: no match for ‘operator==’ (operand types are ‘std::__tuple_element_t<0, std::tuple<ISMRMRD::AcquisitionHeader, Gadgetron::hoNDArray<std::complex<float> >, std::optional<Gadgetron::hoNDArray<float> > > >’ {aka ‘const ISMRMRD::AcquisitionHeader’} and ‘std::__tuple_element_t<0, std::tuple<ISMRMRD::AcquisitionHeader, Gadgetron::hoNDArray<std::complex<float> >, std::optional<Gadgetron::hoNDArray<float> > > >’ {aka ‘const ISMRMRD::AcquisitionHeader’})
I tried to merge https://github.com/SyneRBI/SIRF-SuperBuild/tree/ismrmrd1421 here and even use ISMRMRD branch, but it still fails.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Kris Thielemans
Nov 7, 2021, 6:07:33 PM11/7/21
to SyneRBI/SIRF-SuperBuild, Subscribed
An error which has to do with ISMRMRD .
error: no match for ‘operator==’ (operand types are ‘std::__tuple_element_t<0, std::tuple<ISMRMRD::AcquisitionHeader, Gadgetron::hoNDArray<std::complex<float> >, std::optional<Gadgetron::hoNDArray<float> > > >’ {aka ‘const ISMRMRD::AcquisitionHeader’} and ‘std::__tuple_element_t<0, std::tuple<ISMRMRD::AcquisitionHeader, Gadgetron::hoNDArray<std::complex<float> >, std::optional<Gadgetron::hoNDArray<float> > > >’ {aka ‘const ISMRMRD::AcquisitionHeader’})
I tried to merge https://github.com/SyneRBI/SIRF-SuperBuild/tree/ismrmrd1421 here and even use ISMRMRD
branchlatest commit, but it still fails.
no idea. @dchansen, @hansenms any idea? This test run was with an old commit of ISMRMRD
https://github.com/ismrmrd/ismrmrd/commits/1066d90 but I suppose if it's too old, CMake should have picked up on it?
Michael Hansen
Nov 7, 2021, 7:45:44 PM11/7/21
to SyneRBI/SIRF-SuperBuild, Subscribed
@KrisThielemans, I am not really sure what the ask is. There is some code somewhere, which is trying to compare and acquisition to another acquisition, but there is no == operator for that. I cannot see where that code is or what the context is, so it is really impossible to provide any comments on this.
Edoardo Pasca
Nov 8, 2021, 2:48:46 AM11/8/21
to SyneRBI/SIRF-SuperBuild, Push
@paskino pushed 6 commits.
- 7e48a96 trying ISMRMRD 1.4.2.1
- 45f073a corrected version_config.cmake (missing DEFAULT_ISMRMRD_URL)
- f3a093f trying v1.4.2
- 4611d59 corrected version_config.cmake (SyneRBI/ismrmrd->ismrmrd/ismrmrd)
- f825390 Merge remote-tracking branch 'origin/ismrmrd1421' into build_Gadgetron_master
- 1c5855b update to latest ISMRMRD commit
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or unsubscribe.
David Christoffer Hansen
Nov 8, 2021, 3:35:53 AM11/8/21
to SyneRBI/SIRF-SuperBuild, Subscribed
Gadgetron doesn't really version check ISMRMRD (or indeed most of its dependencies, except Boost), so you wouldn't be getting an error.
In general, Gadgetron will use the latest release of ISMRMRD available, and it would be safest to update the two in sync.
The error you're getting is pretty straight forward: In the version of ISMRMRD you're using, Acquisitionheader didn't have an equality operator. Now it does. So now you can compare two Acquisitions in Gadgetron.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Edoardo Pasca
Nov 8, 2021, 3:43:21 AM11/8/21
to SyneRBI/SIRF-SuperBuild, Push
@paskino pushed 1 commit.
- 735898f use ISMRMRD master
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or unsubscribe.
Edoardo Pasca
Nov 8, 2021, 5:59:38 AM11/8/21
to SyneRBI/SIRF-SuperBuild, Subscribed
OK, great @dchansen that file builds now with ISMRMRD master and Gadgetron master.
Next problem
[ 63%] Building CXX object test/CMakeFiles/test_all.dir/nhlbi_compression_tests.cpp.o
/home/runner/work/SIRF-SuperBuild/SIRF-SuperBuild/build/sources/Gadgetron/test/nhlbi_compression_tests.cpp:194:25: error: expected constructor, destructor, or type conversion before ‘(’ token
194 | INSTANTIATE_TEST_SUITE_P(
| ^
/home/runner/work/SIRF-SuperBuild/SIRF-SuperBuild/build/sources/Gadgetron/test/nhlbi_compression_tests.cpp:236:25: error: expected constructor, destructor, or type conversion before ‘(’ token
236 | INSTANTIATE_TEST_SUITE_P(
| ^
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Kris Thielemans
Nov 8, 2021, 6:50:19 AM11/8/21
to SyneRBI/SIRF-SuperBuild, Subscribed
Relevant line is here https://github.com/gadgetron/gadgetron/blob/12ffc43debb9bad2e170713006d29dea78d966bf/test/nhlbi_compression_tests.cpp#L194
INSTANTIATE_TEST_SUITE_P is part of GTest http://google.github.io/googletest/reference/testing.html#INSTANTIATE_TEST_SUITE_P so I guess our GTest is out of date
Edoardo Pasca
Nov 8, 2021, 8:47:38 AM11/8/21
to SyneRBI/SIRF-SuperBuild, Push
@paskino pushed 1 commit.
- 0574327 bump GTest version
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or unsubscribe.
Edoardo Pasca
Nov 8, 2021, 9:30:13 AM11/8/21
to SyneRBI/SIRF-SuperBuild, Subscribed
A couple of steps further, I updated GTest, added JSON as prerequisite for Gadgetron but still fails https://github.com/SyneRBI/SIRF-SuperBuild/runs/4139944170?check_suite_focus=true#step:6:18829
Another point is, I'm currently using RocksDB from ubuntu, but it may not be a correct version? On the other hand the compilation fails both on Ubuntu 18.04 and 20.04
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Edoardo Pasca
Nov 11, 2021, 8:46:15 AM11/11/21
to SyneRBI/SIRF-SuperBuild, Push
@paskino pushed 1 commit.
- 7f3e38d bump Boost to 1.77.0
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or unsubscribe.
Edoardo Pasca
Nov 15, 2021, 12:50:23 PM11/15/21
to SyneRBI/SIRF-SuperBuild, Subscribed
The latest error comes from missing gadgetron.xml.example in the Gadgetron repo, gadgetron/gadgetron#1024
This build doesn't build Gadgetron tests, however trying that would fail linking with
[ 55%] Linking CXX executable test_all
cd /home/ofn77899/devel/bgm2/builds/Gadgetron/build/test && /usr/local/bin/cmake -E cmake_link_script CMakeFiles/test_all.dir/link.txt --verbose=1
/usr/bin/g++-9 -DBOOST_ERROR_CODE_HEADER_ONLY -DBOOST_SYSTEM_NO_DEPRECATED -fopenmp -Werror=return-type -Werror=terminate -Werror=non-virtual-dtor -Werror=format-overflow -O3 -DNDEBUG CMakeFiles/test_all.dir/tests.cpp.o CMakeFiles/test_all.dir/hoNDArray_elemwise_test.cpp.o CMakeFiles/test_all.dir/hoNDArray_blas_test.cpp.o CMakeFiles/test_all.dir/hoNDArray_utils_test.cpp.o CMakeFiles/test_all.dir/hoNDArray_reductions_test.cpp.o CMakeFiles/test_all.dir/read_writer_test.cpp.o CMakeFiles/test_all.dir/hoNDFFT_test.cpp.o CMakeFiles/test_all.dir/hoNFFT_test.cpp.o CMakeFiles/test_all.dir/hoNDWavelet_test.cpp.o CMakeFiles/test_all.dir/curveFitting_test.cpp.o CMakeFiles/test_all.dir/image_morphology_test.cpp.o CMakeFiles/test_all.dir/pattern_recognition_test.cpp.o CMakeFiles/test_all.dir/cmr_mapping_test.cpp.o CMakeFiles/test_all.dir/hoNDArray_linalg_test.cpp.o CMakeFiles/test_all.dir/core_test.cpp.o CMakeFiles/test_all.dir/threadpool_test.cpp.o CMakeFiles/test_all.dir/from_string_test.cpp.o CMakeFiles/test_all.dir/hoNDArrayView_test.cpp.o CMakeFiles/test_all.dir/ChannelAlgorithmsTest.cpp.o CMakeFiles/test_all.dir/cmr_strain_test.cpp.o CMakeFiles/test_all.dir/cmr_thickening_test.cpp.o CMakeFiles/test_all.dir/cmr_analytical_strain_test.cpp.o CMakeFiles/test_all.dir/hoSDC_test.cpp.o CMakeFiles/test_all.dir/nhlbi_compression_tests.cpp.o CMakeFiles/test_all.dir/gadgets/AcquisitionAccumulateTrigget_test.cpp.o CMakeFiles/test_all.dir/gadgets/FlagTriggerParsing_test.cpp.o -o test_all -Wl,-rpath,/home/ofn77899/devel/bgm2/builds/Gadgetron/build/core/readers:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/gadgets/mri_core:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/toolboxes/cmr:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/toolboxes/pattern_recognition:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/toolboxes/mri/sdc/cpu:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/core/writers:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/toolboxes/denoise:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/toolboxes/operators/cpu:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/toolboxes/mri_core:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/toolboxes/registration/optical_flow/cpu:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/toolboxes/dwt/cpu:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/toolboxes/image/cpu:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/toolboxes/klt/cpu:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/toolboxes/image_io:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/toolboxes/nfft/cpu:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/toolboxes/fft/cpu:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/toolboxes/core/cpu/math:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/toolboxes/core/cpu/hostutils:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/core:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/toolboxes/core/cpu:/home/ofn77899/devel/bgm2/builds/Gadgetron/build/toolboxes/log:/home/ofn77899/devel/bgm2/INSTALL/lib: ../core/readers/libgadgetron_core_readers.so.4.1.2 ../gadgets/mri_core/libgadgetron_mricore.so.4.1.2 ../toolboxes/cmr/libgadgetron_toolbox_cmr.so.4.1.2 ../toolboxes/pattern_recognition/libgadgetron_toolbox_pr.so.4.1.2 ../toolboxes/mri/sdc/cpu/libgadgetron_toolbox_cpusdc.so.4.1.2 /home/ofn77899/devel/bgm2/INSTALL/lib/libgtest.a ../core/writers/libgadgetron_core_writers.so.4.1.2 ../toolboxes/denoise/libgadgetron_toolbox_denoise.so.4.1.2 ../toolboxes/operators/cpu/libgadgetron_toolbox_cpuoperator.so.4.1.2 ../toolboxes/mri_core/libgadgetron_toolbox_mri_core.so.4.1.2 ../toolboxes/registration/optical_flow/cpu/libgadgetron_toolbox_cpureg.so.4.1.2 ../toolboxes/dwt/cpu/libgadgetron_toolbox_cpudwt.so.4.1.2 ../toolboxes/image/cpu/libgadgetron_toolbox_cpu_image.so.4.1.2 ../toolboxes/klt/cpu/libgadgetron_toolbox_cpuklt.so.4.1.2 ../toolboxes/image_io/libgadgetron_toolbox_image_analyze_io.so.4.1.2 ../toolboxes/nfft/cpu/libgadgetron_toolbox_cpunfft.so.4.1.2 ../toolboxes/fft/cpu/libgadgetron_toolbox_cpufft.so.4.1.2 ../toolboxes/core/cpu/math/libgadgetron_toolbox_cpucore_math.so.4.1.2 -llapacke /usr/lib/x86_64-linux-gnu/liblapacke.so /usr/lib/x86_64-linux-gnu/libopenblas.so /home/ofn77899/devel/bgm2/INSTALL/lib/libfftw3f.a /home/ofn77899/devel/bgm2/INSTALL/lib/libfftw3.a ../toolboxes/core/cpu/hostutils/libgadgetron_toolbox_hostutils.so.4.1.2 ../core/libgadgetron_core.so.4.1.2 ../toolboxes/core/cpu/libgadgetron_toolbox_cpucore.so.4.1.2 /home/ofn77899/devel/bgm2/INSTALL/lib/libismrmrd.so.1.5.0 /home/ofn77899/devel/bgm2/INSTALL/lib/libhdf5.so /home/ofn77899/devel/bgm2/INSTALL/lib/libboost_filesystem.so.1.78.0 /home/ofn77899/devel/bgm2/INSTALL/lib/libboost_atomic.so.1.78.0 ../toolboxes/log/libgadgetron_toolbox_log.so.4.1.2 /home/ofn77899/devel/bgm2/INSTALL/lib/libboost_system.so.1.78.0
cd /home/ofn77899/devel/bgm2/builds/Gadgetron/build/test && /usr/local/bin/cmake -D TEST_TARGET=test_all -D TEST_EXECUTABLE=/home/ofn77899/devel/bgm2/builds/Gadgetron/build/test/test_all -D TEST_EXECUTOR= -D TEST_WORKING_DIR=/home/ofn77899/devel/bgm2/builds/Gadgetron/build/test -D TEST_EXTRA_ARGS= -D TEST_PROPERTIES= -D TEST_PREFIX= -D TEST_SUFFIX= -D NO_PRETTY_TYPES=FALSE -D NO_PRETTY_VALUES=FALSE -D TEST_LIST=test_all_TESTS -D CTEST_FILE=/home/ofn77899/devel/bgm2/builds/Gadgetron/build/test/test_all[1]_tests.cmake -D TEST_DISCOVERY_TIMEOUT=5 -P /usr/local/share/cmake-3.17/Modules/GoogleTestAddTests.cmake
/home/ofn77899/devel/bgm2/builds/Gadgetron/build/test/test_all: error while loading shared libraries: libhdf5.so.100.1.0: cannot open shared object file: No such file or directory
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Kris Thielemans
Nov 15, 2021, 3:06:55 PM11/15/21
to SyneRBI/SIRF-SuperBuild, Subscribed
This build doesn't build Gadgetron tests, however trying that would fail linking with
cd /home/ofn77899/devel/bgm2/builds/Gadgetron/build/test && /usr/local/bin/cmake -D TEST_TARGET=test_all -D TEST_EXECUTABLE=/home/ofn77899/devel/bgm2/builds/Gadgetron/build/test/test_all -D TEST_EXECUTOR= -D TEST_WORKING_DIR=/home/ofn77899/devel/bgm2/builds/Gadgetron/build/test -D TEST_EXTRA_ARGS= -D TEST_PROPERTIES= -D TEST_PREFIX= -D TEST_SUFFIX= -D NO_PRETTY_TYPES=FALSE -D NO_PRETTY_VALUES=FALSE -D TEST_LIST=test_all_TESTS -D CTEST_FILE=/home/ofn77899/devel/bgm2/builds/Gadgetron/build/test/test_all[1]_tests.cmake -D TEST_DISCOVERY_TIMEOUT=5 -P /usr/local/share/cmake-3.17/Modules/GoogleTestAddTests.cmake /home/ofn77899/devel/bgm2/builds/Gadgetron/build/test/test_all: error while loading shared libraries: libhdf5.so.100.1.0: cannot open shared object file: No such file or directoryWhile libhdf5.so.100.1.0 does exist.
This is not a linking error but a run-time error (when executing the test). It is linking fine with /home/ofn77899/devel/bgm2/INSTALL/lib/libhdf5.so. I cannot check if that is a symbolic link to libhdf5.so.100.1.0. (in the same location). Did you set LD_LIBRARY_PATH (i.e. source env_sirf.sh) before running the test?
Edoardo Pasca
Nov 15, 2021, 3:38:51 PM11/15/21
to SyneRBI/SIRF-SuperBuild, Subscribed
ah, that's good. No, I haven't set the LD_LIBRARY_PATH as this happens during make Gadgetron.
I will set it.
Edoardo Pasca
Nov 15, 2021, 3:41:44 PM11/15/21
to SyneRBI/SIRF-SuperBuild, Subscribed
On my machine
ls -lh INSTALL/lib/libhdf5*
-rw-r--r-- 1 ofn77899 ofn77899 7.9M Nov 15 15:02 INSTALL/lib/libhdf5.a
-rw-r--r-- 1 ofn77899 ofn77899 851K Nov 15 15:02 INSTALL/lib/libhdf5_cpp.a
lrwxrwxrwx 1 ofn77899 ofn77899 22 Nov 15 15:02 INSTALL/lib/libhdf5_cpp.so -> libhdf5_cpp.so.100.1.0
lrwxrwxrwx 1 ofn77899 ofn77899 21 Nov 15 15:02 INSTALL/lib/libhdf5_cpp.so.100.1.0 -> libhdf5_cpp.so.1.10.1
-rw-r--r-- 1 ofn77899 ofn77899 527K Nov 15 15:02 INSTALL/lib/libhdf5_cpp.so.1.10.1
-rw-r--r-- 1 ofn77899 ofn77899 3.0K Nov 15 15:01 INSTALL/lib/libhdf5.settings
lrwxrwxrwx 1 ofn77899 ofn77899 18 Nov 15 15:02 INSTALL/lib/libhdf5.so -> libhdf5.so.100.1.0
lrwxrwxrwx 1 ofn77899 ofn77899 17 Nov 15 15:02 INSTALL/lib/libhdf5.so.100.1.0 -> libhdf5.so.1.10.1
-rw-r--r-- 1 ofn77899 ofn77899 4.1M Nov 15 15:02 INSTALL/lib/libhdf5.so.1.10.1
Kris Thielemans
Nov 15, 2021, 3:51:02 PM11/15/21
to SyneRBI/SIRF-SuperBuild, Subscribed
ah, that's good. No, I haven't set the
LD_LIBRARY_PATHas this happens duringmake Gadgetron.
They run a test during building? That's very non-standard...
Edoardo Pasca
Nov 16, 2021, 9:36:40 AM11/16/21
to SyneRBI/SIRF-SuperBuild, Push
@paskino pushed 3 commits.
- bf1ca35 build tests
- cfbbdb9 skip copying of missing config file
- caa540c pass LD_LIBRARY_DIR at BUILD step
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or unsubscribe.
Edoardo Pasca
Nov 16, 2021, 10:03:52 AM11/16/21
to SyneRBI/SIRF-SuperBuild, Subscribed
let me celebrate the last commit which meant that Gadgetron built!
The failure is related to Siemens_to_ISMRMRD
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Edoardo Pasca
Nov 16, 2021, 10:06:45 AM11/16/21
to SyneRBI/SIRF-SuperBuild, Push
@paskino pushed 1 commit.
- 631b777 bump siemens_to_ismrmrd commit
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or unsubscribe.
Edoardo Pasca
Nov 16, 2021, 12:02:51 PM11/16/21
to SyneRBI/SIRF-SuperBuild, Subscribed
I'm sure it built at some point! :(
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Kris Thielemans
Nov 16, 2021, 1:05:25 PM11/16/21
to SyneRBI/SIRF-SuperBuild, Subscribed
Random search on the error throws up google/googletest#2702 where they had the same illegal instruction which was related to RocksDB. Obviously that could be a red herring, but as Gadgetron uses that as well, just maybe it has the same cause. Not sure if RocksDB can be tested (or if that issue would allow you to find what the RocksDB problem was)
Edoardo Pasca
Nov 17, 2021, 4:55:54 AM11/17/21
to SyneRBI/SIRF-SuperBuild, Push
@paskino pushed 1 commit.
- 6b7a007 do not build testing for gadgetron
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or unsubscribe.
Kris Thielemans
Nov 19, 2021, 7:26:35 AM11/19/21
to SyneRBI/SIRF-SuperBuild, Subscribed
@KrisThielemans requested changes on this pull request.
In docker/build_gadgetron-ubuntu.sh:
> @@ -6,10 +6,10 @@ apt-get update -qq apt-get install -yq --no-install-recommends \ libhdf5-serial-dev \ libboost-dev libboost-chrono-dev \ - libboost-filesystem-dev libboost-thread-dev \ - libboost-date-time-dev libboost-regex-dev \ - libboost-program-options-dev libboost-atomic-dev \ - libboost-test-dev libboost-timer-dev \ + libboost-filesystem-dev libboost-thread-dev \ + libboost-date-time-dev libboost-regex-dev \ + libboost-program-options-dev libboost-atomic-dev \ + libboost-test-dev libboost-timer-dev \
don't we need coroutine and context now? Also in the VB script...
In patches/copy_file_if_not_exists.py:
> @@ -5,7 +5,7 @@
def copy_if_not_exists(src, dst):
from_file = os.path.abspath(src)
to_file = os.path.abspath(dst)
- if not os.path.exists(to_file):
+ if not os.path.exists(to_file) and os.path.exists(from_file):
not sure why you do this, but shouldn't you check then if the file is the same?
> else()
# Ubutnu 16.04 version should be fine
set(Boost_REQUIRED_VERSION 1.58.0)
endif()
- set(Boost_URL http://downloads.sourceforge.net/project/boost/boost/${Boost_VERSION}/boost_1_65_1.zip)
- set(Boost_MD5 9824a7a3e25c9d4fdf2def07bce8651c)
+ # set(Boost_URL http://downloads.sourceforge.net/project/boost/boost/${Boost_VERSION}/boost_1_71_0.zip)
+ # https://github.com/boostorg/math/pull/676
+ # https://github.com/boostorg/math/commit/720536a08e4e33639869e1b7a99d9ec923409c0b
+ # seems the bugfix entered Boost 1.78.0 which is not yet released, but probably 1.71.0 is patched on Ubuntu
+ set (Boost_URL https://sourceforge.net/projects/boost/files/boost/1.78.0.beta1/boost_1_78_0_b1.zip)
we'd need to create an issue to move to the released version then
> @@ -108,17 +113,19 @@ set(DEFAULT_NIFTYREG_TAG 99d584e2b8ea0bffe7e65e40c8dc818751782d92 ) set(DEFAULT_NIFTYREG_REQUIRED_VERSION 1.5.68) ## ISMRMRD -if (WIN32) - set(DEFAULT_ISMRMRD_URL https://github.com/SyneRBI/ismrmrd ) - set(DEFAULT_ISMRMRD_TAG program_options_fix) -else() - set(DEFAULT_ISMRMRD_URL https://github.com/ismrmrd/ismrmrd ) - set(DEFAULT_ISMRMRD_TAG v1.4.1) -endif() +set(DEFAULT_ISMRMRD_URL https://github.com/ismrmrd/ismrmrd ) +set(DEFAULT_ISMRMRD_TAG origin/master)
can't keep this when merging
> ## Gadgetron set(DEFAULT_Gadgetron_URL https://github.com/gadgetron/gadgetron ) -set(DEFAULT_Gadgetron_TAG b6191eaaa72ccca6c6a5fe4c0fa3319694f512ab) +set(DEFAULT_Gadgetron_TAG origin/master)
will have to fix before merging
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Edoardo Pasca
Nov 19, 2021, 8:46:40 AM11/19/21
to SyneRBI/SIRF-SuperBuild, Subscribed
@paskino commented on this pull request.
In docker/build_gadgetron-ubuntu.sh:
> @@ -6,10 +6,10 @@ apt-get update -qq apt-get install -yq --no-install-recommends \ libhdf5-serial-dev \ libboost-dev libboost-chrono-dev \ - libboost-filesystem-dev libboost-thread-dev \ - libboost-date-time-dev libboost-regex-dev \ - libboost-program-options-dev libboost-atomic-dev \ - libboost-test-dev libboost-timer-dev \ + libboost-filesystem-dev libboost-thread-dev \ + libboost-date-time-dev libboost-regex-dev \ + libboost-program-options-dev libboost-atomic-dev \ + libboost-test-dev libboost-timer-dev \
Currently I cannot compile with the version of Boost in Ubuntu 18.04, so this is irrelevant.
I need to start using 20.04.
Edoardo Pasca
Nov 19, 2021, 8:47:51 AM11/19/21
to SyneRBI/SIRF-SuperBuild, Subscribed
@paskino commented on this pull request.
In patches/copy_file_if_not_exists.py:
> @@ -5,7 +5,7 @@
def copy_if_not_exists(src, dst):
from_file = os.path.abspath(src)
to_file = os.path.abspath(dst)
- if not os.path.exists(to_file):
+ if not os.path.exists(to_file) and os.path.exists(from_file):
I do this because of gadgetron/gadgetron#1024
In practice in the new Gadgetron repo the example xml config is not there.
Edoardo Pasca
Nov 19, 2021, 8:49:14 AM11/19/21
to SyneRBI/SIRF-SuperBuild, Subscribed
@paskino commented on this pull request.
> ## Gadgetron set(DEFAULT_Gadgetron_URL https://github.com/gadgetron/gadgetron ) -set(DEFAULT_Gadgetron_TAG b6191eaaa72ccca6c6a5fe4c0fa3319694f512ab) +set(DEFAULT_Gadgetron_TAG origin/master)
yes, as with ISMRMRD we will need to choose a version or commit
Edoardo Pasca
Jan 10, 2022, 5:10:40 AM1/10/22
to SyneRBI/SIRF-SuperBuild, Subscribed
@paskino commented on this pull request.
In patches/copy_file_if_not_exists.py:
> @@ -5,7 +5,7 @@
def copy_if_not_exists(src, dst):
from_file = os.path.abspath(src)
to_file = os.path.abspath(dst)
- if not os.path.exists(to_file):
+ if not os.path.exists(to_file) and os.path.exists(from_file):
The file is no longer needed
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Kris Thielemans
Jan 14, 2022, 8:10:37 AM1/14/22
to SyneRBI/SIRF-SuperBuild, Subscribed
@KrisThielemans commented on this pull request.
> @@ -30,15 +30,20 @@ if (APPLE) # really should be checking for CLang
set(Boost_MD5 f4096c4583947b0eb103c8539f1623a3)
else()
# Use version in Ubuntu 18.04
change comment
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Kris Thielemans
Jan 14, 2022, 8:11:42 AM1/14/22
to SyneRBI/SIRF-SuperBuild, Subscribed
> @@ -108,17 +113,19 @@ set(DEFAULT_NIFTYREG_TAG 99d584e2b8ea0bffe7e65e40c8dc818751782d92 ) set(DEFAULT_NIFTYREG_REQUIRED_VERSION 1.5.68) ## ISMRMRD -if (WIN32) - set(DEFAULT_ISMRMRD_URL https://github.com/SyneRBI/ismrmrd ) - set(DEFAULT_ISMRMRD_TAG program_options_fix) -else() - set(DEFAULT_ISMRMRD_URL https://github.com/ismrmrd/ismrmrd ) - set(DEFAULT_ISMRMRD_TAG v1.4.1) -endif() +set(DEFAULT_ISMRMRD_URL https://github.com/ismrmrd/ismrmrd ) +set(DEFAULT_ISMRMRD_TAG origin/master)
change to v1.5.0 (i.e. merge master and resolve conflicts)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Kris Thielemans
Jan 21, 2022, 9:10:25 AM1/21/22
to SyneRBI/SIRF-SuperBuild, Subscribed
@KrisThielemans requested changes on this pull request.
> else()
# Ubutnu 16.04 version should be fine
set(Boost_REQUIRED_VERSION 1.58.0)
endif()
- set(Boost_URL http://downloads.sourceforge.net/project/boost/boost/${Boost_VERSION}/boost_1_65_1.zip)
- set(Boost_MD5 9824a7a3e25c9d4fdf2def07bce8651c)
+ # set(Boost_URL http://downloads.sourceforge.net/project/boost/boost/${Boost_VERSION}/boost_1_71_0.zip)
+ # https://github.com/boostorg/math/pull/676+ # https://github.com/boostorg/math/commit/720536a08e4e33639869e1b7a99d9ec923409c0b + # seems the bugfix entered Boost 1.78.0 which is not yet released, but probably 1.71.0 is patched on Ubuntu + set (Boost_URL https://sourceforge.net/projects/boost/files/boost/1.78.0.beta1/boost_1_78_0_b1.zip)
1.78 is released
> else()
# Ubutnu 16.04 version should be fine
set(Boost_REQUIRED_VERSION 1.58.0)
endif()
- set(Boost_URL http://downloads.sourceforge.net/project/boost/boost/${Boost_VERSION}/boost_1_65_1.zip)
- set(Boost_MD5 9824a7a3e25c9d4fdf2def07bce8651c)
+ # set(Boost_URL http://downloads.sourceforge.net/project/boost/boost/${Boost_VERSION}/boost_1_71_0.zip)
+ # https://github.com/boostorg/math/pull/676
+ # https://github.com/boostorg/math/commit/720536a08e4e33639869e1b7a99d9ec923409c0b
+ # seems the bugfix entered Boost 1.78.0 which is not yet released, but probably 1.71.0 is patched on Ubuntu
+ set (Boost_URL https://sourceforge.net/projects/boost/files/boost/1.78.0.beta1/boost_1_78_0_b1.zip)
we could switch to a git (recursive) checkout of https://github.com/boostorg/boost. Avoids having to mess with md5s
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Kris Thielemans
Nov 1, 2022, 11:44:15 AM11/1/22
to SyneRBI/SIRF-SuperBuild, Subscribed
@KrisThielemans requested changes on this pull request.
I'd like to get rid of the gcc9 work-arounds. People might be using clang or whatever anyway.
With any luck, their CMake checks compiler capabilities. We could potentially copy some stuff from them that checks this such that it would fall-over early. Or just tell people. Not sure
In SuperBuild/External_Gadgetron.cmake:
> @@ -55,14 +59,19 @@ if(NOT ( DEFINED "USE_SYSTEM_${externalProjName}" AND "${USE_SYSTEM_${externalPr
# BLAS
find_package(BLAS)
- if (APPLE AND NOT (CBLAS_LIBRARY AND CBLAS_INCLUDE_DIR))
- # if the variables don't exist, let the user set them.
- SET(CBLAS_LIBRARY "" CACHE FILEPATH "CBLAS library")
- SET(CBLAS_INCLUDE_DIR "" CACHE PATH "CBLAS include directory")
- message(FATAL_ERROR "Gadgetron needs CBLAS_LIBRARY and CBLAS_INCLUDE_DIR. If
- these variables do not exist in your CMake, create them manually. CBLAS_LIBRARY
- and CBLAS_INCLUDE_DIR should be FILEPATH and PATH, respectively, and live in
- /usr/local/Cellar/openblas/ if installed with \"brew install openblas\".")
+
+ message(STATUS "CBLAS ${CBLAS_LIBRARY} ${CBLAS_INCLUDE_DIR}")
+ if (NOT (CBLAS_LIBRARY AND CBLAS_INCLUDE_DIR))
+ if (APPLE)
I guess I'd remove this. Probably useful for other systems as well. Although see below
In SuperBuild/External_Gadgetron.cmake:
> - -DCBLAS_INCLUDE_DIR:PATH=${CBLAS_INCLUDE_DIR}
- -DCBLAS_LIBRARY:FILEPATH=${CBLAS_LIBRARY}
this removal means that the APPLE stuff above will be ignored.
This might take some more thought. (maybe create a CBLAS_FLAGS variable which defaults to empty).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Edoardo Pasca
Nov 1, 2022, 12:36:25 PM11/1/22
to SyneRBI/SIRF-SuperBuild, Subscribed
@KrisThielemans I updated to the latest master, but the build fails miserably at CMake time in Gadgetron:
CMake Error at CMakeLists.txt:279 (find_package): By not providing "Findcpr.cmake" in CMAKE_MODULE_PATH this project has asked CMake to find a package configuration file provided by "cpr", but CMake did not find one. Could not find a package configuration file provided by "cpr" with any of the following names: cprConfig.cmake cpr-config.cmake Add the installation prefix of "cpr" to CMAKE_PREFIX_PATH or set "cpr_DIR" to a directory containing one of the above files. If "cpr" provides a separate development package or SDK, be sure it has been installed. -- Configuring incomplete, errors occurred!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Kris Thielemans
Nov 1, 2022, 12:44:39 PM11/1/22
to SyneRBI/SIRF-SuperBuild, Subscribed
ah well, I guess we can try to get through it with the system curl for the moment?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Kris Thielemans
Nov 1, 2022, 12:48:37 PM11/1/22
to SyneRBI/SIRF-SuperBuild, Subscribed
probably disable the docker CI for this PR for a while longer.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
It is loading more messages.
0 new messages