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

add DetectionEfficiencies (aka "normalisation") #35

Merged

Conversation

KrisThielemans
Copy link
Contributor

No description provided.

@KrisThielemans KrisThielemans added the enhancement New feature or request label Oct 27, 2024
@KrisThielemans KrisThielemans self-assigned this Oct 27, 2024
@KrisThielemans
Copy link
Contributor Author

KrisThielemans commented Oct 27, 2024

We have a terminology problem. In ScannerGeometry we define detecting_elements as a list of ReplicatedBoxSolidVolumes (where each ReplicatedBoxSolidVolume could have a different material for instance).

DetectorModule: !record
fields:
detectingElements: ReplicatedBoxSolidVolume*
# list of unique ids for every replicated solid volume
# constraint: size(detectingElements) == size(detectingElementsIds)
detectingElementIds: uint*

However, in this PR, we currently use detecting_element as a single "crystal" (or "solid volume"), e.g.
# Detection efficiencies for every detecting element and energy window
# If size along energyBinIdx is 1, the effiencies are assumed to be the same for every energy bin.
# Constraint: size(DetElEfficiencies, "energyBinIdx") == scannerInformation.numberOfEnergyBins or 1
DetElEfficiencies: float[detElIdx, energyBinIdx]

Note that we use detector_id in CoincidenceEvent, implying that a "detector" is a single "crystal"
CoincidenceEvent: !record
fields:
# the pair of detector elements
detectorIds: uint*2

I think that ScannerGeometry.detecting_elements is actually a pretty confusing name for what it is. Anyone any suggestions?

I also think that detector_id isn't great, as for many, a whole module could easily be the "detector". The name detecting_element (or det_el) seems to be more appropriate for the lowest level concept.

@KrisThielemans
Copy link
Contributor Author

Also note that currently I'm not using the "detector ids" independent from the number. See #28 (comment)

@KrisThielemans KrisThielemans marked this pull request as draft October 27, 2024 18:06
@KrisThielemans
Copy link
Contributor Author

I've now added (in Python) the facility to add multiple rings of blocks (while still using only rotations for symmetries), and updated the plotting to use boxes.
image

@KrisThielemans
Copy link
Contributor Author

We have a terminology problem. In ScannerGeometry we define detecting_elements as a list of ReplicatedBoxSolidVolumes (where each ReplicatedBoxSolidVolume could have a different material for instance).
...
However, in this PR, we currently use detecting_element as a single "crystal" (or "solid volume")

On second thought, I now think there isn't really a problem. ScannerGeometry.detectingElements essentially is a list of a (replicated) list of what we call in the PR detEls. Aside from the "double list", the concept is fine. So, I'll leave this as-is for now.

I also think that detector_id isn't great, as for many, a whole module could easily be the "detector". The name detecting_element (or det_el) seems to be more appropriate for the lowest level concept.

I do think that this is a problem, but it should be addressed in a different PR.

@KrisThielemans KrisThielemans marked this pull request as ready for review November 1, 2024 14:36
@KrisThielemans KrisThielemans merged commit 8f0531d into ETSInitiative:main Nov 1, 2024
1 check passed
@KrisThielemans KrisThielemans deleted the DetectionEfficiencies branch November 1, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant