UCL
Virtual xtal interface.
Interface for the virtual xtals.
Update CListModeDataROOT.cxx, Scanner.h, and 4 more files...
The definition of the scanner overall is very circular and confusing and probably we should re-write from scratch.
However, I took the lead from the mCT example were the number of the detectors per ring includes the virtual crystals (13+1)*48. Respectively the num_rings should include the virtual rings.
The changes, I made allow us to design a scanner if a 2-fold way, 1 gap per block (in mCT every 13 crystals) or 1 gap every several blocks. The later is in better agreement with most scanners I would know, where the block separation between blocks is small of e.g. half crystal, but there is 1 or 2 crystal between the rotated (bigger) blocks (r-sectors in GATE).
For example, my scanner has 18 sectors with 4 transaxial blocks with 8 transaxial crystals.
Now both number of detectors per ring 18*(4*8+1) and 18*4*(8+1) can pass the consistency test.
And soon the CListModeDataROOT will be able to know were to push the gap.
I think that bein able to reposition the gaps is quite important.
I would like to add that the modification does not interfere with the singles units, which can still be independent.
The definition of the scanner overall is very circular and confusing and probably we should re-write from scratch
yeah. at some point... You cleaned at least the set_params stuff up in #304. Let's not attempt a major rewrite until #304 and #577 are merged.
The changes, I made allow us to design a scanner if a 2-fold way, 1 gap per block (in mCT every 13 crystals) or 1 gap every several blocks. The later is in better agreement with most scanners I would know, where the block separation between blocks is small of e.g. half crystal, but there is 1 or 2 crystal between the rotated (bigger) blocks (r-sectors in GATE).
I certainly agree that many scanners would have bigger gaps between "buckets" than "blocks". However, I hadn't realised that you wanted to fix that with this PR. The name of the PR and previous conversation led me to think you only wanted to be able to set the number of virtual crystals (which is what most of this PR is about).
In fact, I cannot see how you could fix this without having additional members num_axial_virtual_crystals_per_bucket etc (or proably better num_additional_axial_virtual_crystals_per_bucket as it'd add up to the one between blocks).
To me adding that facility seems to need a fair amount of work, but maybe I'm wrong.
Well, I am not really fixing it, for that we would need a num_virtual_crystal_per_bucket() function, but just letting the consistency test pass and the make the CListModeROOT do the right thing.
The key logic is that based on the number of detector per ring you set, we can figure out how many virtual crystals there are. Is the divisor of block number or block*crystal.
I understand that some tests succeed but there might be further implications down the line.
hmm, after all, I introduced the gaps in buckets.
~~My main question is whether you prefer the num_of_detectors_per_ring in the template to be the physical detectors only, or both gaps and detectors~~
~~I think that the second is easier. Otherwise, we have to go everywhere and replace the get_numXX() functions with get_num_logicalXX()~~
~~While if the total number in the template includes the virtual crystals we have to introduce functions get_num_phisicalXX() in fewer cases~~
Actually, I think that in the template we should set the number of physical detectors and the then get functions to do the calculations. However, this means that we cannot have set functions. There have to become explicitly set_physical, or very very very well documented. What do you think?
int
Scanner::get_num_rings() const
{ return get_num_physical_rings() + (get_num_virtual_axial_crystals_per_bucket() - 1) * get_num_axial_buckets() +
(get_num_virtual_axial_crystals_per_block()-1) * get_num_axial_blocks();}
int
Scanner::get_num_detectors_per_ring() const
{
return num_detectors_per_ring + (get_num_virtual_transaxial_crystals_per_bucket() - 1) * get_num_transaxial_buckets() +
(get_num_virtual_transaxial_crystals_per_block()-1) * get_num_transaxial_blocks();}
int
Scanner::get_num_physical_rings() const
{ return num_rings;
}
int
Scanner::get_num_physical_detectors_per_ring() const
{
return num_detectors_per_ring;
}
I think that in the template we should set the number of physical detectors and the then get functions to do the calculations. However, this means that we cannot have set functions. There have to become explicitly set_physical, or very very very well documented. What do you think?
While reading/setting things in terms of physical detectors would make a lot of sense, changing the meaning of get_num_rings() to suddenly be get_num_physical_rings() would break quite a lot of stuff. Given my pace of accepting PRs, I'd strongly recommend not attempting that... Of course, making it clearer in the documentation what is what would help.
Of course, having members get_num_physical_rings(), get_num_non_physical_rings() (with the sum get_num_rings() ) is an excellent idea. Providing an alias for get_num_rings() that makes it clear what it means is of course fine as well. We could then consider deprecating the old names.
@NikEfth I cannot recall where we were with this, but aside from the comments above, it probably needs a revisit now that #833 has been merged.