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

IRT: add signal and/or timing cuts #705

Open
c-dilks opened this issue Jun 5, 2023 · 0 comments
Open

IRT: add signal and/or timing cuts #705

c-dilks opened this issue Jun 5, 2023 · 0 comments
Labels
topic: PID Relates to PID reconstruction

Comments

@c-dilks
Copy link
Member

c-dilks commented Jun 5, 2023

Is your feature request related to a problem? Please describe.
Need signal and/or timing cuts in IrtCherenkovParticleID

Describe the solution you'd like
Add to IrtCherenkovParticleIDConfig

veprbl pushed a commit that referenced this issue 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
@kkauder kkauder added the topic: PID Relates to PID reconstruction label Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: PID Relates to PID reconstruction
Projects
Status: Todo
Development

No branches or pull requests

2 participants