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

Remove radiusOfInnermostHit from tracks #326

Merged
merged 2 commits into from
Jul 8, 2024
Merged

Conversation

jmcarcell
Copy link
Member

@jmcarcell jmcarcell commented Jun 20, 2024

@jmcarcell jmcarcell linked an issue Jun 20, 2024 that may be closed by this pull request
jmcarcell added a commit to key4hep/k4FWCore that referenced this pull request Jun 20, 2024
jmcarcell added a commit to key4hep/k4SimDelphes that referenced this pull request Jun 20, 2024
jmcarcell added a commit to key4hep/k4EDM4hep2LcioConv that referenced this pull request Jun 20, 2024
and compute it from hits when going from EDM4hep to LCIO. See
key4hep/EDM4hep#326
@jmcarcell jmcarcell changed the title Remove radiusOfInnermostHit Remove radiusOfInnermostHit from tracks Jun 21, 2024
@tmadlener
Copy link
Contributor

The fact that this requires changes in k4SimDelphes makes me think this will also show up in some analyses in FCCAnalyses, but I haven't checked.

@jmcarcell
Copy link
Member Author

I checked and didn't find anything in FCCAnalyses

@tmadlener
Copy link
Contributor

OK. Then maybe it was there only temporarily, or we just filled it in k4SimDelphes because we could. Thanks for checking.

@hegner hegner self-requested a review June 24, 2024 07:05
@hegner
Copy link
Contributor

hegner commented Jun 24, 2024

Perfect. Good we checked this again :)

@hegner
Copy link
Contributor

hegner commented Jun 24, 2024

can we maybe finally fix the release build test environment? Otherwise I am fine with merging

@tmadlener
Copy link
Contributor

Fixing the release build would require a new Key4hep release. For EDM4hep we could in principle pick up the latest podio tag, but for other packages, we would also need a new EDM4hep tag. I am not sure if we want to make such a tag at the moment.

tmadlener pushed a commit to key4hep/k4SimDelphes that referenced this pull request Jun 26, 2024
see key4hep/EDM4hep#326

Co-authored-by: jmcarcell <jmcarcell@users.noreply.github.com>
tmadlener pushed a commit to key4hep/k4EDM4hep2LcioConv that referenced this pull request Jun 28, 2024
and compute it from hits when going from EDM4hep to LCIO. See
key4hep/EDM4hep#326
@tmadlener tmadlener force-pushed the remove-radius-innermosthit branch from 0b08295 to b452236 Compare July 1, 2024 16:13
tmadlener pushed a commit to key4hep/k4EDM4hep2LcioConv that referenced this pull request Jul 2, 2024
and compute it from hits when going from EDM4hep to LCIO. See
key4hep/EDM4hep#326
tmadlener added a commit to key4hep/k4EDM4hep2LcioConv that referenced this pull request Jul 3, 2024
* Don't use radiusOfInnermostHit for EDM4hep tracks

and compute it from hits when going from EDM4hep to LCIO. See
key4hep/EDM4hep#326

* Introdue utility function to determine radius of innermost hit

* Add documentation about radius calculation

---------

Co-authored-by: jmcarcell <jmcarcell@users.noreply.github.com>
Co-authored-by: tmadlener <thomas.madlener@desy.de>
@tmadlener tmadlener force-pushed the remove-radius-innermosthit branch 3 times, most recently from 773d126 to b6f2317 Compare July 8, 2024 13:54
@tmadlener tmadlener force-pushed the remove-radius-innermosthit branch from b6f2317 to 096740e Compare July 8, 2024 14:58
@tmadlener tmadlener merged commit 3d271d0 into main Jul 8, 2024
14 of 17 checks passed
@tmadlener tmadlener deleted the remove-radius-innermosthit branch July 8, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

radiusOfInnerMostHit could be removed from Track
3 participants