HPImaging / sv-mbirct

High Performance Model Based Image Reconstruction for Computed Tomography
BSD 3-Clause "New" or "Revised" License
6 stars 7 forks source link

Hotfix to Bugzilla #17

Closed dyang37 closed 1 year ago

dyang37 commented 1 year ago

This is a hot fix to the bugzilla problem in svmbir, as described here: https://github.com/cabouman/svmbir/issues/268

What we observed is that Bugzilla occurs for num_slices=5, for which max_threads=26. However, the problem did not occur for num_slices=4, for which max_threads=13.

We think this is because we took the ceiling instead of floor function when calculating SV_per_Z=num_slices/SVDEPTH.

A solution is proposed in the link above. I copied it here:

Replace lines 290-293 of recon3d.c with the following:

max_of_num_slices_svdepth = ((num_slices < SVDEPTH) ? num_slices : SVDEPTH)
i = (((Nx < Ny) ? Nx : Ny) / (2*SVLength+1)) * (max_of_num_slices_svdepth/SVDEPTH);
max_threads = ( i < max_threads) ? i : max_threads ;

Previously, the problem is that

SV_per_Z = ceil( num_slices/SVDEPTH ) So that we could allow overlap between SVs when the number of slices was not an integer multiple of the SV depth.

After the fix, the convergence behavior is back to normal for num_slices=3,4,5,6,7,8.

XiaoWang-Github commented 1 year ago

See my comment here https://github.com/cabouman/svmbir/issues/268

sjkisner commented 1 year ago
int max_of_num_slices_svdepth = ((Nz < SVDEPTH) ? Nz : SVDEPTH);
i = (((Nx < Ny) ? Nx : Ny) / (2*SVLength+1)) * (max_of_num_slices_svdepth/SVDEPTH);
max_threads = ( i < max_threads) ? i : max_threads ;

This change is odd. If the number of slices is less than SVDEPTH, for example if Nz=1, then max_threads will be set to 0.

I'm going to close this PR and just limit the threads the same as without the positivity constraint.