doc(dRICH): PID algorithms and collections #708
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7 tasks
wdconinc
approved these changes
Jun 6, 2023
30 tasks
veprbl
pushed a commit
that referenced
this pull request
Jun 29, 2023
Initial connection of [`irt`](https://github.com/eic/irt) to `EICrecon` algorithms, via new algorithm `IrtCherenkovParticleID` and corresponding factory `IrtCherenkovParticleID_factory`. #### Notes - not production ready, therefore not enabled by default in `JEventProcessorPODIO`, but we have been using this for routine current dRICH performance studies - difficult to unit test `IrtCherenkovParticleID`, because of the complexity of its inputs; prefer to defer testing to [DRICH `reconstruction_benchmarks`](https://eicweb.phy.anl.gov/EIC/benchmarks/reconstruction_benchmarks/-/merge_requests/293) - `valgrind` leak checks: #709 (see below for comments) - documentation: #708 #### Caveats - there are several known issues, which should be resolved in separate PRs - #689 - #690 - #691 - #692 - #693 - #705 - `ROOT` dependence is needed, since `irt` heavily relies on objects such as `TVector3` - converters from `EDM4{hep,eic}` vectors to `ROOT` vectors are included in common header `Tools.h` (other downstream PID algorithms will use this header too) - where possible, smart pointers are used in `IrtCherenkovParticleID`, but sometimes we had to unfortunately fall back to raw pointers, for compatibility with the `irt` repository, since: - `irt` does not use any smart pointers - `irt` classes typically include custom destructors; using smart pointers here in `EICrecon` runs the risk of 'double free' violations - any usage of the (banned) operator `new` includes a comment indicating where the allocated object will be destroyed (most likely within an `irt` class destructor) - `irt` objects can be streamed to ROOT files, and closing those ROOT files may `delete` the corresponding object(s); using smart pointers in this scenario can also cause 'double free' violations - we can try to fix this issue in separate PRs, but given the general fragility of streaming objects to ROOT files, I'm afraid this will not be straightforward - not surprisingly, #709 identified a few leaks in the `irt` repository itself - [deferring fixing them until `irt` v2.0](eic/irt#36), where we expect a [large refactor](eic/irt#34) - fixed a few leaks from local geometry service `richgeo`; again unfortunately `irt` does not cooperate with smart pointers
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Briefly, what does this PR introduce?
Documentation for the dRICH PID. This is a bit ahead of
main, but is up-to-date for our development branchirt-algo; therefore, part of it represents plans formain.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