Merge master at v5.2.0 to TOF #52
Conversation
…d ring2; save axial effects to file; added interfileRawDataHeaderSiemens; initialise view and tang_pos to 0 in construct_sino_lookup_table().
- created new class InterfileNormHeaderSiemens where all the parsing is done (moving some bits from BinNormalisationFromECAT8, but adding much more) - add more consistency checks after parsing (still not enough) - fix problem that we always assumed span=11 in the norm header as we now parse it - fix creating of internal proj_data_info which assumed max_ring_difference=num_rings-1 as we now parse it
we were using the wrong variable in a loop in construct_sino_lookup_table. Also use number of non-tof sinograms.
fix to accomodate VS2015
unclear use of member variable and argument, probably was a bug that the argument was ignored.
Siemens data is often acquired with max_ring_difference < num_rings-1, and the axial_effects stored are therefore limited. We now extrapolate them to just use 1.
variable was not initialised before also do an explicit run-time error check and clean-up some white-space
added write_components_to_file keyword (only setable via parsing) by default set to false.
We found that we need to divide as opposed to multiply with the axial effect factors
There was a duplicate line in the other order. Removed now
before we were using a single axial effect factor. Now, we will use all of them appropriate for a particular span. Results are the same for span=11. Also - removed the unused "span" member variable - write warning about larger max ring diffs in set_up() to avoid local static variable
- throw as opposed to return Succeeded::no, improving user experience in Python - more informative messages if anything is wrong
this enables more concise code (avoiding the need for comparing with Succeeded::yes which is especially verbose in Python).
Also make Bin.time_frame_num a variable in Python like other members
Minor fixes.improvements (mostly SWIG)
remind me: are you comparing your pr result with the UCL branch? |
|
with NikEfth/tof_sino_UCL, yes |
|
In fact, I see a difference in the non-TOF sensitivity as well, where tof_sino_UCL produces zero-end planes (which seems wrong), while this PR doesn't. Which might say that this PR introduces a terrible bug, but solves another one... Not sure yet! |
|
non-TOF sensitivity on this PR is the same as on All this doesn't explain why the sensitivity of the TOF recon is wrong of course (which is by the way a non-TOF calculation) |
|
The problem disappears when using TOF for the sensitivity calculation ( |
|
The problem is (amazingly) in |
set_up() checks if norm_proj_data_info >= emission_proj_data_info but needs to handle non-TOF norm data and TOF emssion data. Previously this worked as ProjDataInfo::blindly_equal ignored TOF_mashing, but that is no longer the case, so we need to handle this here.
Tell use they can set STIR_CONFIG_DIR env variable
make sure that currect normalisation::set_up is used for either sensitivity, log-likelihood or Hessian calculations Still need to do listmode Partially fixes NikEfth#56
|
351c29d fixed the sensitivity problem #56, although only for ProjData. Code still needs to be ported to listmode (which currently needs duplication sadly). New error is in This is likely a long-standing problem on |
- the script itself was a bit garbled - it failed because the num_tangential_poss in the .hroot was too small compared to the data, such that counts didn't match - the hroot and template.hs were inconsistent (max num non-arccorrected) I've also made the template TOF.
…n tang_poss Also small clean-up by re-using constructor (C++11 feature)
this is now required
insert break such that the script doesn't continue with tests that will fail
had to fix some files accordingly
- updated README (was still referring to test_root_view_offset) - needed a small Python fix. Also changed label strategy to be clearer. -
KrisThielemans
force-pushed
the
merge_master_at_v5.2.0_to_TOF
branch
from
4b362c3 to
3846284
Compare
KrisThielemans
force-pushed
the
merge_master_at_v5.2.0_to_TOF
branch
from
d193c6a to
01075f6
Compare
|
After merging Reason for this is that the test_PET_GATE.root file contains some weird events with too large time difference, but how this is handled is a bit complicated. (see also #58) After the previous merge, the "internal" On the other hand, if the |
|
Fixing #58 is out-of-scope for this PR. Instead, I will modify the template and .hroot to use TOF mashing factor 1, but with 82 times wider TOF bins (which is equivalent to what was used before). With that, the test succeeds. |
Motivation is twofold: - it doesn't make a lot of sense to have TOF mashing keyword in a ROOT file - run_root_GATE.sh failed due to events that have very large time diff in the test ROOT file. ( see NikEfth#52 (comment) for details)
|
I am postponing the sensitivity sync to listmode for later... |
This merge uses
SegmentIndices,ViewgramIndices,SinogramIndices(as in v5.2.0), but addingtiming_pos_numtoSegmentIndices. Functionality should be identical to previous status..Note that
DataSymmetries*have not been rewritten yet and still ignoretiming_pos_numinViewSegmentNumbers(as it wasn't in there).Fixed a number of small problems along the way, including the fact that
ProjMatrixByBin::set_up()didn't initialise the internal memberimage_info_sptrwhile the TOF calculation used it. (ProjMatrixByBinUsingRayTracing::set_updid the initialisation, but that meant that other classes derived fromProjMatrixByBinwere not TOF-ready).Edit before merge: This PR fixed a few other things and had to work around some problems, see comments (and commit messages) below. It also sets the version to 6.0.0.