-
Notifications
You must be signed in to change notification settings - Fork 6
feat: add Truthiness datatype for Monte Carlo comparisons #128
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
Conversation
ruse-traveler
left a comment
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.
Interesting! I'm really intrigued by this! I'm curious about how this type would get used in practice, so for my own understanding: the vector members should be 1-to-1 with the relations in the associations field, correct?
It's not intended for analyzers but for reconstruction development (so absolutely not for selecting for your analysis only those events that are close to the truth). But I would imagine we can use this to select events that are particularly poorly reconstructed and look at what went wrong, or compare before and after PRs to make sure we don't make things worse. |
Gotcha! Makes sense! This could be really useful as a high level summary of the impact of a change... |
|
Then following up on the vectors: I partly ask because I wonder if it would make sense to define a Truthiness component just to make the interface a little easier... For example, there's an overall, energy, momentum, and PID truthiness at both the event and association level. So we could define a component sort of like: edm4eic::TruthinessContribution:
Members:
- float overall
- float pid
- float energy
- float momentumSo that: edm4eic::Truthiness:
<... description ...>
Members:
- edm4eic::TruthinessContribution eventContribution
- float unassociatedMCParticleContribution
- float unassociatedRecoParticleContribution
VectorMembers:
- edm4eic::TruthinessContribution associationContributions
<... relations ...>
``` |
ruse-traveler
left a comment
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.
Only comments I have so far are the component-idea for consideration above, and some suggestions for naming consistency between this and other types' fields below!
ruse-traveler
left a comment
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.
🙌 Thanks! I think this looks good!
mdiefent
left a comment
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.
This PR introduces edm4eic::TruthinessContribution, a useful metric for assessing reconstruction algorithms on an event-by-event basis. The new data type was discussed in detail during the ePIC Software & Computing meeting on November 12. Similar to the discussion in this PR, we considered potential improvements and concluded that it is appropriate to integrate the truthiness measure now, with the understanding that we can iterate on the implementation in the future. The implementation is solid, and the comments in the code are clear. The criteria for a data model addition, namely discussion and consensus in an ePIC Software & Computing meeting, have been met.
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Briefly, what does this PR introduce?
This PR adds a datatype to record the "truthiness" (as mathematically defined...) for a reconstructed event; where truthiness is the "quality of seeming or being felt to be true, even if not necessarily true," or in this case also "the amount of confidently proclaiming the wrong thing to be true."
Mathematically, truthiness is a non-negative value that is zero only for perfectly reconstructed events (positive-definite), and is radially increasing in the error of the reconstruction (greater error leads to greater truthiness).
It is possible to define truthiness in multiple ways, but we will typically use some combination of the following components:
There are non-reconstruction reasons why the truthiness will be non-zero in realistic scenarios:
Nevertheless, the decrease of the overall average event truthiness for the same geometry and input hit collections is intended to indicate an improved reconstruction, and converse.
What kind of change does this PR introduce?
Please check if this PR fulfills the following:
Does this PR introduce breaking changes? What changes might users need to make to their code?
No.
Does this PR change default behavior?
No.