STIR icon indicating copy to clipboard operation
STIR copied to clipboard

ML norm code should work by bucket, not by block

Open KrisThielemans opened this issue 3 years ago • 4 comments

As found by @nikefth, @francescaleek and @emikhaylova, the current assumption behind symmetries in find_ML_normfactors3D that the scanner is invariant over rotations by block is often incorrect in practice. Most scanners will have a few blocks in a "bucket", and the rotational (and for some scanners even axial translation) symmetry should be applied on the bucket level.

If the symmetries are wrong, @francescaleek found that the patterns in the estimated geometric factors will incorrect (mostly underestimated).

KrisThielemans avatar Mar 02 '21 18:03 KrisThielemans

Some comments:

  • ML norm code currently uses num_crystals_per_block, it should use num_crystals_per_bucket instead
  • The handling of virtual crystals in a block/bucket configuration might take some thinking about.
    • Both in trans-axial and axial directions

robbietuk avatar Mar 12 '21 14:03 robbietuk

I have given this a shot and while I thought it would be nice and easy, there are ~180 uses of foobar_per_block mentions that simply need to be replaced with foobar_per_bucket. Therefore, I will wait for #833 to be merged before I fix this problem otherwise the conflicts will be significant and confusing

robbietuk avatar Mar 15 '21 13:03 robbietuk

#940 solves this without the renaming. That still needs to be done.

KrisThielemans avatar Nov 29 '21 23:11 KrisThielemans

re-opened such that we don't forget to rename all the internal functions

KrisThielemans avatar Nov 29 '21 23:11 KrisThielemans