defer installation of some CIL run requirements after build #937

paskino · 2025-02-11T14:40:31Z

fixes curl linking problem for ISMRMD with system HDF5 on Ubuntu 22.04 #935

Defers the installation of CIL run requirements which confuse CMake.

paskino · 2025-02-11T16:10:05Z

There is an issue with Gadgetron or dcmtk and GCC? https://github.com/SyneRBI/SIRF-SuperBuild/actions/runs/13265453075/job/37031643450?pr=937

/usr/include/dcmtk/ofstd/ofstdinc.h:121:2: error: #error "Macro INCLUDE_CSTRING not supported anymore. Include <cstring> directly."
  121 | #error "Macro INCLUDE_CSTRING not supported anymore. Include <cstring> directly."

This is the error on my docker instance, potentially I haven't redirected stderr so the output seems incomplete.

#24 86.45 ninja: build stopped: subcommand failed.
#24 86.45 ninja: build stopped: subcommand failed.
#24 ERROR: process "/bin/bash -o pipefail -c BUILD_FLAGS=\"-G Ninja -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} -DSTIR_ENABLE_OPENMP=${STIR_ENABLE_OPENMP} -DUSE_SYSTEM_Armadillo=${USE_SYSTEM_Armadillo} -DUSE_SYSTEM_Boost=${USE_SYSTEM_Boost} -DUSE_SYSTEM_FFTW3=${USE_SYSTEM_FFTW3} -DUSE_SYSTEM_HDF5=${USE_SYSTEM_HDF5} -DUSE_ITK=${USE_ITK} -DUSE_SYSTEM_SWIG=${USE_SYSTEM_SWIG} -DUSE_NiftyPET=${USE_NiftyPET} -DBUILD_siemens_to_ismrmrd=${BUILD_siemens_to_ismrmrd} -DBUILD_pet_rd_tools=${BUILD_pet_rd_tools} -DGadgetron_USE_CUDA=${Gadgetron_USE_CUDA} -DBUILD_CIL=${BUILD_CIL}\"  EXTRA_BUILD_FLAGS=\"${EXTRA_BUILD_FLAGS}\"  bash /opt/scripts/user_sirf-ubuntu.sh  && fix-permissions /opt/SIRF-SuperBuild /opt/ccache" did not complete successfully: exit code: 1

KrisThielemans · 2025-02-11T16:53:22Z

There is. gadgetron/gadgetron#1277. However, this shouldn't occur on ubuntu 22.04. Does it say somewhere what version of dcmtk is being used?

paskino · 2025-02-12T08:27:55Z

Good spot! My docker image says it's 24.04

(base) root@2a418c82a9b7:~# lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 24.04.1 LTS
Release:        24.04
Codename:       noble

I suppose the base docker image has upgraded to 24.04 more or less recently without us noticying.

SIRF-SuperBuild/Dockerfile

Line 2 in 5f1a7f4

ARG BASE_CONTAINER=quay.io/jupyter/scipy-notebook:latest

We hit a similar issue but for a too old image with CIL.

paskino · 2025-02-12T15:21:44Z

On my docker instance with the ubuntu-22.04 base image it builds

(base) root@2035ef435116:/opt/SIRF-SuperBuild# lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 22.04.4 LTS
Release:        22.04
Codename:       jammy

KrisThielemans · 2025-02-12T21:55:13Z

Dockerfile

-  sed -r -i -e '/^\s*- (cil|ccpi-regulariser).*/d' /opt/scripts/requirements.yml; \
+  sed -r -i -e '/^\s*- (cil|ccpi-regulariser|h5py|dxchange).*/d' /opt/scripts/requirements.yml; \
+  else \
+    sed -r -i -e '/^\s*- (h5py|dxchange|cil|ccpi-regulariser|pillow|olefile|pywavelets|cil-data|tqdm|numba|zenodo_get).*/d' /opt/scripts/requirements.yml; \


I think SIRF-exercises do need numba and tqdm, zenodo_get would be useful as well

KrisThielemans · 2025-02-12T21:57:14Z

Why did you move the python setup later on GHA? GHA is using the system python and pip. This doesn't have anything to do with conda discussions/problems on docker.

By the way, why not pin the hdf5 version (both conda and pip I guess to be the same as the system one. Then it wouldn't matter which one is picked up. For conda, I suppose we'd still need to install cxx-compiler to avoid what we saw at #935 (comment)

paskino · 2025-02-13T10:01:00Z

Let's try to install hdf5 from conda/pip of the same version of the system one to minimise incompatibilities.

paskino · 2025-02-13T10:02:38Z

.github/workflows/c-cpp.yml

+    - uses: actions/setup-python@v5
+      with:
+        python-version: '${{ steps.deps.outputs.python-version }}'
+        cache: pip
+    - name: pip install
+      working-directory: docker
+      run: PYTHON_EXECUTABLE=python3 PYTHON_INSTALL_DIR="$HOME/virtualenv" ./user_python-ubuntu.sh
+    - uses: hendrikmuhs/ccache-action@v1
+      with:
+        key: ${{ matrix.os }}-${{ env.COMPILER }}-${{ matrix.compiler_version }}-${{ env.BUILD_TYPE }}-${{ github.ref_name }}-${{ github.run_id }}
+        restore-keys: |
+          ${{ matrix.os }}-${{ env.COMPILER }}-${{ matrix.compiler_version }}-${{ env.BUILD_TYPE }}-${{ github.ref_name }}
+          ${{ matrix.os }}-${{ env.COMPILER }}-${{ matrix.compiler_version }}-${{ env.BUILD_TYPE }}
+        append-timestamp: false


let's revert this once the same version of HDF5 that is in the system is installed via pip/conda.

… on docker

KrisThielemans

GHA and the VM re-use the docker scripts, so they are broken now.
I think you will need cxx-compilers for CMake
Logically speaking, it seems now to make more sense to add armadillo as a conda-forge package https://anaconda.org/conda-forge/armadillo, For pip, there seems to be pyarma https://pyarma.sourceforge.io/, which might be ok.

Seems we're on a long path here...

KrisThielemans · 2025-02-25T09:43:57Z

docker/requirements.yml

  - pillow            # cil
  - olefile >=0.46    # cil
  - pywavelets        # cil
  - cil-data >=21.3.0 # cil
  - ipp >=2021.10     # cil
  - tqdm              # cil
  - numba             # cil 
+  - h5py        # gadgetron and CIL
+  - hdf5        # gadgetron and CIL


and STIR actually

I meant, hdf5 is needed by STIR

KrisThielemans · 2025-02-25T09:55:42Z

CHANGES.md

- Udated Versions:
+- Docker image updates:
+  - defer installation of CIL requirements after build (#937)
+- Updated Versions:


looks like you need to list gadgetron update as well.

KrisThielemans · 2025-02-25T09:59:35Z

Dockerfile

@@ -25,19 +25,27 @@ ARG BUILD_CIL="OFF"
 COPY docker/requirements.yml /opt/scripts/
 # https://jupyter-docker-stacks.readthedocs.io/en/latest/using/common.html#conda-environments
 # https://github.com/TomographicImaging/CIL/blob/master/Dockerfile
+# First remove the CIL run-dependencies from requirements.yml as they install hdf5 and CMake gets confused


no longer correct comment?

not correct indeed, but it's still not passing unittests!

paskino · 2025-02-26T12:01:23Z

I tried armadillo from conda-forge but the superbuild didn't find the devel files.

Yes, as usual this is a long and painful road.

paskino · 2025-02-26T13:44:27Z

I get the following error on Gadgetron configuration. Indeed the system boost is 1.78.0.3 on Ubuntu 22.04

CMake Error at /usr/local/share/cmake-3.29/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find Boost: Found unsuitable version "1.74.0", but required is at
  least "1.80.0" (found /usr/include, found components: coroutine system
  date_time program_options filesystem timer context chrono)

KrisThielemans · 2025-03-07T08:24:51Z

The Gadgetron boost error will be because you update Gadgetron (which I told you to do). Let's keep it at the original version for now to get this through and see.

defer installation of some CIL run requirements after build

42c81af

- fixes SyneRBI#935

paskino added 3 commits February 12, 2025 08:35

use patched Gadgetron

561937d

revert gadgetron changes and use ubuntu22.04 for docker base image

b1c3014

add date to Gadgetron hash

b2f740c

paskino added 4 commits February 12, 2025 15:24

run GHA on Ubuntu 22.04

5f745dc

added changes

4ac8902

defer pip install in GHA

6d67322

added zenodo_get for CIL >=24.2.0

eeb1549

KrisThielemans reviewed Feb 12, 2025

View reviewed changes

paskino commented Feb 13, 2025

View reviewed changes

paskino added 6 commits February 13, 2025 14:44

install hdf5 version 1.10.7 as on Ubuntu 22.04

aa49ed2

revert change of pip install order

c1a6e38

try to find an appropriate version by relaxing patch level

6b20771

pip no use cache

97a4988

build h5py in pip

e3184a1

do not install hdf5 from ubuntu packages and force build of Armadillo…

48e0ef1

… on docker

KrisThielemans reviewed Feb 25, 2025

View reviewed changes

update GHA

8f89c5c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

defer installation of some CIL run requirements after build #937

defer installation of some CIL run requirements after build #937

paskino commented Feb 11, 2025

paskino commented Feb 11, 2025

KrisThielemans commented Feb 11, 2025

paskino commented Feb 12, 2025 •

edited

Loading

paskino commented Feb 12, 2025

KrisThielemans Feb 12, 2025

KrisThielemans commented Feb 12, 2025

paskino commented Feb 13, 2025 •

edited

Loading

paskino Feb 13, 2025

KrisThielemans left a comment

KrisThielemans Feb 25, 2025

KrisThielemans Mar 7, 2025

KrisThielemans Feb 25, 2025

KrisThielemans Feb 25, 2025

paskino Feb 26, 2025

paskino commented Feb 26, 2025

paskino commented Feb 26, 2025

KrisThielemans commented Mar 7, 2025

defer installation of some CIL run requirements after build #937

Are you sure you want to change the base?

defer installation of some CIL run requirements after build #937

Conversation

paskino commented Feb 11, 2025

paskino commented Feb 11, 2025

KrisThielemans commented Feb 11, 2025

paskino commented Feb 12, 2025 • edited Loading

paskino commented Feb 12, 2025

KrisThielemans Feb 12, 2025

Choose a reason for hiding this comment

KrisThielemans commented Feb 12, 2025

paskino commented Feb 13, 2025 • edited Loading

paskino Feb 13, 2025

Choose a reason for hiding this comment

KrisThielemans left a comment

Choose a reason for hiding this comment

KrisThielemans Feb 25, 2025

Choose a reason for hiding this comment

KrisThielemans Mar 7, 2025

Choose a reason for hiding this comment

KrisThielemans Feb 25, 2025

Choose a reason for hiding this comment

KrisThielemans Feb 25, 2025

Choose a reason for hiding this comment

paskino Feb 26, 2025

Choose a reason for hiding this comment

paskino commented Feb 26, 2025

paskino commented Feb 26, 2025

KrisThielemans commented Mar 7, 2025

paskino commented Feb 12, 2025 •

edited

Loading

paskino commented Feb 13, 2025 •

edited

Loading