refactor: Rework particle selection in Examples #3742
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
This was referenced Oct 16, 2024
github-actions
bot
added
Infrastructure
AJPfleger
reviewed
Oct 16, 2024
kodiakhq bot
pushed a commit
that referenced
this pull request
Oct 17, 2024
Currently Fatras might abort before reaching the pointer where we initialize the particle. This leads to desynchronization between initial and final particles which will then lead to warning/error logs downstream ``` ParticleSele WARNING No final particle found for 4|11|169|0|0 ``` The initial call of the actor is now caught be checking the propagation stage which then initializes the result object gracefully. Discovered in #3742
AJPfleger
approved these changes
Oct 18, 2024
andiwand
added
automerge
and removed
🛑 blocked
|
kodiakhq bot
pushed a commit
that referenced
this pull request
Oct 19, 2024
Adding `TrackFitterPerformanceWriter` to the track finding jobs in addition to `CKFPerformanceWriter`. blocked by - #3742
kodiakhq bot
pushed a commit
that referenced
this pull request
Oct 19, 2024
I usually expect some accumulated plots in the performance writers we can directly look at. At the same time the `CKFPerformanceWriter` is generic enough to write the performance for any finder. Therefore I propose to - Rename `TrackFinderPerformanceWriter` to `TrackFinderNTupleWriter` (this pr) - (and later) Rename `CKFPerformanceWriter` to `TrackFinderPerformanceWriter` blocked by - #3742
kodiakhq bot
pushed a commit
that referenced
this pull request
Oct 19, 2024
Followup of #3737 I usually expect some accumulated plots in the performance writers we can directly look at. At the same time the `CKFPerformanceWriter` is generic enough to write the performance for any finder. Therefore I propose to - Rename `TrackFinderPerformanceWriter` to `TrackFinderNTupleWriter` (#3737) - Rename `CKFPerformanceWriter` to `TrackFinderPerformanceWriter` (this pr) blocked by - #3742 - #3737
kodiakhq bot
pushed a commit
that referenced
this pull request
Nov 2, 2024
After removing `TruthSeedRanges` with #3742 we can also safely remove `TruthSeedSelector` which does very much the same as the `ParticleSelector` by now.
Rosie-Hasan
pushed a commit
to Rosie-Hasan/acts
that referenced
this pull request
Nov 13, 2024
Currently Fatras might abort before reaching the pointer where we initialize the particle. This leads to desynchronization between initial and final particles which will then lead to warning/error logs downstream ``` ParticleSele WARNING No final particle found for 4|11|169|0|0 ``` The initial call of the actor is now caught be checking the propagation stage which then initializes the result object gracefully. Discovered in acts-project#3742
Rosie-Hasan
pushed a commit
to Rosie-Hasan/acts
that referenced
this pull request
Nov 13, 2024
- remove `TruthSeedRanges` as this a duplication of `ParticleSelector ` - enhance `ParticleSelector` by also input/output final particle selections which are necessary to access the number of hits - raise some pT cuts to the usual 1 GeV - handle downstream changes blocked by - acts-project#3744
Rosie-Hasan
pushed a commit
to Rosie-Hasan/acts
that referenced
this pull request
Nov 13, 2024
Adding `TrackFitterPerformanceWriter` to the track finding jobs in addition to `CKFPerformanceWriter`. blocked by - acts-project#3742
Rosie-Hasan
pushed a commit
to Rosie-Hasan/acts
that referenced
this pull request
Nov 13, 2024
…ject#3737) I usually expect some accumulated plots in the performance writers we can directly look at. At the same time the `CKFPerformanceWriter` is generic enough to write the performance for any finder. Therefore I propose to - Rename `TrackFinderPerformanceWriter` to `TrackFinderNTupleWriter` (this pr) - (and later) Rename `CKFPerformanceWriter` to `TrackFinderPerformanceWriter` blocked by - acts-project#3742
Rosie-Hasan
pushed a commit
to Rosie-Hasan/acts
that referenced
this pull request
Nov 13, 2024
Followup of acts-project#3737 I usually expect some accumulated plots in the performance writers we can directly look at. At the same time the `CKFPerformanceWriter` is generic enough to write the performance for any finder. Therefore I propose to - Rename `TrackFinderPerformanceWriter` to `TrackFinderNTupleWriter` (acts-project#3737) - Rename `CKFPerformanceWriter` to `TrackFinderPerformanceWriter` (this pr) blocked by - acts-project#3742 - acts-project#3737
Rosie-Hasan
pushed a commit
to Rosie-Hasan/acts
that referenced
this pull request
Nov 13, 2024
After removing `TruthSeedRanges` with acts-project#3742 we can also safely remove `TruthSeedSelector` which does very much the same as the `ParticleSelector` by now.
Rosie-Hasan
pushed a commit
to Rosie-Hasan/acts
that referenced
this pull request
Nov 20, 2024
After removing `TruthSeedRanges` with acts-project#3742 we can also safely remove `TruthSeedSelector` which does very much the same as the `ParticleSelector` by now.
kodiakhq bot
pushed a commit
that referenced
this pull request
Dec 6, 2024
…in Examples (#3947) This clears out an oversight in a previous PR #3742 where I replaced numbers of measurements with number of hits. This number can be different and should not be confused. I add an optional input for the particle to measurement map which is then queried if present and used for the number of measurement cuts. blocked by: - #3944 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes - **New Features** - Enhanced particle selection with new configuration options for minimum and maximum hit counts. - Updated particle selection criteria to focus on hit counts instead of measurement counts. - **Bug Fixes** - Improved validation checks for input measurements to ensure proper initialization. - **Documentation** - Updated comments and documentation to reflect changes in parameter naming from "measurements" to "hits." - **Tests** - Adjusted test cases to align with the new parameter naming conventions. - **Chores** - Minor code formatting and organization improvements throughout various scripts. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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.
TruthSeedRangesas this a duplication ofParticleSelectorParticleSelectorby also input/output final particle selections which are necessary to access the number of hitsblocked by