Skip to content

Conversation

MargaretDuff
Copy link
Member

@MargaretDuff MargaretDuff commented May 14, 2025

Changes

  • Wrapped all tigre algrorithms so they can be called as a Reconstructor for example:
from cil.plugins.tigre import tigre_algo_wrapper
algo = tigre_algo_wrapper(name='sart', initial=None, image_geometry=ig, data=absorption, niter=10)
img, qual = algo.run()

This has been tested on all geometries for most of the currently available tigre algorithms, see here https://github.com/MargaretDuff/cil_test_notebooks/blob/master/os-sart-tigre/basic_tigre_wrapping.ipynb and the unit tests.

There is a bug with those Tigre algorithms that enforce TV regularisation using the "im3ddeoniser" in tigre, if you have a 2d geometry and if you are running on multiple gpus. A warning will show for users and setting the gpuids in the kwargs works ( see the test notebook). A bug report has been put into tigre CERN/TIGRE#681

Discarded options:

Alternative discarded solution

### Option 1 - wrapped tigre's OS SART, SIRT and SART as CIL algorithms so they could be called e.g. ```python from cil.plugins.tigre import ART, SIRT, SART, OSSART OSSART_algorithm_wrap = OSSART(initial=None, image_geometry=ig, data=b, blocksize=10) OSSART_algorithm_wrap.run(5) solution= OSSART_algorithm_wrap.solution ``` To do: - [x] Test on 2D geometries and cone beam geometries - [ ] Test the callback functionality and algorithm stopping - [ ] Decide if we want to explicitly expose more options from Tigre e.g. OrderStrategy for OS SART. Any unused kwargs will be passed. - [x] Investigate memory-efficient ways of calculating the objective - [ ] Decide what the user needs to provide for the algorithm to run. At minimum, data (with acquisition geometry) and image geometry. Image geometry could be extracted from an initial, if that is passed. If both image geometry and initial is passed, do we check their geometries are the same? - [ ] Look at ways we can save/reuse memory - [ ] Write unit tests - [x] Write doc strings - including explaining that this is just for CT forward problems.

See here: https://github.com/MargaretDuff/cil_test_notebooks/blob/master/os-sart-tigre/cil_data_tigre_sart_sirt.ipynb

Related issues/links

Closes #2159

Checklist

  • I have performed a self-review of my code
  • I have added docstrings in line with the guidance in the developer guide
  • I have updated the relevant documentation
  • I have implemented unit tests that cover any new or modified functionality
  • CHANGELOG.md has been updated with any functionality change
  • Request review from all relevant developers
  • Change pull request label to 'Waiting for review'

Contribution Notes

Please read and adhere to the developer guide and local patterns and conventions.

  • The content of this Pull Request (the Contribution) is intentionally submitted for inclusion in CIL (the Work) under the terms and conditions of the Apache-2.0 License
  • I confirm that the contribution does not violate any intellectual property rights of third parties

@MargaretDuff MargaretDuff marked this pull request as draft May 14, 2025 16:18
@MargaretDuff
Copy link
Member Author

Discussed with developers today:

  • We would like to try a general wrapping of tigre algorithms e.g. tigre.algo( name, initial, image_geometry, data, niter, **kwargs) which then converts the geometry, data and initial to tigre compatible objects and passes to tigre as name(initial, data, tigre_geom, tigre_angles, niter, **kwargs). Currently all of the tigre algorithms take this form. We would provide this basic wrapping and pass to tigre documentation for information and exploration
  • There is an open question about what we then pass back to CIL. We could return just the result ( which we could pass back to a CIL object) or return the result and the tigre algorithm or something else.
  • Unit tests would just check the CIL parts of this - we would make no claims on the accuracy of the algorithms.

@MargaretDuff
Copy link
Member Author

Discussed with developers:

  • We prefer option 2, it works with all current tigre algorithms and it is clearer that it is Tigre's algorithm and the credit goes to them
  • We want to wrap it as a processor with an init, set_input and get_output. get_output could take a number of iterations and still be decided where the initial is set
  • We should not restrict which tigre algorithms can be passed to this, just maybe have a list of tested algorithms in the docstring.
  • We should link to the tigre documentation in the docstring

@MargaretDuff MargaretDuff changed the title Draft PR for discussion - wrapping OS SART, SIRT and SART from tigre as a CIL algorithm Tigre algorithm wrapping Jul 29, 2025
@MargaretDuff
Copy link
Member Author

MargaretDuff commented Jul 29, 2025

@gfardell - I was having another think about this - we discussed writing the wrapper as a processor but to force the code to have an init, set_input and get_output seems challenging e.g. in the init we would pass: image geometry, method, iteration number and maybe initial, set_input would pass the data and would actually initialise the algorithm and then process would do everything. To me it feels a bit backward and you wouldn't get any computational benefit from this set up

I was wondering if we could write it as a reconstructor like in the recon class with just an init and run method? It also already has the machinery to check the image and acquisition geometries.

I think it might make sense if the processors go from the same type of data e.g. image to image and acquisition to acquisition but the reconstructors go from acquisition to image

@MargaretDuff MargaretDuff self-assigned this Aug 20, 2025
@MargaretDuff MargaretDuff marked this pull request as ready for review August 20, 2025 16:23
@MargaretDuff MargaretDuff requested a review from gfardell August 20, 2025 16:24
@github-project-automation github-project-automation bot moved this to Todo in CIL work Aug 20, 2025
@MargaretDuff MargaretDuff moved this from Todo to In Progress in CIL work Aug 20, 2025
@MargaretDuff MargaretDuff moved this from In Progress to Blocked in CIL work Aug 20, 2025
@gfardell gfardell added this to the v25.1.0 milestone Sep 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Blocked

Development

Successfully merging this pull request may close these issues.

OS-SART algorithm

3 participants