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

Allow TOF mashing in SSRB #1464

Merged
merged 18 commits into from
Jul 4, 2024
Merged

Conversation

NicoleJurjew
Copy link
Contributor

Changes in this pull request

Testing performed

Related issues

Checklist before requesting a review

  • I have performed a self-review of my code
  • [] I have added docstrings/doxygen in line with the guidance in the developer guide
  • [] I have implemented unit tests that cover any new or modified functionality (if applicable)
  • The code builds and runs on my machine
  • documentation/release_XXX.md has been updated with any functionality change (if applicable)

@NicoleJurjew
Copy link
Contributor Author

Checks have been performed by mashing uniform 1 sinograms:

image

@KrisThielemans
Copy link
Collaborator

This looks good to me, but weirdly enough it seems like we broke recon_test_pack/run_scatter_tests.sh. Could you run that one on your machine as well?

NicoleJurjew and others added 6 commits June 24, 2024 14:08
This results in shorter code, but also consistent.

There was a "problem" in the previous version that the TOF
bin was actually used as tang_pos_num when determining get_m()
(but it didn't create a problem as currently get_m() in independent of both).
even if do_norm=1, we should not normalise with the number of TOF bins.
This is because the projectors will take the width of the TOF bin
properly into account. (In contrast, for "span", they don't).
add a few arguments and configurable environment variables
to be able to modify the output of the simulation
Using the cache has very little impact on run-time for a single forward projection,
but affects memory a lot. This is especially important for TOF.
@KrisThielemans KrisThielemans added this to the v6.2 milestone Jul 3, 2024
@KrisThielemans
Copy link
Collaborator

I had to undo the normalisation for "num_mashed_tof_bins". The projectors are set-up such that the total number of counts remains the same independent of number of TOF bins/width etc (i.e. the same as non-TOF). Therefore, when mashing TOF bins and setting do_norm=1, we do not need to divide by the the number of contributing TOF bins.

I've now added a test to check all this via reconstruction.

This should be good to go. Let's see how the tests are doing.

When using a single int as argument, the old style version is
actually not used anymore due to the overload with the verbosity presumably.
So use boost::format
span=2, max_rd=2 is unexpected, and leads to a bug in FBP3DRP etc.
Changing to the expected case of max_rd=3 for now.
@KrisThielemans KrisThielemans merged commit 32befa6 into UCL:master Jul 4, 2024
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants