-
Notifications
You must be signed in to change notification settings - Fork 69
Tigre algorithm wrapping #2158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Tigre algorithm wrapping #2158
Changes from 27 commits
3f46e3e
c622590
f9966f6
f0352d6
dc75e11
328597f
2703b11
2d29ce5
8f024e8
55ed113
0f9423b
fdc87b2
8b3dd6a
19ba47f
4fe7570
1ba41d9
ce3d619
c5e77d4
81d1352
38b6e5e
bd842cb
edabaa1
d23bf83
99186cb
f44839d
c65ae06
9f5a4f8
c75adc6
643deba
9c6bca2
e4fd877
6f1e6f3
dd4406c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,219 @@ | ||
| # Copyright 2025 United Kingdom Research and Innovation | ||
| # Copyright 2025 The University of Manchester | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| # | ||
| # Authors: | ||
| # CIL Developers, listed at: https://github.com/TomographicImaging/CIL/blob/master/NOTICE.txt | ||
|
|
||
| from cil.recon import Reconstructor | ||
| try: | ||
| import tigre.algorithms as algs | ||
| from cil.plugins.tigre import CIL2TIGREGeometry | ||
| except ImportError: | ||
| raise ImportError("TIGRE is not installed. Please install it to use this module.") | ||
| from cil.framework import ImageData | ||
| import logging | ||
| import numpy as np | ||
| import warnings | ||
| from cil.framework.labels import AcquisitionDimension | ||
|
|
||
| log = logging.getLogger(__name__) | ||
|
|
||
| try: | ||
| from tigre.utilities.gpu import GpuIds | ||
| has_gpu_sel = True | ||
| except ModuleNotFoundError: | ||
| has_gpu_sel = False | ||
|
|
||
| import weakref | ||
|
|
||
| class tigre_algo_wrapper(Reconstructor): | ||
|
|
||
| def __init__(self, name=None, initial=None, image_geometry=None, data=None, niter=0, **kwargs): | ||
| """ | ||
|
MargaretDuff marked this conversation as resolved.
Outdated
|
||
| A wrapper for TIGRE algorithms, allowing the use of CIL geometries and data. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| name : str | ||
| Name of the TIGRE algorithm to use (e.g., 'ART', 'SART', 'SIRT', 'OSSART'). | ||
| initial : ImageData, optional | ||
| Initial guess for the reconstruction. If None, a zero-initialized image is used. | ||
| image_geometry : ImageGeometry | ||
| The geometry of the image to be reconstructed. | ||
| data : AcquisitionData | ||
| The measured projection data. | ||
| niter : int, default=0 | ||
| Number of iterations for the reconstruction algorithm. | ||
| **kwargs : dict | ||
| Additional keyword arguments passed to the TIGRE reconstruction algorithm. | ||
|
|
||
| Returns | ||
| ------- | ||
| ImageData | ||
| The reconstructed image. | ||
| quality : float | ||
| Quality measures computed by the algorithm, if applicable. See the tigre algorithm documentation for details. | ||
|
|
||
| Raises | ||
| ------ | ||
| ValueError | ||
| If `image_geometry` or `data` is None. | ||
|
MargaretDuff marked this conversation as resolved.
Outdated
|
||
|
|
||
| Notes | ||
| ----- | ||
| This class is designed to facilitate the use of TIGRE algorithms within the CIL framework, | ||
| allowing for the use of CIL's `ImageGeometry` and `AcquisitionData` classes. It handles the conversion | ||
| of CIL geometries to TIGRE geometries and prepares the data for the specified algorithm. | ||
| The `name` parameter should match one of the available TIGRE algorithms for example: 'art', 'sirt', 'sart', 'ossart', 'cgls', 'lsmr', 'hybrid_lsqr', 'ista', 'fista', 'sart_tv', 'ossart_tv'. | ||
|
|
||
| Note | ||
| ---- | ||
| We are aware that running the TIGRE algorithms: ISTA, FISTA, SART_TV, OSSART_TV using 2D data can lead to incorrect restults in the TV denoising step, particularly when using more than one GPU. https://github.com/CERN/TIGRE/issues/681 | ||
| You can change the gpuids by passing the `gpuids` keyword argument, for example: | ||
| ```python | ||
| from tigre.utilities.gpu import GpuIds | ||
| gpuids = GpuIds() | ||
| gpuids.devices = [0] # Specify the GPU device IDs you want to use | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like this is something we should be exposing elsewhere and have a consistent interface (i.e. take a list of IDs). But in this case it seems like a kwarg passed directly to the algorithm, which suits out 'light touch' approach, are there other kwargs they might pass?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, they can pass a huge number and it differs per algorithm which ones they can pass |
||
| algo = tigre_algo_wrapper(name='fista', initial=initial_image, image_geometry=image_geom, data=acquisition_data, niter=10, gpuids=gpuids) | ||
| ``` | ||
|
|
||
|
|
||
| Example | ||
| ------- | ||
| >>> from cil.plugins.tigre import tigre_algo_wrapping | ||
| >>> algo = tigre_algo_wrapper(name='SART', initial=initial_image, image_geometry=image_geom, data=acquisition_data, niter=10) | ||
| >>> reconstructed_image, quality = algo.run() | ||
|
|
||
| """ | ||
|
|
||
| missing = [] | ||
| if image_geometry is None: | ||
| missing.append("`image_geometry`") | ||
| if data is None: | ||
| missing.append("`data`") | ||
|
|
||
| if missing: | ||
| raise ValueError(f"You must pass {', '.join(missing)}") | ||
|
MargaretDuff marked this conversation as resolved.
Outdated
|
||
|
|
||
| log.info("%s setting up tigre geometry", self.__class__.__name__) | ||
|
|
||
| if initial is None: | ||
| initial = image_geometry.allocate(0) | ||
|
|
||
| self.tigre_initial = initial.copy().as_array() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the case where initial is None, allocating zeros and then copying isn't necessary. I'd also consider carefully if we need a copy at all. Does tigre change the value? Or is this array reused for the solution, in which case we can hijack this behaviour for our
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So in SIRT, SART, OSSART and OSSART_TV they use the init buffer as the working buffer, binding it without a copy here and then accumulating into it in place here. But they don't return it - Other algorithms differ again: LSMR leaves init untouched, and AB_GMRES does something completely different when returning. Therefore Ithink to keep CIL's general plan that we leave init untouched, and to make sure we're safe for all TIGRE algorithms, we need the copy when a user passes a specific init and not None. |
||
| ig = image_geometry | ||
| ag = data.geometry | ||
| self.tigre_geom, self.tigre_angles = CIL2TIGREGeometry.getTIGREGeometry( | ||
| ig, ag) | ||
| self.tigre_geom.check_geo(self.tigre_angles) | ||
|
MargaretDuff marked this conversation as resolved.
Outdated
|
||
| self.tigre_projections = data.as_array() | ||
|
|
||
| if self.tigre_projections.ndim == 2: | ||
| if any( a==name for a in ['ista', 'fista', 'sart_tv', 'ossart_tv']): | ||
| warnings.warn( | ||
| "We are aware that the TIGRE algorithms: ISTA, FISTA, SART_TV, OSSART_TV using 2D data can lead to incorrect results in the TV denoising step, particularly when using more than one GPU.", UserWarning, stacklevel=2) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I like the wording of this warning. If it's in the TV step why are fista and ista affected? Maybe something like Should it be a warning at all, in other cases we block the backend from running for geometries that aren't fully supported.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So At the moment it is set up to warn if you try and do TV with 2D geometry with any number of GPUs. We think it is an issue just with multiple GPUs so maybe we make it more specific and fail instead of just warn. If tigre fix the bug though, we would have to update?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a fix now in Tigre but I haven't checked it against the versions of tigre CIL uses |
||
|
|
||
|
|
||
| if data.dimension_labels[0] != AcquisitionDimension.ANGLE: | ||
| self.tigre_projections = np.expand_dims(self.tigre_projections, axis=0) | ||
|
|
||
| if self.tigre_geom.is2D: | ||
| self.tigre_projections = np.expand_dims(self.tigre_projections, axis=1) | ||
| self.tigre_initial = np.expand_dims(self.tigre_initial, axis=0) | ||
|
MargaretDuff marked this conversation as resolved.
Outdated
|
||
|
|
||
| self.tigre_algo = getattr(algs, name) | ||
| self.niter = niter | ||
| self.kwargs = kwargs | ||
| if has_gpu_sel: | ||
| self.gpuids = self.kwargs.pop('gpuids', None) | ||
| if self.gpuids is None: | ||
| self.gpuids = GpuIds() | ||
| log.info("Using GPU ids:", self.gpuids) | ||
|
MargaretDuff marked this conversation as resolved.
Outdated
|
||
|
|
||
| self._input = None | ||
|
|
||
| super(tigre_algo_wrapper, self).__init__(data, image_geometry=ig, backend='tigre') | ||
|
|
||
| log.info("%s configured", self.__class__.__name__) | ||
|
|
||
| def set_input(self, input): | ||
| """ | ||
| When called by the parent class during initialisation, sets the input data to run the reconstructor on. The geometry of the dataset must be compatible with the reconstructor. | ||
| When called after initialisation, raises NotImplementedError as changing the input is not currently supported. | ||
| Parameters | ||
| ---------- | ||
| input : AcquisitionData | ||
| A dataset with a compatible geometry | ||
| """ | ||
| if self._input is None: | ||
| if input.geometry != self.acquisition_geometry: | ||
| raise ValueError ("Input not compatible with configured reconstructor. Initialise a new reconstructor with this geometry") | ||
| else: | ||
| self._input = weakref.ref(input) | ||
|
|
||
| else: | ||
| raise NotImplementedError("Setting the input after initialisation is not currently supported.") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't understood what this is about, is it that you don't want to inherit the method from the parent class? If it's not something we want in the API maybe we could separate it's use in the parent so we have a private method for checking the input, and remove the interface for this method?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it's to make it comptible for being a "reconstructor" and is called by the parent class at initialisation but I don't want the user to use it (at the moment). Agree, making it private method would be useful |
||
|
|
||
| def run(self, out=None): | ||
| """ | ||
| Run the specified TIGRE algorithm with the provided parameters. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| out : ImageData, optional | ||
| Output image data to store the result. If None, a new ImageData object is created. | ||
|
|
||
| Returns | ||
| ------- | ||
| out : ImageData | ||
| The reconstructed image data. | ||
| quality : float | ||
| Quality measures computed by the tigre algorithm, if applicable. | ||
|
|
||
| """ | ||
|
|
||
| log.info("%s passing to the tigre algorithm", self.__class__.__name__) | ||
| if has_gpu_sel: | ||
| result = self.tigre_algo( | ||
| proj=self.tigre_projections, | ||
| geo=self.tigre_geom, | ||
| angles=self.tigre_angles, | ||
| init=self.tigre_initial, | ||
| niter=self.niter, | ||
| gpuids=self.gpuids, | ||
| **self.kwargs | ||
| ) | ||
| else: | ||
| result = self.tigre_algo( | ||
| proj=self.tigre_projections, | ||
| geo=self.tigre_geom, | ||
| angles=self.tigre_angles, | ||
| init=self.tigre_initial, | ||
| niter=self.niter, | ||
| **self.kwargs | ||
| ) | ||
|
MargaretDuff marked this conversation as resolved.
Outdated
|
||
|
|
||
| img = result[0] if isinstance(result, tuple) else result | ||
|
|
||
| quality = result[1] if len(result) > 1 else None | ||
|
MargaretDuff marked this conversation as resolved.
Outdated
|
||
|
|
||
| if out is None: | ||
| out = self._image_geometry.allocate(0) | ||
|
|
||
| out.fill(img) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If If out is passed and we are only using it to fill the result maybe that's something we should consider carefully. I would be tempted to not take
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think done. Have left it consistent with CIL elsewhere |
||
|
|
||
| log.info("%s completed", self.__class__.__name__) | ||
|
|
||
| return out, quality | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| # Copyright 2025 United Kingdom Research and Innovation | ||
| # Copyright 2025 The University of Manchester | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| # | ||
| # Authors: | ||
| # CIL Developers, listed at: https://github.com/TomographicImaging/CIL/blob/master/NOTICE.txt | ||
| import unittest | ||
| from utils import has_tigre, has_nvidia, initialise_tests | ||
|
|
||
| from cil.utilities.dataexample import ( | ||
| SIMULATED_PARALLEL_BEAM_DATA, | ||
| SIMULATED_CONE_BEAM_DATA, | ||
| SIMULATED_SPHERE_VOLUME | ||
| ) | ||
| from cil.processors import TransmissionAbsorptionConverter, Slicer | ||
| from cil.framework import ImageData | ||
|
|
||
| import numpy as np | ||
| from unittest_parametrize import parametrize | ||
| from unittest_parametrize import ParametrizedTestCase | ||
|
|
||
| initialise_tests() | ||
|
|
||
| import warnings | ||
|
|
||
| from testclass import CCPiTestClass | ||
|
|
||
| if has_tigre: | ||
| from tigre.utilities.gpu import GpuIds | ||
| from cil.plugins.tigre import ProjectionOperator, tigre_algo_wrapper | ||
|
|
||
| class TestTigreReconstructionAlgorithms(ParametrizedTestCase, unittest.TestCase): | ||
|
|
||
|
|
||
| def get_geometry_data(self, geometry_type): | ||
| if geometry_type == "parallel_2d": | ||
| data = SIMULATED_PARALLEL_BEAM_DATA.get().get_slice(vertical='centre') | ||
| gt = SIMULATED_SPHERE_VOLUME.get().get_slice(vertical='centre') | ||
| elif geometry_type == "parallel_3d": | ||
| data = SIMULATED_PARALLEL_BEAM_DATA.get() | ||
| gt = SIMULATED_SPHERE_VOLUME.get() | ||
| elif geometry_type == "cone_2d": | ||
| gt = SIMULATED_SPHERE_VOLUME.get().get_slice(vertical='centre') | ||
| data = SIMULATED_CONE_BEAM_DATA.get().get_slice(vertical='centre') | ||
| elif geometry_type == "cone_3d": | ||
| gt = SIMULATED_SPHERE_VOLUME.get() | ||
| data = SIMULATED_CONE_BEAM_DATA.get() | ||
| else: | ||
| raise ValueError(f"Unknown geometry type: {geometry_type}") | ||
|
|
||
| absorption = TransmissionAbsorptionConverter()(data) | ||
| ig = gt.geometry | ||
| return ig, absorption, gt | ||
|
|
||
|
|
||
|
|
||
|
|
||
| def run_algorithm(self, name, geometry_type, expect_warning=False, **kwargs): | ||
| ig, absorption, gt = self.get_geometry_data(geometry_type) | ||
|
|
||
| if expect_warning: | ||
| with warnings.catch_warnings(record=True) as w: | ||
| warnings.simplefilter("always") | ||
| algo = tigre_algo_wrapper( | ||
| name=name, | ||
| initial=None, | ||
| image_geometry=ig, | ||
| data=absorption, | ||
| niter=2, | ||
| **kwargs | ||
| ) | ||
| img, qual = algo.run() | ||
| warning_msgs = [str(warn.message) for warn in w] | ||
| self.assertTrue( | ||
| any("incorrect results in the TV denoising step" in msg for msg in warning_msgs), | ||
| f"Expected warning not raised for {name} with {geometry_type}" | ||
| ) | ||
| else: | ||
| algo = tigre_algo_wrapper( | ||
| name=name, | ||
| initial=None, | ||
| image_geometry=ig, | ||
| data=absorption, | ||
| niter=2, | ||
| **kwargs | ||
| ) | ||
| img, qual = algo.run() | ||
|
|
||
| self.assertIsInstance(img, ImageData) | ||
| self.assertEqual(img.shape, ig.shape) | ||
| if qual is not None: | ||
| self.assertTrue(isinstance(qual, (float, int, np.ndarray))) | ||
|
|
||
|
|
||
| @parametrize( | ||
| ("name", "kwargs", "expect_warning", "geometry_type"), | ||
| [ | ||
| ("sart", {}, False, "parallel_2d"), | ||
| ("sirt", {}, False, "parallel_3d"), | ||
| ("ossart", {}, False, "cone_2d"), | ||
| ("lsmr", {}, False, "cone_3d"), | ||
| ("cgls", {}, False, "parallel_2d"), | ||
| ('hybrid_lsqr', {}, False, "cone_2d"), | ||
| ("ista", { | ||
| "hyper": lambda self, ig, ag: ProjectionOperator(ig, ag).norm()**2, | ||
| "Quameasopts": ['RMSE'], | ||
| "tvlambda": 0.01 | ||
| }, True, "cone_2d"), | ||
| ("fista", { | ||
| "hyper": lambda self, ig, ag: 2 * ProjectionOperator(ig, ag).norm()**2, | ||
| "Quameasopts": ['RMSE'], | ||
| "tvlambda": 0.001 | ||
| }, True, "cone_2d"), | ||
| ("sart_tv", {"tvlambda": 50}, True, "parallel_2d"), | ||
| ("ossart_tv", {"tvlambda": 0.005}, True, "parallel_2d"), | ||
| ] | ||
| ) | ||
| @unittest.skipUnless(has_tigre, "Requires TIGRE") | ||
| @unittest.skipUnless(has_nvidia, "Requires NVIDIA GPU for TIGRE") | ||
| def test_tigre_algorithms_with_geometries(self, name, kwargs, expect_warning, geometry_type): | ||
| ig, absorption, _ = self.get_geometry_data(geometry_type) | ||
|
|
||
|
|
||
| gpuids = GpuIds() | ||
| if expect_warning: | ||
| gpuids.devices = [0] | ||
| kwargs['gpuids'] = gpuids | ||
|
|
||
|
|
||
| resolved_kwargs = { | ||
| k: v(self, ig, absorption.geometry) if callable(v) else v | ||
| for k, v in kwargs.items() | ||
| } | ||
| self.run_algorithm(name, geometry_type, expect_warning=expect_warning, **resolved_kwargs) | ||
|
|
||
|
|
Uh oh!
There was an error while loading. Please reload this page.