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

cSTIR_PLSPriorGradient has confusing name #1248

Closed
KrisThielemans opened this issue May 1, 2024 · 3 comments
Closed

cSTIR_PLSPriorGradient has confusing name #1248

KrisThielemans opened this issue May 1, 2024 · 3 comments
Assignees

Comments

@KrisThielemans
Copy link
Member

SIRF/src/xSTIR/cSTIR/cstir.cpp

Lines 1262 to 1263 in 2c66faf

PLSPrior<float>& prior = objectFromHandle<PLSPrior<float> >(ptr_p);
auto sptr_im = prior.get_anatomical_grad_sptr(dir);

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

evgueni-ovtchinnikov commented May 2, 2024

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 KrisThielemans removed the bug label May 2, 2024
@KrisThielemans
Copy link
Member Author

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

@KrisThielemans KrisThielemans changed the title Python PLSPrior gradient is incorrect cSTIR_PLSPriorGradient has confusing name May 2, 2024
@evgueni-ovtchinnikov
Copy link
Contributor

taken care of by recently merged PR #1246

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

No branches or pull requests

2 participants