making normalisation and random functionality templated to support B…

[danieldeidda]

…lockOnCylindrical

[KrisThielemans]

sorry it doesn't let me use the botton "add suggestion to batch"

[KrisThielemans]

I see a warning on AppVeyor

C:\projects\stir\src\buildblock\multiply_crystal_factors.cxx(95): warning C4067: unexpected tokens following preprocessor directive - expected a newline

This is a mistake of mine I guess https://github.com/UCL/STIR/blob/b480282099055080a3efd11f4fe098f4803ed241/src/buildblock/multiply_crystal_factors.cxx#L95 can you replace and with &&?

[KrisThielemans]

sorry it doesn't let me use the botton "add suggestion to batch"

also not from the Files tab?

[danieldeidda]

I'd take the opportunity to remove get_fan_info from ML_norm.h and make it a static function. (It's just a convenience function here. shouldn't be used by anybody else).

get_fan_info is called by find_ML_singles... though

[danieldeidda]

also if get_fan_info goes back to original we will trigger this:

if (proj_data_info_ptr == 0)
        {
          error("Can only process not arc-corrected data\n");
        }

[KrisThielemans]

I'd take the opportunity to remove get_fan_info from ML_norm.h and make it a static function. (It's just a convenience function here. shouldn't be used by anybody else).

get_fan_info is called by find_ML_singles... though

ok true. that should be revamped a bit, but I'm not up for that now. However, that doesn't use its return value, so making it a void will still work.

also if get_fan_info goes back to original we will trigger this:

if (proj_data_info_ptr == 0)
        {
          error("Can only process not arc-corrected data\n");
        }

Those conditions should be removed there, and in the other functions, I'd replace

    if (proj_data.get_proj_data_info_sptr()->get_scanner_ptr()->get_scanner_geometry()=="Cylindrical")
    {
        auto proj_data_info_ptr =
                dynamic_cast<const ProjDataInfoCylindricalNoArcCorr * const>(&proj_data_info);

        make_fan_data_remove_gaps_help(fan_data, num_rings, num_detectors_per_ring, max_delta, fan_size, *proj_data_info_ptr, proj_data);
    }
    else
    {
        auto proj_data_info_ptr =
                dynamic_cast<const ProjDataInfoBlocksOnCylindricalNoArcCorr * const>(&proj_data_info);

        make_fan_data_remove_gaps_help(fan_data, num_rings, num_detectors_per_ring, max_delta, fan_size, *proj_data_info_ptr, proj_data);
    }

with

    if (auto proj_data_info_ptr =
                dynamic_cast<const ProjDataInfoCylindricalNoArcCorr * const>(proj_data.get_proj_data_info_sptr()))
   {
        make_fan_data_remove_gaps_help(fan_data, num_rings, num_detectors_per_ring, max_delta, fan_size, *proj_data_info_ptr, proj_data);
    }
    else if (auto proj_data_info_ptr =
                dynamic_cast<const ProjDataInfoBlocksOnCylindricalNoArcCorr * const>(proj_data.get_proj_data_info_sptr()))

        make_fan_data_remove_gaps_help(fan_data, num_rings, num_detectors_per_ring, max_delta, fan_size, *proj_data_info_ptr, proj_data);
    }
   else
  {
     error("ML_norm functions only support non-arccorrected data (cylindrical or blocks-on-cylindrical) at the moment.");
  }

or something like that.

get_fan_info will need to do a dynamic_cast to ProjDataInfoCylindrical at the moment to get at get_max_ring_difference, but that should be it.

...maybe....

[danieldeidda]

those condition were already removed from get_fan_info but the error is triggered when doing the bit the was originally there:

proj_data_info_ptr =
                dynamic_cast<const ProjDataInfoCylindricalNoArcCorr * const>(&proj_data_info);

        if (proj_data_info_ptr == 0)
        {
          error("Can only process not arc-corrected data\n");
        }

are you saying we can remove this check?

[KrisThielemans]

yes, we need to check on ProjDataInfoCylnidrical, not more

[danieldeidda]

Add a test case to test_ML_norm.cxx. should be trivial

you mean the same as test_proj_data_info(shared_ptr<const ProjDataInfoCylindricalNoArcCorr> proj_data_info_sptr)

but for blocksOnCylindrical?

[KrisThielemans]

yes.

[KrisThielemans]

This is now fine. I've cleaned-up the history a bit, and undid white-space changes in apply_normfactors.cxx. I'm afraid I force pushed to your branch. sorry. That was a bad idea!

[KrisThielemans]

I'll merge soon after it got through the GHA tests (won't wait for Appveyor)

[KrisThielemans]

@emikhay @markus-jehl we believe this should allow randoms and norm without having to switch to cylindrical. Let us know if not.

[markus-jehl]

This works, thank you!