-
Notifications
You must be signed in to change notification settings - Fork 37
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
WIP: Add Reconstructed TOF. #299
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick summary from the discussion that happened at the EDM4hep meeting (see the attached notes).
- Do we need the 5 different path lengths and expected times for the 5 particle species?
- Different track length / time for different species
- Could create new tracks for the different species and link to that
- Refitting (and storing) the track might be prohibitive due to size (and information duplication)
- No clear best solution to this yet. Needs some further discussion
At this point in time it's not yet clear whether refitting and storing tracks for each particle species and then having a simpler RecTof
is "better" than the current design, where there is the one canonical track that can then be linked to the current (more complicated) RecTof
.
Some minor questions / comments also below.
- float time [ns] // time measurement | ||
- std::array<float, 5> timeExp [ns] // expected time for e(0),mu(1),pi(2),K(3),p(4) | ||
- float sigma // time resolution | ||
- std::array<float, 5> pathLength [mm] // length of flight for e(0),mu(1),pi(2),K(3),p(4) | ||
- edm4hep::Vector3d position [mm] // extrapolated hit position |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would potentially re-order these to keep the particle species bits and the "measured" bits together, e.g.
time
sigma
position
sigma
still needs a unit, which, I would assume is ps
?
Is the extrapolated hit position necessary here? I assume you need it for the calculation of the track length, but do you still need it afterwards?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also propose sigma
-> timeError
, which is similar to other places where we have value and valueError
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about renaming sigma
to something like resolution
Sorry didn't see Andre's comment, timeError
seems nice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In principle we also have the edm4hep::Quantity
that bundles both:
Lines 221 to 225 in a296139
edm4hep::Quantity: | |
Members: | |
- int16_t type // flag identifying how to interpret the quantity | |
- float value // value of the quantity | |
- float error // error on the value of the quantity |
However, that also brings along another 16 free bits. Not sure if we want to go there.
- float time [ns] // time measurement | ||
- std::array<float, 5> timeExp [ns] // expected time for e(0),mu(1),pi(2),K(3),p(4) | ||
- float sigma // time resolution | ||
- std::array<float, 5> pathLength [mm] // length of flight for e(0),mu(1),pi(2),K(3),p(4) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add one "unknown/background/other" entry?
BEGINRELEASENOTES
ENDRELEASENOTES