Skip to content

Commit

Permalink
Merge branch 'master' into ML_estimate_component_based_normalisation_…
Browse files Browse the repository at this point in the history
…redesign
  • Loading branch information
robbietuk authored Dec 22, 2024
2 parents cbe81ec + 7508bab commit fd194fc
Show file tree
Hide file tree
Showing 49 changed files with 3,236 additions and 178 deletions.
59 changes: 37 additions & 22 deletions .github/workflows/build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,16 @@ jobs:
strategy:
matrix:
include:
- os: ubuntu-latest
- os: ubuntu-24.04
compiler: gcc
compiler_version: 9
# compiler_version: 9
cuda_version: "0"
BUILD_FLAGS: "-DSTIR_OPENMP=ON"
BUILD_TYPE: "Release"
parallelproj: "ON"
ROOT: "ON"
ITK: "OFF"
- os: ubuntu-latest
- os: ubuntu-24.04
compiler: clang
#compiler_version:
cuda_version: "0"
Expand All @@ -57,7 +57,7 @@ jobs:
ROOT: "OFF"
# currently using ITK 5.2 which doesn't like clang 14
ITK: "OFF"
- os: ubuntu-latest
- os: ubuntu-24.04
compiler: gcc
compiler_version: 10
cuda_version: "0"
Expand All @@ -66,19 +66,20 @@ jobs:
parallelproj: "OFF"
ROOT: "OFF"
ITK: "ON"
- os: ubuntu-latest
- os: ubuntu-24.04
compiler: gcc
compiler_version: 12
compiler_version: 14
cuda_version: "0"
BUILD_FLAGS: "-DSTIR_OPENMP=ON -DCMAKE_CXX_STANDARD=20"
BUILD_TYPE: "RelWithDebInfo"
parallelproj: "ON"
ROOT: "OFF"
ITK: "ON"
- os: ubuntu-latest
- os: ubuntu-24.04
compiler: gcc
compiler_version: 12
cuda_version: "12.1.0"
# currently CUDA doesn't support gcc 14 yet
compiler_version: 13
cuda_version: "12.6.1"
BUILD_FLAGS: "-DSTIR_OPENMP=ON -DCMAKE_CXX_STANDARD=17"
BUILD_TYPE: "Release"
parallelproj: "ON"
Expand Down Expand Up @@ -180,7 +181,7 @@ jobs:
echo CXX="$CXX" >> $GITHUB_ENV
- if: matrix.cuda_version != '0'
uses: Jimver/cuda-toolkit@v0.2.11
uses: Jimver/cuda-toolkit@v0.2.19
id: cuda-toolkit
with:
cuda: ${{ matrix.cuda_version }}
Expand Down Expand Up @@ -234,13 +235,12 @@ jobs:
# brew install openblas
# export OPENBLAS=$(brew --prefix openblas)
#python -m pip install --no-cache-dir --no-binary numpy numpy # avoid the cached .whl!
python -m pip install numpy
python -m pip install numpy pytest
;;
(*)
python -m pip install numpy
python -m pip install numpy pytest
;;
esac
python -m pip install pytest
if test "${{matrix.parallelproj}}XX" == "ONXX"; then
git clone --depth 1 --branch v1.7.3 https://github.com/gschramm/parallelproj
Expand All @@ -255,13 +255,23 @@ jobs:
rm -rf parallelproj
fi
# Install ROOT (warning: currently only valid on Ubuntu)
# Install ROOT (warning: brittle due to OS versions etc)
if test "${{matrix.ROOT}}XX" == "ONXX"; then
ROOT_file=root_v6.28.12.Linux-ubuntu20-x86_64-gcc9.4.tar.gz
case ${{matrix.os}} in
(ubuntu*)
sudo apt install libtbb-dev libvdt-dev libgif-dev
ROOT_file=root_v6.34.00.Linux-ubuntu24.04-x86_64-gcc13.2.tar.gz
#root_v6.34.00.Linux-ubuntu24.10-x86_64-gcc14.2.tar.gz
;;
(macOS*)
ROOT_file=https://root.cern/download/root_v6.34.00.macos-15.1-arm64-clang160.tar.gz
;;
esac
wget https://root.cern/download/"$ROOT_file"
tar -xzvf "$ROOT_file"
rm "$ROOT_file"
source root/bin/thisroot.sh
echo ROOTSYS="$ROOTSYS" >> $GITHUB_ENV
fi
# we'll install some dependencies with shared libraries, so need to let the OS know
Expand Down Expand Up @@ -303,7 +313,11 @@ jobs:
CMAKE_INSTALL_PREFIX=${GITHUB_WORKSPACE}/install
# make available to jobs below
echo CMAKE_INSTALL_PREFIX="$CMAKE_INSTALL_PREFIX" >> $GITHUB_ENV
EXTRA_BUILD_FLAGS="-DBUILD_SWIG_PYTHON=ON -DPYTHON_EXECUTABLE=`which python`"
if [ -n "$ROOTSYS" ]; then
# make sure we find ROOT (and vdt, which is installed in the same place)
EXTRA_BUILD_FLAGS=-DCMAKE_PREFIX_PATH:PATH="$ROOTSYS"
fi
EXTRA_BUILD_FLAGS="${EXTRA_BUILD_FLAGS} -DBUILD_SWIG_PYTHON=ON -DPython_EXECUTABLE=`which python`"
EXTRA_BUILD_FLAGS="${EXTRA_BUILD_FLAGS} -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX} -DCMAKE_BUILD_TYPE=${BUILD_TYPE}"
EXTRA_BUILD_FLAGS="${EXTRA_BUILD_FLAGS} -DDOWNLOAD_ZENODO_TEST_DATA=ON"
EXTRA_BUILD_FLAGS="${EXTRA_BUILD_FLAGS} -DDISABLE_STIR_LOCAL=OFF -DSTIR_LOCAL=${GITHUB_WORKSPACE}/examples/C++/using_STIR_LOCAL"
Expand All @@ -321,6 +335,13 @@ jobs:
;;
esac
# Enable tmate debugging of manually-triggered workflows if the input option was provided
- name: Setup tmate session if triggered
#if: ${{ failure() }}
uses: mxschmitt/action-tmate@v3
timeout-minutes: 30
if: ${{ github.event_name == 'workflow_dispatch' && inputs.debug_enabled == 'true' }}

- name: build
shell: bash
env:
Expand All @@ -330,12 +351,6 @@ jobs:
source ${GITHUB_WORKSPACE}/my-env/bin/activate
cmake --build . -j 2 --config ${BUILD_TYPE}} --target install
# Enable tmate debugging of manually-triggered workflows if the input option was provided
- name: Setup tmate session if triggered
uses: mxschmitt/action-tmate@v3
timeout-minutes: 15
if: ${{ github.event_name == 'workflow_dispatch' && inputs.debug_enabled == 'true' }}

- name: ctest
shell: bash
env:
Expand Down
2 changes: 2 additions & 0 deletions .mailmap
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ Georg Schramm <georg.schramm@kuleuven.be> <40211162+gschramm@users.noreply.githu
Georg Schramm <georg.schramm@kuleuven.be> <georg.schramm@posteo.net>
Georg Schramm <georg.schramm@kuleuven.be> <georg.schramm@kuleuven.be>
Nicole Jurjew <nicole.jurjew.20@ucl.ac.uk>
Nicole Jurjew <nicole.jurjew.20@ucl.ac.uk> <86268422+NicoleJurjew@users.noreply.github.com>
Tahereh Niknejad <tahereh.niknejad@tecnico.ulisboa.pt>
Tahereh Niknejad <tahereh.niknejad@tecnico.ulisboa.pt> <tahereh@Taherehs-MacBook-Pro.local>
Sam D Porter <92305641+samdporter@users.noreply.github.com>
Expand All @@ -70,3 +71,4 @@ Matthew Strugari <matthew.strugari@dal.ca>
Matthew Strugari <matthew.strugari@dal.ca> <56315593+mastergari@users.noreply.github.com>
Imraj Singh <imraj.singh.20@ucl.ac.uk>
Imraj Singh <imraj.singh.20@ucl.ac.uk> <72553490+Imraj-Singh@users.noreply.github.com>
Dimitra Kyriakopoulou <dimitra.kyriakopoulou.13@ucl.ac.uk>
10 changes: 5 additions & 5 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
repos:
- repo: https://github.com/doublify/pre-commit-clang-format
rev: '62302476'
hooks:
- id: clang-format
files: \.(c|cc|cxx|cpp|cu|h|hpp|hxx|inl|txx)$
- repo: https://github.com/pre-commit/mirrors-clang-format
rev: v14.0.6
hooks:
- id: clang-format
files: \.(c|cc|cxx|cpp|cu|h|hpp|hxx|inl|txx)$
8 changes: 6 additions & 2 deletions documentation/devel/editor-settings.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Developer documentation: editor settings

White-spaces and indentation with multiple developers are a pain. Please adhere to
our white-space policy, which we try to enforce via [clang-format](https://clang.llvm.org/docs/ClangFormat.html).
Check that site for integration with your editor/IDE.
our white-space policy, which we enforce via [pre-commit](https://pre-commit.com/), including
running of [clang-format](https://clang.llvm.org/docs/ClangFormat.html).
See [git-hooks.md](git-hooks.md) for more information.

Developer experience will be best if you set your editor/IDE to use the same settings,
including versions of the formatters (`clang-format` in particular).
22 changes: 19 additions & 3 deletions documentation/devel/git-hooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,27 @@
We use [pre-commit](https://pre-commit.com) to check coding conventions, including
white-space formatting rules.

Unfortunately, exact formatting used depends on the `clang-format` version. Therefore,
you do have to install the same version as what is currently used in our GitHub Action.
Unfortunately, exact formatting used depends on the `clang-format` (and others?) version. Therefore,
you might need to install the same version as what is currently used in our
[.pre-commit-config.yaml](../../.pre-commit-config.yaml) for your editor to pick up
the same version.

## Installation of software

### TL;DR

```sh
pip/conda install pre-commit
git add blabla
pre-commit run
# optionally check files
# "add" changes made by the hooks
git add blabla
git commit
```

### More detail

We highly recommend to use `conda`:
```sh
cd /whereever/STIR
Expand Down Expand Up @@ -48,4 +64,4 @@ or one-off with

git commit --no-verify

You will then need to run `pre-commit run --all-files` when done before we will merge your pull request.
You will then need to run `pre-commit run --all-files` when done before we will merge your pull request.
55 changes: 51 additions & 4 deletions documentation/release_6.3.htm
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,21 @@ <h2> Summary for end users (also to be read by developers)</h2>

<h3>New functionality</h3>
<ul>
<li>
The analytic Spline Reconstruction Technique (SRT) algorithm has been added in 2 different versions: for PET
(inverting the 2D Radon transform) and for SPECT (inverting the 2D attenuated Radon transform). The latter
allows quantitatively correct analytic reconstruction of SPECT data (after scatter correction).

The reference for the implemented algorithms is:<br>
Fokas, A. S., A. Iserles, and V. Marinakis.
<cite>Reconstruction algorithm for single photon emission computed tomography and its numerical implementation.</cite>
Journal of the Royal Society Interface* 3.6 (2006): 45-54.
<br>
The reference for the implementation is:<br>
Dimitra Kyriakopoulou, <cite>Analytical and Numerical Aspects of Tomography</cite>, UCL PhD thesis, (not yet publically accessible)
<br>
<a href=https://github.com/UCL/STIR/pull/1420>PR #1420</a>
</li>
<li>
<code>ScatterSimulation</code> can now downsample the scanner transaxially (crystals per ring) for <code>BlocksOnCylindrical</code>,
scanners, which speeds up <code>ScatterEstimation</code> considerably. By default, downsampling the detectors per reading
Expand All @@ -30,22 +45,50 @@ <h3>New functionality</h3>
However, projection data is currently still always returned as non-TOF (but list-mode data is read as TOF).<br>
<a href=https://github.com/UCL/STIR/pull/1503>PR #1503</a>
</li>
<li>
<tt>stir_timings</tt> has now an extra option to parse a par-file for a projector-pair.
</li>
<li>
Added the ability to set a forward projector for mask projection in the <code>ScatterEstimation</code> class.<br>
<a href=https://github.com/UCL/STIR/pull/1530>PR #1530</a>
</li>
<li>
Duration in sinogram interfile/exam_info obtained from <tt>lm_to_projdata</tt> has the correct value if we unlist all the events. This is not true for ROOT files<br>
<a href=https://github.com/UCL/STIR/pull/1519>PR #1519</a>
</li>
</ul>


<h3>Changed functionality</h3>

<li>
Rework <code>MLEstimateComponentBasedNormalisation</code> into a class.
This allows for more flexibility in the use of the class without calling CLI executables. <br>
<a href=https://github.com/UCL/STIR/pull/1499>PR # 1499</a>
</li>

<ul>
<li>
Default ECAT scanner configurations updated to use a negative intrinsic tilt.
</li>
</ul>
<h3>Bug fixes</h3>

<ul>
<li>
Fixed a bug in the scatter estimation code (introduced in release 5.1.0) if input data is 3D and "cylindrical" (there was no bug for "blocksoncylindrical" data).
The scatter estimation runs on data constructed via SSRB. However, the attenuation correction factors were incorrectly obtained with adding of oblique segments (as opposed to averaging).
This resulted in intermediate images that had the wrong attenuation correction which were approximately num_segments times larger. This was compensated by the tail-fitting, but resulted in unexpected scale factors (scale factors were around 1/num_segments times what was expected).
This means that if you used the "min/max scale factor" feature in the scatter estimate, you will have to adjust your threshold values. Expected scatter tail-fitting scale factors should now be restored to ~1-1.5 (depending on the amount of multiple and out-of-FOV scatter).
See <a href="https://github.com/UCL/STIR/issues/1532">Issue #1532</a> for more detail. Fixed by using averaging functionality of SSRB instead of adding segments for attenuation correction factors.
<a href=https://github.com/UCL/STIR/pull/1531>PR #1531</a>
</li>
</ul>

<h3>Build system</h3>

<ul>
<li>Enable more diagnostics in CMake when finding CERN's ROOT (we used to silence them)<br>
<a href=https://github.com/UCL/STIR/pull/1552>PR #1552</a>
</li>
</ul>

<h3>Known problems</h3>
<p>See <a href=https://github.com/UCL/STIR/labels/bug>our issue tracker</a>.</p>
Expand All @@ -63,7 +106,11 @@ <h3>Changed functionality</h3>


<h3>Bug fixes</h3>

<ul>
<li>Fixed minor incompatibility with gcc-14 and clang-18 buy adding an extra include file<br>
<a href=https://github.com/UCL/STIR/pull/1552>PR #1552</a>
</li>
</ul>

<h3>Other code changes</h3>

Expand Down
18 changes: 18 additions & 0 deletions examples/samples/SRT2D.par
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
SRT2DParameters :=
input file := sino_hof_no_noise.hs

xy output image size (in pixels) := -1
z output image size (in pixels) := -1
zoom := 1

;post-filter type := Wiener
;Wiener Filter Parameters :=
;End Wiener Filter Parameters :=

;post-filter type := Gamma
;Gamma Filter Parameters :=
;End Gamma Filter Parameters :=

output filename prefix := SRT2D

END :=
20 changes: 20 additions & 0 deletions examples/samples/SRT2DSPECT.par
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
SRT2DSPECTParameters :=

;SRT2DSPECT receives as input the forward projection of the attenuation image (attensino.hs) and the measured emission sinogram (sino.hs).
input file := sino.hs
attenuation projection filename := attensino.hs

xy output image size (in pixels) := -1

;post-filter type := Wiener
;Wiener Filter Parameters :=
;End Wiener Filter Parameters :=

;post-filter type := Gamma
;Gamma Filter Parameters :=
;End Gamma Filter Parameters :=

output filename prefix := srt_recon


end:=
Loading

0 comments on commit fd194fc

Please sign in to comment.