Occasional failures in recon_test_pack SRT2D test on MocOS, clang21 and OpenMP

[KrisThielemans]

A recent clang 21 job on OSX failed with https://github.com/UCL/STIR/actions/runs/18307335644/job/52127574274#step:17:1414

Running SRT2D SRT2D_test_sim.par
Input ROI mean: 1
Output ROI mean: 0.981458
DIFFERENCE IN ROI VALUES IS TOO LARGE. CHECK RECONSTRUCTION LOG my_SRT2D_test_sim.par_force_zero_view_offset.log

A previous job with essentially the same code has an ROI difference of 0.996258

This is how the reconstructed image looks like, which isn't so good.

@Dimitra-Kyriakopoulou do you see similar things? Could be a "clang-compiler with openMP" problem, or something else.

[Dimitra-Kyriakopoulou]

Dear Professor @KrisThielemans,

I) I am deeply puzzled that the result shows a cross type artifact; locally on my pc the results were always homogeneous. Please see attached srt_zoom_2.png.

II) However, I took the opportunity to fix the image grid: in place of a fixed normalized grid ([-1,1]), the grid is now derived from physical voxel sizes (mm) and the scanner’s tangential sampling (mm), then normalized by the detector half-FOV (Rmax). This means that SRT now uses the same mm→normalized mapping as FBP, so zoom/FOV work properly, cropping disappears, and centering matches that of FBP. Please see attached srt_zoom_fixed.png.

However, this did not affect the test results, because SRT’s reconstructed circle was larger —not smaller— than the one used for testing.

III) The occasionality of the test's results could be explained by numerical precision differences in Clang 21 with OpenMP.

However, the deeply troubling recon image with the cross artifact is created prior to the tests.
The algorithm itself is deterministic, however OpenMP can cause even mathematically deterministic loops to produce slightly different numerical results, depending on the order of floating-point summations. I wonder if the “cross” could result from how partial sums are combined across threads.

Hence, for safety, maybe we should consider commenting out the parallelization code.

THANK YOU WHOLEHEARTEDLY FOR ALL YOUR INVALUABLE HELP!!!
Dimitra

[KrisThielemans]

I took the opportunity to fix the image grid

is this ready for a PR?

OpenMP can cause even mathematically deterministic loops to produce slightly different numerical results, depending on the order of floating-point summations. I wonder if the “cross” could result from how partial sums are combined across threads.
Hence, for safety, maybe we should consider commenting out the parallelization code.

It'd be worrying if this is a numerical error. More than likely there is some race-condition. I don't have time to look into this, and neither do you. As I'm about to release STIR 6.3, I therefore agree that it's best to disable OpenMP in SRT2D. Easiest might be to simply put

#ifdef STIR_OPENMP
#undef STIR_OPENMP // disable OpenMP for now due to https://github.com/UCL/STIR/issues/1626
#endif

at the start of SRT2DReconstruction.cxx.

[Dimitra-Kyriakopoulou]

Dear Professor @KrisThielemans,
Yes, the fix for image grid is ready for PR. I also added the disable comment for OpenMP.

The updated SRT2D code is in the GRD2D-DDSR2D repository, which is under Pull Request. I completed the review comments for the Pull Request, and all tests are successful except for the Pre-Commit. Once this is complete, it will be ready for merging into master.
Can you please help me with this test? Thank you wholeheartedly for all your help!!!

I am really sorry for the delay,
Dimitra

[KrisThielemans]

#1595 disabled openmp for SRT2D.