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

STIR-python OSMAPOSL priors are GeneralisedPrior objects rather than child class priors #707

Open
robbietuk opened this issue Oct 8, 2020 · 10 comments

Comments

@robbietuk
Copy link
Collaborator

OSMAPOSL reconstruction object is defined and setup in python as self.recon. This setup process has a penalty defined (e.g. QP). The call self.recon.get_objective_function().get_prior() returns
<Swig Object of type 'stir::shared_ptr< stir::GeneralisedPrior< stir::DiscretisedDensity< 3,float > > > *' at 0x125b172f0>. This GeneralisedPrior object does not contain any methods specific to the QP/RDP and therefore evaluations of things like parabolic_surrogate_curvature() (QP), or get_gamma()/set_gamma() (RDP) cannot be performed.

Can get_prior() return the child class prior?, e.g. QP or RDP.

@robbietuk robbietuk changed the title STIR-python priors are GeneralisedPrior objects rather than child class priors STIR-python OSMAPOSL priors are GeneralisedPrior objects rather than child class priors Oct 8, 2020
@KrisThielemans
Copy link
Collaborator

see #662 and #284

feel like trying what's in #284 (comment)?

@robbietuk
Copy link
Collaborator Author

I think I will. Unfortunately I don't think it can be put off for much longer.

@KrisThielemans
Copy link
Collaborator

could be very easy, but we'll only find out by trying. The SWIG list is quite helpful though. Let me know

@robbietuk
Copy link
Collaborator Author

I shall do. Let me learn SWIG first 😄

@KrisThielemans
Copy link
Collaborator

That could easily take up the rest if you PhD!

(edit src/swig/stir.i, add a single %factory line, see what happens...)

@robbietuk
Copy link
Collaborator Author

Is there any way we could do something similar to

STIR/src/swig/stir.i

Lines 1620 to 1626 in d1cd05c

%inline %{
template <class T>
stir::PoissonLogLikelihoodWithLinearModelForMeanAndProjData<T> *
ToPoissonLogLikelihoodWithLinearModelForMeanAndProjData(stir::GeneralisedObjectiveFunction<T> *b) {
return dynamic_cast<stir::PoissonLogLikelihoodWithLinearModelForMeanAndProjData<T>*>(b);
}
%}

The python call would be something like stir.ToQP(self.recon.get_objective_function().get_prior()), which would return class type QP, with all the methods avalible.

I am guessing this would require an individual stir method for each prior (e.g. stir.ToRDP() and stir.ToPLS())and is not ideal for code clarity.

@KrisThielemans
Copy link
Collaborator

this is easy enough. We probably only need it for PriorWithParabolicSurrogate at the moment?

@robbietuk
Copy link
Collaborator Author

Ill give it a go for PriorWithParabolicSurrogate if not, Ill just do each prior individually 😄 I dont think think would be a perminent fix though. If it works, should we PR it to STIR master?

@KrisThielemans
Copy link
Collaborator

for what we need, this should work.

Regarding a PR, depends if we can get the SWIG people to help out first or not!

@robbietuk
Copy link
Collaborator Author

See robbietuk#10

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