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 one-to-one relation with raw tracker/calorimeter hit for reconstructed tracker/calorimeter hit #1535

Merged
merged 18 commits into from
Aug 7, 2024

Conversation

rahmans1
Copy link
Contributor

@rahmans1 rahmans1 commented Jul 26, 2024

Briefly, what does this PR introduce?

Add one-to-one relation with raw tracker/calorimeter hit for reconstructed tracker/calorimeter hit. Contingent on eic/EDM4eic#86.

What kind of change does this PR introduce?

  • Bug fix (issue #__)
  • New feature (issue #__)
  • Documentation update
  • Other: __

Please check if this PR fulfills the following:

  • Tests for the changes have been added
  • Documentation has been added / updated
  • Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

Does this PR change default behavior?

@rahmans1 rahmans1 requested review from wdconinc and bschmookler July 26, 2024 13:23
@github-actions github-actions bot added topic: tracking Relates to tracking reconstruction topic: calorimetry relates to calorimetry topic: digitization labels Jul 26, 2024
@rahmans1 rahmans1 changed the title Add one-to-one relation with raw tracker/calorimeter hit for reconstructed tracker/calorimeter hit Add one-to-one relation with raw tracker hit for reconstructed tracker hit Jul 26, 2024
@rahmans1 rahmans1 changed the title Add one-to-one relation with raw tracker hit for reconstructed tracker hit Add one-to-one relation with raw tracker/calorimeter hit for reconstructed tracker/calorimeter hit Jul 26, 2024
@rahmans1 rahmans1 changed the title Add one-to-one relation with raw tracker/calorimeter hit for reconstructed tracker/calorimeter hit #DRAFT: Add one-to-one relation with raw tracker/calorimeter hit for reconstructed tracker/calorimeter hit Jul 27, 2024
@rahmans1 rahmans1 marked this pull request as draft July 27, 2024 19:36
@rahmans1 rahmans1 changed the title #DRAFT: Add one-to-one relation with raw tracker/calorimeter hit for reconstructed tracker/calorimeter hit Add one-to-one relation with raw tracker/calorimeter hit for reconstructed tracker/calorimeter hit Jul 27, 2024
@wdconinc
Copy link
Contributor

I think there is a desire to move forward on tracking here, where the implementation is essentially complete thanks to one-to-one associations and relations, but I fear that if we add calorimeter hit associations and relations to the scope of this PR it will be slowed down. In particular, the CalorimeterHitDigi does pre-digitization merging of SimCalorimeterHits, so the associations are going to be requiring some non-trivial code.

So, I suggest we keep this PR for tracker (and photomultiplier) hits only, and create a similar PR for calorimeters that may need more non-trivial code.

@wdconinc
Copy link
Contributor

So, I suggest we keep this PR for tracker (and photomultiplier) hits only, and create a similar PR for calorimeters that may need more non-trivial code.

In particular, we want more than just the leading hit in

rawhits->create(
leading_hit.getCellID(),
(adc > m_cfg.capADC ? m_cfg.capADC : adc),
tdc
);

and that then requires that we decide on a weight strategy (hit.getEnergy() / edep probably).

Copy link

sonarcloud bot commented Aug 7, 2024

@veprbl veprbl marked this pull request as ready for review August 7, 2024 15:54
@veprbl veprbl added this pull request to the merge queue Aug 7, 2024
Merged via the queue into main with commit 134f1bf Aug 7, 2024
86 of 87 checks passed
@veprbl veprbl deleted the rahmans1-patch-1 branch August 7, 2024 16:09
github-merge-queue bot pushed a commit that referenced this pull request Aug 14, 2024
…y number of hits (#1564)

### Briefly, what does this PR introduce?
This PR builds on #1535 to create `edm4eic::Track` associations to
`edm4hep::MCParticles`. Results in weights like this:
```
events->Draw("CentralCKFTrackAssociations.weight")
```

![image](https://github.com/user-attachments/assets/a775165c-f644-49f7-8943-322daf4cc2f7)

```
events->Draw("sqrt(MCParticles[_CentralCKFSeededTrackAssociations_sim.index].momentum.x**2+MCParticles[_CentralCKFSeededTrackAssociations_sim.index].momentum.y**2+MCParticles[_CentralCKFSeededTrackAssociations_sim.index].momentum.z**2)")
```

![image](https://github.com/user-attachments/assets/217ba69b-f288-4b6b-b49b-6dbcdaf7ca2f)


### What kind of change does this PR introduce?
- [ ] Bug fix (issue #__)
- [x] New feature (issue: track associations)
- [ ] Documentation update
- [ ] Other: __

### Please check if this PR fulfills the following:
- [ ] Tests for the changes have been added
- [ ] Documentation has been added / updated
- [ ] Changes have been communicated to collaborators

### Does this PR introduce breaking changes? What changes might users
need to make to their code?
No.

### Does this PR change default behavior?
No.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: calorimetry relates to calorimetry topic: digitization topic: tracking Relates to tracking reconstruction
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants