search

SyneRBI / SIRF

Main repository for the CCP SynerBI software
http://www.ccpsynerbi.ac.uk
Other
57 stars 29 forks source link

cSTIR_PLSPriorGradient has confusing name #1248

Closed KrisThielemans closed 1 month ago

KrisThielemans commented 2 months ago

https://github.com/SyneRBI/SIRF/blob/2c66faff3bc0f12c864cfc2a2931eba5ade60ba0/src/xSTIR/cSTIR/cstir.cpp#L1262-L1263 computes the "image gradient" of the anatomical image, not the gradient of the prior w.r.t. the argument.

The python implementation should just call cSTIR_priorGradient, like for all other priors.

evgueni-ovtchinnikov commented 2 months ago

PLSPrior inherits method get_gradient (and its alias gradient) from Prior.

cSTIR_PLSPriorGradient is called from PLSPrior.get_anatomical_grad (and essentially wraps stir::PLSPrior.get_anatomical_grad_sptr()) - should I perhaps just rename the former to cSTIR_PLSPriorAnatomicalGrad to avoid confusion?

KrisThielemans commented 2 months ago

ah. great. yes, I think a rename will help. Therefore not urgent.

evgueni-ovtchinnikov commented 1 month ago

taken care of by recently merged PR #1246