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

TOF segment bug fix, extra members and update visualisation to add TOF #1319

Merged
merged 4 commits into from
Jan 11, 2024

Conversation

KrisThielemans
Copy link
Collaborator

  • fixes ProjData::get_segment_by_*(const SegmentIndices&)
  • adds some more members to Segment classes that use *Indices
  • update @robbietuk 's visualisation tool to add a TOF slider etc

@KrisThielemans KrisThielemans added this to the v6.0 milestone Jan 11, 2024
@KrisThielemans
Copy link
Collaborator Author

@NicoleJurjew this might come in handy for showing your TOF data

@robbietuk
Copy link
Collaborator

Am I right in thinking the summation over TOF bins collapses into a non-TOF view/sinogram? If so, we should add a checkbox to do this.

@KrisThielemans
Copy link
Collaborator Author

We could do, but it's another feature. There are many other desirable features here, including having 2 projection datasets, displaying over tof dimension etc etc.

If you're happy with the current implementation, I'll merge this

@robbietuk
Copy link
Collaborator

I assume this is tested? Currently the demo uses examples/recon_demo/smalllong.hs, which is not TOF data.

Otherwise, merge.

@robbietuk
Copy link
Collaborator

robbietuk commented Jan 11, 2024

There is a bug, segment_number argument is not used.

def get_limits(self, dimension: ProjDataDims, segment_number: int) -> tuple:
"""
Returns the limits of the projection data in the indicated dimension.
:param dimension: The dimension to get the limits for, type SinogramDimensions.
:param segment_number: The segment number to get the limits for. Only _truly_ required for axial position.
:return: A tuple containing the minimum and maximum value of the dimension (min, max).
"""
if self.projdata is None:
return 0, 0
if dimension == ProjDataDims.SEGMENT_NUM:
return self.projdata.get_min_segment_num(), \
self.projdata.get_max_segment_num()
elif dimension == ProjDataDims.AXIAL_POS:
return self.projdata.get_min_axial_pos_num(self.get_current_segment_num()), \
self.projdata.get_max_axial_pos_num(self.get_current_segment_num())

return self.projdata.get_min_axial_pos_num(self.get_current_segment_num()), \ 
    self.projdata.get_max_axial_pos_num(self.get_current_segment_num()) 

Should read

return self.projdata.get_min_axial_pos_num(segment_num), \ 
    self.projdata.get_max_axial_pos_num(segment_num) 

Edit: I can't edit your branch or make a suggested change.

@KrisThielemans
Copy link
Collaborator Author

ok, but that was an existing bug then, so should have created problems in non-TOF as well. Can you check?

I assume this is tested?

yes, on some local TOF data. It looked fine, but I probably didn't try to switch segments

@robbietuk
Copy link
Collaborator

ok, but that was an existing bug then, so should have created problems in non-TOF as well. Can you check?

Yes. It is actually a latent bug so no need to fix here. c0c4c5b drew my attention to it.

yes, on some local TOF data. It looked fine, but I probably didn't try to switch segments

I think you can still merge this and I'll add an issue and PR some fixes.

@KrisThielemans KrisThielemans changed the title TOF segment bug fix, extra members and update visualation to add TOF TOF segment bug fix, extra members and update visualisation to add TOF Jan 11, 2024
@KrisThielemans KrisThielemans merged commit 151c457 into UCL:master Jan 11, 2024
6 of 7 checks passed
@KrisThielemans KrisThielemans deleted the TOF_segment_and_vis branch January 11, 2024 18:12
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

Successfully merging this pull request may close these issues.

2 participants