UCL / STIR

Software for Tomographic Image Reconstruction
http://stir.sourceforge.net/
Other
108 stars 90 forks source link

CLI projector utilites with cache use excessive memory for block geometries #1365

Open robbietuk opened 7 months ago

robbietuk commented 7 months ago

Running the CLI command:

back_project bp_ones_cli ones.hs ones.hv

uses the default configuration of BackProjectorByBin (e.g. BackProjectorByBinUsingProjMatrixByBin).

This data being projected is non-TOF and short axial FOV (nothing excessive for standard PET scanners in STIR) using the block geometry. The ProjData file is ~200MB on disk. The geometry does disable symmetries:

WARNING: Disabling all symmetries except for symmtery_z since they are not implemented in block geometry yet.

I find that back projecting half the data uses 20GB+ memory (htop report), which is then killed by my system before completion.


Modifying: https://github.com/UCL/STIR/blob/1f5b06f71eb3fb360dd726bd77bd7f334b735844/src/utilities/back_project.cxx#L95-L96 to

      shared_ptr<ProjMatrixByBin> PM(new ProjMatrixByBinUsingRayTracing());
      PM->enable_cache(false);
      back_projector_sptr.reset(new BackProjectorByBinUsingProjMatrixByBin(PM));

removes this large memory usage. It appears the LOR cache storage is too large for blocks (due to the lack of symmetries?)


Does the back_project (and likewise forward_project) have any use for cache? If not, can it be disabled by default, as above? OR does it have a use when more symmetries are added?

The primary issue is for standard non-TOF scanners (using blocks), this utility is generally unusable in its default configuration.

KrisThielemans commented 6 months ago

yes, this is the same as for calculate_attenuation_coefficients. It might make sense to have a enable_caching(bool value=true) member of ForwardProjectorByBin, BackProjectorByBin and ProjectorPairByBin, which the projectors that using a matrix can then use to set the caching off. This seems cleaner than having to know about the matrix.

KrisThielemans commented 6 months ago

Obviously, you can pass a parameter file now that does this, as mentioned on the list.

robbietuk commented 6 months ago

yes, this is the same as for calculate_attenuation_coefficients

They use the same projector methodology so yes.

It might make sense to have a enable_caching(bool value=true) member of ForwardProjectorByBin, BackProjectorByBin and ProjectorPairByBin, which the projectors that using a matrix can then use to set the caching off.

Would this mean that, for example, parallelproj forward/back projectors would have the option to enable_caching, although it would have no practical functionality?

Obviously, you can pass a parameter file now that does this, as mentioned on the list.

Yes but having an optional parameter file requirement to run the CLI utilities isn't very user friendly.

KrisThielemans commented 6 months ago

Would this mean that, for example, parallelproj forward/back projectors would have the option to enable_caching, although it would have no practical functionality?

yes. If they have no cache, it won't make a difference. Otherwise it seems impossible to do this at the highest level of the hierarchy. Having the "check on I'm using a matrix" in several places in the code is surely inferior. Especially if people want to run this from Python as well.

Obviously, you can pass a parameter file now that does this, as mentioned on the list.

Yes but having an optional parameter file requirement to run the CLI utilities isn't very user friendly.

What do you mean? You prefer to make the parameter file required?

robbietuk commented 6 months ago

Obviously, you can pass a parameter file now that does this, as mentioned on the list.

Yes but having an optional parameter file requirement to run the CLI utilities isn't very user friendly.

What do you mean? You prefer to make the parameter file required?

No, sorry, the parameter file should remain optional. However, the current status means the projector utilities are unusable with standard BlocksOnCylinder geometries without a machine with exceptional memory capacity. That is, unless you provide an optional parameter file that turns the caching off. I agree that disabling the caching by default is a better solution.