Skip to content
New issue

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

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

Already on GitHub? # to your account

[SWIG] split stir.i and change order of declarations #1019

Merged
merged 3 commits into from
Apr 19, 2022

Conversation

KrisThielemans
Copy link
Collaborator

The GeneralisedObjectiveFunction hierarchy was defined before normalisation and projectors, leading to problems setting/getting corresponding members.

As this was very messy, I've split stir.i in several files. It still leads to one huge *wrap.cxx but at least reading stir.i is now a bit easier. (still work to do).

This commit also fixes a line in SWIG for ProjData::get_subset, but that function is still ignored, so it doesn't change the actual Python interface.

Fixes #1018

The GeneralisedObjectiveFunction hierarchy was defined before
normalisation and projectors, leading to problems setting/getting
corresponding members.

As this was very messy, I've split stir.i in several files.
It still leads to one huge *wrap.cxx but at least reading stir.i
is now a bit easier. (still work to do).

This commit also fixes a line in SWIG for ProjData::get_subset,
but that function is still ignored, so it doesn't change the actual
Python interface.
@KrisThielemans
Copy link
Collaborator Author

@markus-jehl can you please check this? Also, because of the reordering, the FBP problem tha tyou had might be resolved, so you could try to enable the "instantiations" (now in stir_reconstructors.i)

@markus-jehl
Copy link
Contributor

Sure, I'll have a look if this still works with my reconstruction pipeline and if that solves the FBP problem! Will probably be next week, though.

Copy link
Collaborator

@robbietuk robbietuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the simplification of stir.i. I was able to compile this PR and I now have access to stir.BinNormalisationFromProjData(), which addresses #1018

@markus-jehl
Copy link
Contributor

I just checked it and it all works fine for me as well. Defining the FBP templates no longer crashes everything, so that's a plus in my view :-) I still can't define the FBP templates in a way that exposes their member functions, but this is not just about working out how this needs to be done in SWIG.

@KrisThielemans
Copy link
Collaborator Author

thanks. can you create a new issue for the FBP problem (maybe there's one already..)

@KrisThielemans KrisThielemans added this to the v5.0.1 milestone Apr 19, 2022
@KrisThielemans KrisThielemans merged commit d607b3e into UCL:master Apr 19, 2022
@KrisThielemans KrisThielemans deleted the SWIG_reorder branch April 19, 2022 08:38
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BinNormalisation is not correctly configured for python
3 participants