[CCPPETMR/SIRF-SuperBuild] Add cil (#154)
Edoardo Pasca
Adds CCPi optimisation and regularisation.
Adds new iterative reconstruction algorithms from CIL. Currently tested with SPDHG.
Requires:
CCPPETMR/SIRF#237
CCPPETMR/SIRF#238
You can view, comment on, or merge this pull request online at:
https://github.com/CCPPETMR/SIRF-SuperBuild/pull/154
Commit Summary
- added CIL (ccpi-framework) to the dependency
- Adds CIL to the SuperBuild
- uses version_config to set ccpi/cil versions
- reflects changes to CCPi-RGL
- builds latest master
- points to spdhg branch
- Added CCPi-FrameworkPlugins as CIL dependency
- Merge branch 'master' into add_cil
- fix to CIL commit hash
- comments removed
- CCPi-Regularisation-Toolkit depends on Cython
- removed External_CIL.cmake
- removed reference to hackathor repo
File Changes
- A CMake/FindCython.cmake (49)
- M SuperBuild.cmake (7)
- A SuperBuild/External_CCPi-Framework.cmake (127)
- A SuperBuild/External_CCPi-FrameworkPlugins.cmake (127)
- A SuperBuild/External_CCPi-RGL.cmake (153)
- M version_config.cmake (32)
Patch Links:
- https://github.com/CCPPETMR/SIRF-SuperBuild/pull/154.patch
- https://github.com/CCPPETMR/SIRF-SuperBuild/pull/154.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
Kris Thielemans
@KrisThielemans requested changes on this pull request.
> else() - set(DEFAULT_SIRF_TAG v1.1.1) + set(DEFAULT_SIRF_TAG origin/spdhg_from_cil)
can't merge with this
In SuperBuild/External_CCPi-RGL.cmake:
> + message("CIL TAG " ${${proj}_TAG} )
+
+ # conda build should never get here
+ if("${PYTHON_STRATEGY}" STREQUAL "PYTHONPATH")
+ # in case of PYTHONPATH it is sufficient to copy the files to the
+ # $PYTHONPATH directory
+ set (BUILD_PYTHON ${PYTHONLIBS_FOUND})
+ if (BUILD_PYTHON)
+ set(PYTHON_DEST_DIR "" CACHE PATH "Directory of the SIRF Python modules")
+ if (PYTHON_DEST_DIR)
+ set(PYTHON_DEST "${PYTHON_DEST_DIR}")
+ else()
+ set(PYTHON_DEST "${CMAKE_INSTALL_PREFIX}/python")
+ endif()
+ message(STATUS "Python libraries found")
+ message(STATUS "SIRF Python modules will be installed in " ${PYTHON_DEST})
copy-paste mistake
> @@ -0,0 +1,49 @@ +# code adapted from +# https://github.com/symengine/symengine.py/blob/master/cmake/FindCython.cmake
Needs some reference to its license
https://github.com/symengine/symengine.py/blob/master/LICENSE
In SuperBuild.cmake:
> @@ -203,6 +203,13 @@ if (BUILD_petmr_rd_tools)
list(APPEND ${PRIMARY_PROJECT_NAME}_DEPENDENCIES petmr_rd_tools)
endif()
+if ("${PYTHON_STRATEGY}" STREQUAL "CONDA")
+ set (BUILD_CIL OFF)
+endif()
+if (BUILD_CIL)
needs an option
In SuperBuild/External_CCPi-Framework.cmake:
> +set(${proj}_BINARY_DIR "${SUPERBUILD_WORK_DIR}/builds/${proj}/build" )
+set(${proj}_DOWNLOAD_DIR "${SUPERBUILD_WORK_DIR}/downloads/${proj}" )
+set(${proj}_STAMP_DIR "${SUPERBUILD_WORK_DIR}/builds/${proj}/stamp" )
+set(${proj}_TMP_DIR "${SUPERBUILD_WORK_DIR}/builds/${proj}/tmp" )
+
+if(NOT ( DEFINED "USE_SYSTEM_${externalProjName}" AND "${USE_SYSTEM_${externalProjName}}" ) )
+ message(STATUS "${__indent}Adding project ${proj}")
+
+ ### --- Project specific additions here
+ set(libcilreg_Install_Dir ${SUPERBUILD_INSTALL_DIR})
+
+ #message(STATUS "HDF5_ROOT in External_SIRF: " ${HDF5_ROOT})
+ set(CMAKE_LIBRARY_PATH ${CMAKE_LIBRARY_PATH} ${SUPERBUILD_INSTALL_DIR})
+ set(CMAKE_INCLUDE_PATH ${CMAKE_INCLUDE_PATH} ${SUPERBUILD_INSTALL_DIR})
+
+ set(ENV{CIL_VERSION} 0.10.1)
weird of course. should be in version_config?
In SuperBuild/External_CCPi-FrameworkPlugins.cmake:
> @@ -0,0 +1,127 @@ +#======================================================================== +# Author: Benjamin A Thomas +# Author: Kris Thielemans +# Author: Edoardo Pasca +# Copyright 2017 University College London +# Copyright 2017 STFC
and 2018
In SuperBuild/External_CCPi-RGL.cmake:
> @@ -0,0 +1,153 @@ +#======================================================================== +# Author: Benjamin A Thomas +# Author: Kris Thielemans +# Author: Edoardo Pasca +# Copyright 2017 University College London +# Copyright 2017 STFC
and 2018
Edoardo Pasca
@paskino commented on this pull request.
> else() - set(DEFAULT_SIRF_TAG v1.1.1) + set(DEFAULT_SIRF_TAG origin/spdhg_from_cil)
Clearly not. It needs merging of CCPPETMR/SIRF#237 and CCPPETMR/SIRF#238 and then it'll go away.
This PR required changes in more or less 3 different repos. I wanted to make a working PR for this one. Once we start merging the other PRs this will be cleaned.
Edoardo Pasca
@paskino commented on this pull request.
In SuperBuild/External_CCPi-RGL.cmake:
> + message("CIL TAG " ${${proj}_TAG} )
+
+ # conda build should never get here
+ if("${PYTHON_STRATEGY}" STREQUAL "PYTHONPATH")
+ # in case of PYTHONPATH it is sufficient to copy the files to the
+ # $PYTHONPATH directory
+ set (BUILD_PYTHON ${PYTHONLIBS_FOUND})
+ if (BUILD_PYTHON)
+ set(PYTHON_DEST_DIR "" CACHE PATH "Directory of the SIRF Python modules")
+ if (PYTHON_DEST_DIR)
+ set(PYTHON_DEST "${PYTHON_DEST_DIR}")
+ else()
+ set(PYTHON_DEST "${CMAKE_INSTALL_PREFIX}/python")
+ endif()
+ message(STATUS "Python libraries found")
+ message(STATUS "SIRF Python modules will be installed in " ${PYTHON_DEST})
What do you refer to?
Edoardo Pasca
@paskino commented on this pull request.
In SuperBuild/External_CCPi-Framework.cmake:
> +set(${proj}_BINARY_DIR "${SUPERBUILD_WORK_DIR}/builds/${proj}/build" )
+set(${proj}_DOWNLOAD_DIR "${SUPERBUILD_WORK_DIR}/downloads/${proj}" )
+set(${proj}_STAMP_DIR "${SUPERBUILD_WORK_DIR}/builds/${proj}/stamp" )
+set(${proj}_TMP_DIR "${SUPERBUILD_WORK_DIR}/builds/${proj}/tmp" )
+
+if(NOT ( DEFINED "USE_SYSTEM_${externalProjName}" AND "${USE_SYSTEM_${externalProjName}}" ) )
+ message(STATUS "${__indent}Adding project ${proj}")
+
+ ### --- Project specific additions here
+ set(libcilreg_Install_Dir ${SUPERBUILD_INSTALL_DIR})
+
+ #message(STATUS "HDF5_ROOT in External_SIRF: " ${HDF5_ROOT})
+ set(CMAKE_LIBRARY_PATH ${CMAKE_LIBRARY_PATH} ${SUPERBUILD_INSTALL_DIR})
+ set(CMAKE_INCLUDE_PATH ${CMAKE_INCLUDE_PATH} ${SUPERBUILD_INSTALL_DIR})
+
+ set(ENV{CIL_VERSION} 0.10.1)
The environment variable is going to be removed from the requirements for the build of CCPi-Framework.
Matthias J. Ehrhardt
@mehrhardt commented on this pull request.
I don't think I can contribute to this piece of code. @KrisThielemans and @evgueni-ovtchinnikov should do this.
Kris Thielemans
@KrisThielemans commented on this pull request.
In SuperBuild/External_CCPi-RGL.cmake:
> + message("CIL TAG " ${${proj}_TAG} )
+
+ # conda build should never get here
+ if("${PYTHON_STRATEGY}" STREQUAL "PYTHONPATH")
+ # in case of PYTHONPATH it is sufficient to copy the files to the
+ # $PYTHONPATH directory
+ set (BUILD_PYTHON ${PYTHONLIBS_FOUND})
+ if (BUILD_PYTHON)
+ set(PYTHON_DEST_DIR "" CACHE PATH "Directory of the SIRF Python modules")
+ if (PYTHON_DEST_DIR)
+ set(PYTHON_DEST "${PYTHON_DEST_DIR}")
+ else()
+ set(PYTHON_DEST "${CMAKE_INSTALL_PREFIX}/python")
+ endif()
+ message(STATUS "Python libraries found")
+ message(STATUS "SIRF Python modules will be installed in " ${PYTHON_DEST})
SIRF Python module -> CIL?
Kris Thielemans
a note on copyright dates and notices. If you copied code from somewhere, you need to preserve its copyright data/notice. At the same time, you need to make sure it's up to date with your changes.
Seems therefore that you either remove Ben and me, or you keep the original date. sorry
Edoardo Pasca
@paskino pushed 2 commits.
- 3a0e11a update CIL versions to master
- 5617455 Merge branch 'add_cil' of https://github.com/CCPPETMR/SIRF-SuperBuild into add_cil
—
You are receiving this because you are subscribed to this thread.
Edoardo Pasca
@paskino pushed 4 commits.
- ceeddc0 Merge branch 'master' into add_cil
- a2e70b2 Merge branch 'add_cil' of https://github.com/CCPPETMR/SIRF-SuperBuild into add_cil
- 4c66176 Merge remote-tracking branch 'origin' into add_cil
- b431327 need to pass env variable during build
—
You are receiving this because you are subscribed to this thread.
Edoardo Pasca
@paskino pushed 17 commits.
- e244b1a updated Gadgetron and ISMRMRD tags
- 4205dbc Merge branch 'master' into ubuntu18.04
- a652614 Merge branch 'master' into ubuntu18.04
- 90740d5 set gadgetron version to 3.17.0
- d23f78c fix typo
- 85ca375 Merge branch 'gcc6' into ubuntu18.04
- a2f24b0 updated boost to 1.65.1 for Gadgetron
- edd8628 [TRAVIS] update gcc
- 7bfd43f [TRAVIS] on trusty, use gcc-7 with our own ACE
- b36b0fd Merge remote-tracking branch 'origin/master' into ubuntu18.04
- eb22e04 Merge branch 'master' into ubuntu18.04
- bbce834 Merge branch 'master' into ubuntu18.04
- 9db7f7d [TRAVIS] forgot to replace gcc-5 with gcc-6
- 101bb79 Merge remote-tracking branch 'origin' into ubuntu18.04
- 17514b5 Merge branch 'master' into ubuntu18.04
- 97ec1a4 Merge remote-tracking branch 'origin/ubuntu18.04' into add_cil
- db0cb7b Merge remote-tracking branch 'origin/master' into add_cil
—
You are receiving this because you are subscribed to this thread.
Edoardo Pasca
@paskino commented on this pull request.
> @@ -0,0 +1,49 @@ +# code adapted from +# https://github.com/symengine/symengine.py/blob/master/cmake/FindCython.cmake
https://github.com/CCPPETMR/SIRF-SuperBuild/blob/add_cil/CMake/FindCython.cmake#L3
Edoardo Pasca
@paskino pushed 2 commits.
—
You are receiving this because you are subscribed to this thread.
Edoardo Pasca
@paskino commented on this pull request.
In SuperBuild/External_CCPi-Framework.cmake:
> +set(${proj}_BINARY_DIR "${SUPERBUILD_WORK_DIR}/builds/${proj}/build" )
+set(${proj}_DOWNLOAD_DIR "${SUPERBUILD_WORK_DIR}/downloads/${proj}" )
+set(${proj}_STAMP_DIR "${SUPERBUILD_WORK_DIR}/builds/${proj}/stamp" )
+set(${proj}_TMP_DIR "${SUPERBUILD_WORK_DIR}/builds/${proj}/tmp" )
+
+if(NOT ( DEFINED "USE_SYSTEM_${externalProjName}" AND "${USE_SYSTEM_${externalProjName}}" ) )
+ message(STATUS "${__indent}Adding project ${proj}")
+
+ ### --- Project specific additions here
+ set(libcilreg_Install_Dir ${SUPERBUILD_INSTALL_DIR})
+
+ #message(STATUS "HDF5_ROOT in External_SIRF: " ${HDF5_ROOT})
+ set(CMAKE_LIBRARY_PATH ${CMAKE_LIBRARY_PATH} ${SUPERBUILD_INSTALL_DIR})
+ set(CMAKE_INCLUDE_PATH ${CMAKE_INCLUDE_PATH} ${SUPERBUILD_INSTALL_DIR})
+
+ set(ENV{CIL_VERSION} 0.10.1)
moved the variable to version_config as suggested
Kris Thielemans
@KrisThielemans requested changes on this pull request.
nearly done I think. thanks!
In SuperBuild/External_CCPi-FrameworkPlugins.cmake:
> + # if SETUP_PY one can launch the conda build.sh script setting
+ # the appropriate variables.
+ ExternalProject_Add(${proj}
+ ${${proj}_EP_ARGS}
+ GIT_REPOSITORY ${${proj}_URL}
+ GIT_TAG ${${proj}_TAG}
+ SOURCE_DIR ${${proj}_SOURCE_DIR}
+ BINARY_DIR ${${proj}_BINARY_DIR}
+ DOWNLOAD_DIR ${${proj}_DOWNLOAD_DIR}
+ STAMP_DIR ${${proj}_STAMP_DIR}
+ TMP_DIR ${${proj}_TMP_DIR}
+ INSTALL_DIR ${libcilreg_Install_Dir}
+
+ CONFIGURE_COMMAND ""
+ BUILD_COMMAND ""
+ INSTALL_COMMAND ${CMAKE_COMMAND} -E env CIL_VERSION=0.10.0 SRC_DIR=${${proj}_BINARY_DIR} RECIPE_DIR=${${proj}_SOURCE_DIR}/Wrappers/Python/conda-recipe PYTHON=${PYTHON_EXECUTABLE} bash ${${proj}_SOURCE_DIR}/Wrappers/Python/conda-recipe/build.sh
careful
> else() - set(DEFAULT_SIRF_TAG v1.1.1) + set(DEFAULT_SIRF_TAG origin/add_to_sirf_classes)
will need to handle this
In SuperBuild/External_CCPi-Framework.cmake:
> + # if SETUP_PY one can launch the conda build.sh script setting
+ # the appropriate variables.
+ ExternalProject_Add(${proj}
+ ${${proj}_EP_ARGS}
+ GIT_REPOSITORY ${${proj}_URL}
+ GIT_TAG ${${proj}_TAG}
+ SOURCE_DIR ${${proj}_SOURCE_DIR}
+ BINARY_DIR ${${proj}_BINARY_DIR}
+ DOWNLOAD_DIR ${${proj}_DOWNLOAD_DIR}
+ STAMP_DIR ${${proj}_STAMP_DIR}
+ TMP_DIR ${${proj}_TMP_DIR}
+ INSTALL_DIR ${libcilreg_Install_Dir}
+
+ CONFIGURE_COMMAND ""
+ BUILD_COMMAND ""
+ INSTALL_COMMAND ${CMAKE_COMMAND} -E env CIL_VERSION=0.10.0 SRC_DIR=${${proj}_BINARY_DIR} RECIPE_DIR=${${proj}_SOURCE_DIR}/Wrappers/Python/conda-recipe PYTHON=${PYTHON_EXECUTABLE} bash ${${proj}_SOURCE_DIR}/Wrappers/Python/conda-recipe/build.sh
careful
> ## STIR set(DEFAULT_STIR_URL https://github.com/UCL/STIR ) set(DEFAULT_STIR_TAG fd3a7576a11930856d6af50d217f17d4848c2bff) + ## Gadgetron
this need to be removed. Gadgetron and ISMRMRD should not be changed by this PR
In .travis.yml:
> @@ -27,6 +27,18 @@ cache: # shared between builds # i.e. -boost == -DUSE_SYSTEM_Boost=OFF, which means that Boost will be built. matrix: include: + - os: linux
these changes need to be removed.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
Edoardo Pasca
@paskino commented on this pull request.
In SuperBuild/External_CCPi-Framework.cmake:
> + # if SETUP_PY one can launch the conda build.sh script setting
+ # the appropriate variables.
+ ExternalProject_Add(${proj}
+ ${${proj}_EP_ARGS}
+ GIT_REPOSITORY ${${proj}_URL}
+ GIT_TAG ${${proj}_TAG}
+ SOURCE_DIR ${${proj}_SOURCE_DIR}
+ BINARY_DIR ${${proj}_BINARY_DIR}
+ DOWNLOAD_DIR ${${proj}_DOWNLOAD_DIR}
+ STAMP_DIR ${${proj}_STAMP_DIR}
+ TMP_DIR ${${proj}_TMP_DIR}
+ INSTALL_DIR ${libcilreg_Install_Dir}
+
+ CONFIGURE_COMMAND ""
+ BUILD_COMMAND ""
+ INSTALL_COMMAND ${CMAKE_COMMAND} -E env CIL_VERSION=0.10.0 SRC_DIR=${${proj}_BINARY_DIR} RECIPE_DIR=${${proj}_SOURCE_DIR}/Wrappers/Python/conda-recipe PYTHON=${PYTHON_EXECUTABLE} bash ${${proj}_SOURCE_DIR}/Wrappers/Python/conda-recipe/build.sh
currently pointing to v1.1.1
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
Edoardo Pasca
@paskino pushed 2 commits.
- acb8f23 build command update
- 6c3c182 Merge branch 'add_cil' of https://github.com/CCPPETMR/SIRF-SuperBuild into add_cil
Casper da Costa-Luis
Casper da Costa-Luis
Now getting a weird error during make:
Traceback (most recent call last):
File "/home/travis/build/CCPPETMR/SIRF-SuperBuild/builds/CCPi-RGL/build/Wrappers/Python/setup-regularisers.py", line 6, in <module>
from Cython.Distutils import build_ext
ImportError: No module named Cython.Distutils
even though cython was installed and found
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
Casper da Costa-Luis
@casperdcl pushed 1 commit.
- 20dea0c re-enable docker
—
You are receiving this because you are subscribed to this thread.
Casper da Costa-Luis
Richard Brown
-
I would change the cmake variable
cython_bintocython_EXECUTABLE(similar toSWIG_EXECUTABLEandPYTHON_EXECUTABLE). -
On OSX, I'm getting an openMP error:
/Users/rich/Documents/Code/SIRF-SuperBuild/Build/sources/CCPi-RGL/Core/regularisers_CPU/FGP_TV_core.h:25:10: fatal error:
'omp.h' file not found
#include "omp.h"
I haven't looked through the code, but presumably you need inspire yourself from BUILD_STIR_WITH_OPEN_MP (default to OFF for APPLE), which then needs to get passed through to CIL.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
Edoardo Pasca
@paskino pushed 3 commits.
- 9e95fee build the add_to_sirf_classes branch of SIRF
- a752ff6 Merge branch 'add_cil' of https://github.com/CCPPETMR/SIRF-SuperBuild into add_cil
- b6a201b merge master .travis.yml
Edoardo Pasca
I see the OSX problem, but I've never tested the regularisation on OSX. I guess I'll disable CIL for OSX.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
Kris Thielemans
@KrisThielemans requested changes on this pull request.
looks essentially fine. just a few tiny changes.
In SuperBuild/External_CCPi-Framework.cmake:
> +
+ else()
+ if(${USE_SYSTEM_${externalProjName}})
+ find_package(${proj} ${${externalProjName}_REQUIRED_VERSION} REQUIRED)
+ message("USING the system ${externalProjName}, set ${externalProjName}_DIR=${${externalProjName}_DIR}")
+ endif()
+ ExternalProject_Add_Empty(${proj} DEPENDS "${${proj}_DEPENDENCIES}"
+ SOURCE_DIR ${${proj}_SOURCE_DIR}
+ BINARY_DIR ${${proj}_BINARY_DIR}
+ DOWNLOAD_DIR ${${proj}_DOWNLOAD_DIR}
+ STAMP_DIR ${${proj}_STAMP_DIR}
+ TMP_DIR ${${proj}_TMP_DIR}
+ )
+ endif()
+
+ mark_as_superbuild(
we have this in various places, but here it definitely doesn't make sense (i.e. there's no such variable)
In SuperBuild/External_CCPi-Framework.cmake:
> + set(${proj}_INCLUDE_DIR ${${proj}_SOURCE_DIR})
+
+ add_test(NAME CIL_FRAMEWORK_TESTS_1
+ COMMAND ${PYTHON_EXECUTABLE} -m unittest discover -p test_Data*.py
+ WORKING_DIRECTORY ${${proj}_SOURCE_DIR}/Wrappers/Python/test)
+ add_test(NAME CIL_FRAMEWORK_TESTS_2
+ COMMAND ${PYTHON_EXECUTABLE} -m unittest discover -p test_algor*.p
+ WORKING_DIRECTORY ${${proj}_SOURCE_DIR}/Wrappers/Python/test)
+ add_test(NAME CIL_FRAMEWORK_TESTS_3
+ COMMAND ${PYTHON_EXECUTABLE} -m unittest discover -p test_run_*.py
+ WORKING_DIRECTORY ${${proj}_SOURCE_DIR}/Wrappers/Python/test)
+
+ else()
+ if(${USE_SYSTEM_${externalProjName}})
+ find_package(${proj} ${${externalProjName}_REQUIRED_VERSION} REQUIRED)
+ message("USING the system ${externalProjName}, set ${externalProjName}_DIR=${${externalProjName}_DIR}")
⬇️ Suggested change
- message("USING the system ${externalProjName}, set ${externalProjName}_DIR=${${externalProjName}_DIR}")
+ message("USING the system ${externalProjName}")
I don't think the _DIR variable is used at all (no CMake config)
In SuperBuild/External_CCPi-FrameworkPlugins.cmake:
> +
+ else()
+ if(${USE_SYSTEM_${externalProjName}})
+ find_package(${proj} ${${externalProjName}_REQUIRED_VERSION} REQUIRED)
+ message("USING the system ${externalProjName}, set ${externalProjName}_DIR=${${externalProjName}_DIR}")
+ endif()
+ ExternalProject_Add_Empty(${proj} DEPENDS "${${proj}_DEPENDENCIES}"
+ SOURCE_DIR ${${proj}_SOURCE_DIR}
+ BINARY_DIR ${${proj}_BINARY_DIR}
+ DOWNLOAD_DIR ${${proj}_DOWNLOAD_DIR}
+ STAMP_DIR ${${proj}_STAMP_DIR}
+ TMP_DIR ${${proj}_TMP_DIR}
+ )
+ endif()
+
+ mark_as_superbuild(
same comments as above
In SuperBuild/External_CCPi-RGL.cmake:
> + set_property(CACHE PYTHON_STRATEGY PROPERTY STRINGS PYTHONPATH SETUP_PY CONDA)
+
+ ExternalProject_Add(${proj}
+ ${${proj}_EP_ARGS}
+ GIT_REPOSITORY ${${proj}_URL}
+ GIT_TAG ${${proj}_TAG}
+ #GIT_TAG origin/cmaking
+ SOURCE_DIR ${${proj}_SOURCE_DIR}
+ BINARY_DIR ${${proj}_BINARY_DIR}
+ DOWNLOAD_DIR ${${proj}_DOWNLOAD_DIR}
+ STAMP_DIR ${${proj}_STAMP_DIR}
+ TMP_DIR ${${proj}_TMP_DIR}
+ INSTALL_DIR ${libcilreg_Install_Dir}
+ # apparently this is the only way to pass environment variables to
+ # external projects
+ CONFIGURE_COMMAND ${CMAKE_COMMAND} -E env CIL_VERSION=${CIL_VERSION} cmake ${${proj}_SOURCE_DIR} -DCMAKE_INSTALL_PREFIX=${libcilreg_Install_Dir}
This will fail on Windows surely. Just add a comment
In SuperBuild/External_CCPi-RGL.cmake:
> +
+ else()
+ if(${USE_SYSTEM_${externalProjName}})
+ find_package(${proj} ${${externalProjName}_REQUIRED_VERSION} REQUIRED)
+ message("USING the system ${externalProjName}, set ${externalProjName}_DIR=${${externalProjName}_DIR}")
+ endif()
+ ExternalProject_Add_Empty(${proj} DEPENDS "${${proj}_DEPENDENCIES}"
+ SOURCE_DIR ${${proj}_SOURCE_DIR}
+ BINARY_DIR ${${proj}_BINARY_DIR}
+ DOWNLOAD_DIR ${${proj}_DOWNLOAD_DIR}
+ STAMP_DIR ${${proj}_STAMP_DIR}
+ TMP_DIR ${${proj}_TMP_DIR}
+ )
+ endif()
+
+ mark_as_superbuild(
as above
In docker/user_sirf-ubuntu.sh:
> @@ -3,7 +3,7 @@
set -ev
INSTALL_DIR="${1:-/opt}"
# SIRF
-git clone https://github.com/CCPPETMR/SIRF-SuperBuild --recursive -b master $INSTALL_DIR/SIRF-SuperBuild
+git clone https://github.com/CCPPETMR/SIRF-SuperBuild --recursive -b add_cil $INSTALL_DIR/SIRF-SuperBuild
will have to change
> if (APPLE) # really should be checking for CLang
# Boost 1.65 contains a bug for recent Clang https://github.com/CCPPETMR/SIRF-SuperBuild/issues/170
set(Boost_VERSION 1.68.0)
set(Boost_REQUIRED_VERSION 1.66.0)
set(Boost_URL http://downloads.sourceforge.net/project/boost/boost/${Boost_VERSION}/boost_1_68_0.zip)
set(Boost_MD5 f4096c4583947b0eb103c8539f1623a3)
else()
- # Use version in Ubuntu 18.04
- set(Boost_VERSION 1.65.1)
- set(Boost_REQUIRED_VERSION 1.65.1)
- set(Boost_URL http://downloads.sourceforge.net/project/boost/boost/${Boost_VERSION}/boost_1_65_1.zip)
- set(Boost_MD5 9824a7a3e25c9d4fdf2def07bce8651c)
+ # Use version in Ubuntu 18.04
revert all these
> +else() + # set(DEFAULT_SIRF_TAG v1.1.1) + set (DEFAULT_SIRF_TAG origin/add_to_sirf_classes)
will need to change
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
Edoardo Pasca
I believe I addressed the requested changes.
Travis fails to build during test because some packages aren't found (specifically python PIL, which is installed). I'm unsure if and how to install with pip, help is welcome.
I created a BUILD_CIL and BUILD_CIL_LITE options. With the latter one installs CCPi-Framework, CCPi-FrameworkPlugins, CCPi-Regularisation-Toolkit. With the former one installs the additional X-ray CT engine ASTRA-toolbox, CCPi-Astra, TomoPhantom.
Currently these option are only available on Linux.
The things that are still to be changed are the SIRF TAG and CIL TAGs.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
Edoardo Pasca
STIR.py", line 1173
if not out is None:
^
TabError: inconsistent use of tabs and spaces in indentation
Kris Thielemans
I'm afraid I cannot accept a PR that makes most Travis jobs fail... I know there's OSX problems but this breaks even Linux.
@casperdcl would you be able to help with the pip problem?
Casper da Costa-Luis
@KrisThielemans ah this looks fun. Over a hundred commits and comments. Haven't been following. What's the pip problem?
Edoardo Pasca
@casperdcl I need the Python Image Library, PIL or pillow installed on travis.
it's installed but it's not found during tests. That's just the first build in the travis matrix.
You can have a look (if you want) in the log.
I guess the other failures are due to the previous comment.
Casper da Costa-Luis
@casperdcl pushed 1 commit.
- 420f1cf potential CIL dependency fix
—
You are receiving this because you are subscribed to this thread.
Casper da Costa-Luis
Casper da Costa-Luis
PIL's deprecated in favour of the Pillow fork. You were also installing dependencies in the system rather than the working python env.
Should be fixed now (there are other unrelated errors).
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
Edoardo Pasca
@paskino pushed 2 commits.
- 9f46c85 update SIRF branch
- a8ed0b0 Merge branch 'add_cil' of https://github.com/CCPPETMR/SIRF-SuperBuild into add_cil
—
You are receiving this because you are subscribed to this thread.
Casper da Costa-Luis
new problem:
https://travis-ci.org/CCPPETMR/SIRF-SuperBuild/jobs/590322738#L2087
TomoPhantom is finding python2.7 rather than the env's $PY_EXE
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
Edoardo Pasca
That's interesting. The way TomoPhantom finds python is the same as SIRF's
Casper da Costa-Luis
for the SuperBuild there's a -DPYTHON_EXECUTABLE=$PY_EXE which I assume is propagated to SIRF. You'll need the same for TomoPhantom
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
Casper da Costa-Luis
@casperdcl pushed 2 commits.
—
You are receiving this because you are subscribed to this thread.
Casper da Costa-Luis
sigh nvm just fixed it; you'll have to ensure TomoPhantom uses the env vars (432ff49) just as SIRF does.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
Edoardo Pasca
Thanks @casperdcl ! The last bit that fails is that travis builds the wrong branch of SIRF:
it should use add_to_sirf_classes
but in facts it uses add_to_sirf_classes_merge_master.
Does travis do some kind of caching?
Casper da Costa-Luis
before your comment edit, about SIRF tags, it's probably best to specify -DSIRF_TAG=... somewhere in .travis.yml amd revert before merge rather than mess with version_config
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
Edoardo Pasca
I see your point. I added an if (CIL_BUILD) in version_config that will allow when BUILD_CIL=ON to build the add_to_sirf_classes branch of SIRF rather than master or v2.0.0
There remain a few problems:
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
Edoardo Pasca
@casperdcl docker builds fail because of some further dependencies not met for astra