Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
robbietuk opened this issue Feb 2, 2024 · 5 comments
Labels

Comments

@robbietuk
Copy link
Collaborator

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:

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

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.

@robbietuk robbietuk changed the title CLI projector utilites with cache use excessive memory for blocks CLI projector utilites with cache use excessive memory for block geometries Feb 2, 2024
@KrisThielemans
Copy link
Collaborator

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
Copy link
Collaborator

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

@robbietuk
Copy link
Collaborator Author

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
Copy link
Collaborator

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
Copy link
Collaborator Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants