ML_estimate_component_based_normalisation and apply_norm_factors3D cannot be used without saving to disk

[robbietuk]

These are executable files for computing normalization and a brief description of how they are implmented:

• find_ML_normfactors3D: calls ML_estimate_component_based_normalisation and the single function there. Additionally, the documentation in ML_estimate_component_based_normalisation.h is lacking.
• find_ML_normfactors: implements much of the functionality in the main function.
• apply_normfactors3D: implements things in the main function, at least is very short.
• apply_normfactors: does the same as apply_normfactors3D (code duplication?)

Most of the following pertains to find_ML_normfactors3D and find_ML_normfactors. The above functionality saves the computed data to be saved to disk, rather than returning/utilizing a shared_ptr.

---

The core functionality of these executables should be split into various components.

1. The main function should handle only the arguments handling and saving data.
2. Mostly, functions/classes that take the core arguments and compute/process the data.
3. Can handle saving of the data, if specified by the arguments.
4. Function/ object returns a pointer to the computed data, that can be saved by the main exe.

---

To implement this:

• The exe's need to maintain backwards compatibility.
• Replace the ML_estimate_component_based_normalisation function with a class/function..
• Allow member variables/argument to determine if data is saved to disk.
• Store/return computed data as members that can be saved/handled by the exe main functions.

[robbietuk]

Upon further review of apply_normfactors3D, the utility does in fact use the BinNormalisationPETFromComponents class. However, there is currently no setter for the crystal_efficiencies, geometric_factors and block_factorsas the utility uses a streaming method to get the data

STIR/src/utilities/apply_normfactors3D.cxx

Lines 91 to 97 in c51067d

	std::ifstream in(in_filename);
	in >> norm.geometric_factors();
	if (!in)
	{
	warning("Error reading %s, using all 1s instead\n", in_filename);
	do_geo = false;
	}

[KrisThielemans]

Yes, the intention was to let all this work via BinNormalisationPETFromComponents. The current file IO is rather ugly and inconvenient, but we'll need to have it for backwards compatibility for a while. In principle, ML_estimate... could directly return a BinNormalisationPETFromComponents object. That needs some careful thought however.

[robbietuk]

Yes, the intention was to let all this work via BinNormalisationPETFromComponents.

Do you mean for BinNormalisationPETFromComponents to have functionality to perform ML_estimate... and compute the efficiencies etc?

In principle, ML_estimate... could directly return a BinNormalisationPETFromComponents object. That needs some careful thought however.

It could be as simple as a method on the ML_estimate... class having a method that constructs a BinNormalisationPETFromComponents. I think I am confused as to why architecture is such that the find_ML_normfactors3D and apply_normfactors3D utilities are split.

[KrisThielemans]

Yes, the intention was to let all this work via BinNormalisationPETFromComponents.

Do you mean for BinNormalisationPETFromComponents to have functionality to perform ML_estimate... and compute the efficiencies etc?

no, I don't think that's a good idea.

In principle, ML_estimate... could directly return a BinNormalisationPETFromComponents object. That needs some careful thought however.

It could be as simple as a method on the ML_estimate... class having a method that constructs a BinNormalisationPETFromComponents.

yes

I think I am confused as to why architecture is such that the find_ML_normfactors3D and apply_normfactors3D utilities are split.

well, Reconstruction is a different hierarchy than DiscretisedDensity :-). Why would we put "estimation" and "usage" in one class?