SRT2D repository: SRT 2D PET and SPECT algorithms

[Dimitra-Kyriakopoulou]

Changes in this pull request

2 new analytic reconstruction algorithms SRT 2D PET in the src→ analytic → SRT2D folder and SRT 2D SPECT in the src→ analytic → SRT2DSPECT folder of my SRT2D repository.
[In 2016 in SRT2D folder had been uploaded the prior versions of both these two codes: these old versions do not work with the current STIR version (as there had been pointer etc changes in STIR in between). Also the new versions of the 2 algorithms are improved, e.g. the old SRT2DSPECT run only for uniform data, whereas the new one doesn’t have such restriction.]

Testing performed

• I installed STIR's last version of 7 February 2024 on my pc, and run the codes: they compiled, and they gave correct results. SRT2D run with the Hoffmann phantom, and SRT2DSPECT with XCAT phantom. As SRT mathematically works for the open disc, the slices of XCAT that touched the scanner were slightly squeezed at the edge (by interpolation).
• The codes had no problem with the arc-correction functionality in terms of successful compilation; however, I did not run them with non-arccorrected data.

Related issues

Checklist before requesting a review

• [] I have performed a self-review of my code
• [] I have added docstrings/doxygen in line with the guidance in the developer guide
• [] I have implemented unit tests that cover any new or modified functionality (if applicable)
• [] The code builds and runs on my machine
• [] documentation/release_XXX.md has been updated with any functionality change (if applicable)

[KrisThielemans]

@Dimitra-Kyriakopoulou can you please make multiple commits locally, and then push. Alternatively, add [ci skip] to a commit message. Every new commit triggers a job on Appveyor that takes ~90 mins run-time. I'm killing most of these...

[Dimitra-Kyriakopoulou]

I am really sorry, I forgot [ci skip] in my current change! Please cancel from AppVeyor.

[Dimitra-Kyriakopoulou]

Thank you wholeheartedly for your review!!
I am really sorry with committing continuously; thank you for your help with [ci skip].

• If the changes I applied are not what you asked for, may I please ask you please helped me with further comments?

-Unfortunately, pre-commit-check.yml reverted to its original content, failed again. The problem is the line:
uses: pre-commit/action@v3.0.0
(which allows for use of Node 20 instead of Node 16, and maybe this transition does not occur in my repository. Let me also note I have 18.04.01 Ubuntu, not the latest.)

-Each review point now has a sign 'Outdated'. Am I to click on the 'Resolve Conversation' button? I would not press this button neither for the pre-commit-check.yml nor for the headers (as I am not certain if the new headers are fully OK).

-In the SPECT code, I had forgotten (and now erased) a line which allowed to choose the slices of a sinogram for a multi-slice phantom (as I was running XCAT); apparently this would lead to wrong results if run with arbitrary data.

Thank you wholeheartedly!! I am looking forward to your next comments!

[Dimitra-Kyriakopoulou]

I think what I wrote about the Nodes might not be the problem. It seems to me now it is indeed the Clang-format... I am please asking for your help on fixing it. Thank you so for all your help!!!

[Dimitra-Kyriakopoulou]

The option 'Re-request Review' appeared, and I clicked on it, thinking it was due to the previous review comments being labeled as 'Outdated' after the changes applied. I now think though if the pre-commit-check.yml should have first been corrected; hence, if pressing this button was wrong, please ignore it, and I am really sorry.

[KrisThielemans]

Below is a screen-shot of output of recon_test_pack/run_test_simulate_and_recon.sh with STIR compiled in Debug-mode, STIR_OPENMP=OFF. Top: SRT2D, bottom FBP2D. This looks fine except from an overall scale factor of 2. This will be because of the choice of image units. As the recon_test_pack runs with zoom:=0.5, SRT2DReconstruction needs to divide by zoom at the very end to get same units as the rest of STIR (for PET).

[Dimitra-Kyriakopoulou]

Dear Professor @KrisThielemans,
THANK YOU WHOLEHEARTEDLY FOR ALL YOUR INVALUABLE HELP!!!

I am afraid I do not see the screenshot.

However, I think there is something else also going on leading to the failure of Build and ctest and recon_test_pack CI tests.
As I need to attach some files to explain, I am writing email to send.
THANK YOU WHOLEHEARTEDLY!!!

[KrisThielemans]

Release STIR_OPENMP=ON recons crash (also on GitHub Actions) however. This is a problem with the OpenMP code (I've tested Release, disabling the OpenMP pragma, and then it's fine).

@Dimitra-Kyriakopoulou wrote

I had run openmp in the past, not now. However, I had not run it with the choice dynamic. I changed it to dynamic, because continuous-integration asked me for dynamic allocation of variables, hence I thought it might also be best it is set so.
I wonder whether this causes the openmp problem.

Not sure what you mean with "dynamic". Which lines in the code? I removed schedule(dynamic) nowait but that didn't help