UCL / STIR

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

Poisson w/ gated proj data, but no motion? #579

Open rijobro opened 4 years ago

rijobro commented 4 years ago

We now have our projectors set up such that they can use a data processor pair prior to forward projection, and following back projection. This therefore makes classes, such as PoissonLogLikelihoodWithLinearModelForMeanAndGatedProjDataWithMotion redundant. PoissonLogLikelihoodWithLinearModelForMeanAndGatedProjData would be good, which can optionally have motion added in via the data processors.

This would clear up confusion regarding the experimental and non-experimental implementations.

KrisThielemans commented 4 years ago

interesting, and good point! Once you take that view, there seems to be nothing really related to gating in that class...

There's one thing that makes PoissonLogLikelihoodWithLinearModelForMeanAndGatedProjDataWithMotion special, i.e. that it goes from multiple projdata to a single image. (This is similar to the kinetic case of course (multiple time frames->parametric image), hence the similarities in the classes.). I'm saying that because conceivably someone might want to use PoissonLogLikelihoodWithLinearModelForMeanAndGatedProjData with TargetT a GateDiscretisedDensity or so (i.e. reconstruct one image for each gate). This could (later on) be made more sophisticated by using "temporal" filtering or penalties.

I suppose we could have PoissonLogLikelihoodWithLinearModelForMeanAndMultiProjData, this would need a set of DataProcessors that convert TargetT to "normal images" (one for each "multi"), and does the loops. In the future, that'd then need an extension of DataProcessor to have in and OutTargetT to cope with the kinetic case, but we wouldn't need that now.

Might be best to template it in the MultiProjData type. I could even see this being templated in the `PoissonLogLikelihoodWithLinearModelForMeanAndProjData template, but clearly that's for the future.

To avoid having the user to specify motion twice, we could make a new LinearDataProcessor class (derived from DataProcessor), which has an adjoint. That would need some thought on how that would look like.

rijobro commented 4 years ago

Definitely, although I'll have to opt for doing the smallest changes possible whilst getting something to work.

Looking further into the poisson loglikelihoods that have already been implemented, it seems that the experimental version of GatedWithMotion never gets built. So I wanted to use Harry's version that is part of the main code. Is it still regularly used? Just trying to figure out if it's a good starting place or not.

I noticed this at the end. Is the division by the number of gates correct? I thought we said it should be the sum of all images, which made me suspicious of the rest of the code. Happy to be wrong, though.

https://github.com/UCL/STIR/blob/b79bb323c1f1abab02f397f04ea8a13995b133c2/src/include/stir/recon_buildblock/PoissonLogLikelihoodWithLinearModelForMeanAndGatedProjDataWithMotion.txx#L544

KrisThielemans commented 4 years ago

I'm slightly confused about this. I thought you were using the experimental version in all your head MC stuff? I hate to think what we'd need to fix in Harry's code. That division doesn't look ok.

I'll have to opt for doing the smallest changes possible whilst getting something to work.

agreed. Minimum amount of work on the STIR side would be to do nothing aside from building the experimental version I guess! Next step up would be to create PoissonLogLikelihoodWithLinearModelForMeanAndMultiProjData. Next step is figuring an easy way to avoid having to specify the motion stuff twice.

rijobro commented 4 years ago

Unfortunately, previous motion functionality I'd added in STIR was for kinetic modelling. I was just looking at using that as the starting point. Guess I would have to switch TargetT to be DiscDensity instead of ParametricDiscDensity. Would have to rip out all motion and kinetic modelling. Hopefully it'll be as easy as deleting a lot of the code, but that seems unlikely.

KrisThielemans commented 4 years ago

Did you try to build the experimental stuff? Should just work... (famous last words)

rijobro commented 4 years ago

Which experimental stuff? PoissonLogLikelihoodWithLinearModelForMeanAndGatedProjDataWithMotion? The experimental version isn't present in any CMakeLists.txt or in the recon_buildblock_registries. So I'm not sure how to tell STIR how to build it (nor when it was last compiled).

KrisThielemans commented 4 years ago

ah. sorry. My work-around (by renaming) was on a branch on our (private) UCL-STIR fork. Now at https://github.com/KrisThielemans/STIR-1/tree/GEMotionCorr

KrisThielemans commented 4 years ago

now in #580