refactor: physmon: *almost* same settings for GX2F and KF #3248
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
github-actions
bot
added
the
Infrastructure
📊: Physics performance monitoring for 87f83b1physmon summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3248 +/- ##
=======================================
Coverage 47.65% 47.65%
=======================================
Files 509 509
Lines 29423 29423
Branches 14132 14132
=======================================
Hits 14023 14023
Misses 5285 5285
Partials 10115 10115 ☔ View full report in Codecov by Sentry. |
This was referenced Jun 5, 2024
AJPfleger
removed
🚧 WIP
AJPfleger
changed the title
andiwand
approved these changes
Jun 7, 2024
acts-project-service
added
the
Fails Athena tests
kodiakhq bot
pushed a commit
that referenced
this pull request
Jun 8, 2024
Use a temporary track container for the propagation during the updates, to ensure the original track container stays untouched. Until now, we were clearing the original container with `trackContainer.clear()`. This is fine, until we have more tracks in our container. blocked by: - #3248 edit: After #3248 the physmon does not change, since we switched from 2 particles/event to 1. Therefore, we also have only one track/container. Before we could see the effect stronger, like in the graphic. However, I don't see a reason to change physmon to catch this. 
kodiakhq bot
pushed a commit
that referenced
this pull request
Jun 11, 2024
This PR allows to fully support measurements with non-diagonal covariance matrices. ## How? Loop over track states after the propagation extract the needed information for the update. Before we used the `collector()` inside the actor for this task. The collector was too confining, since we had to convert all n-dimensional measurements into n 1-dimensional measurements, removing all non-diagonal information from their covariances. Blocked by: - #3247 - #3248
kodiakhq bot
pushed a commit
that referenced
this pull request
Jun 11, 2024
This was missed in: - #3248
paulgessinger
pushed a commit
to paulgessinger/acts
that referenced
this pull request
Jun 14, 2024
This PR allows to fully support measurements with non-diagonal covariance matrices. ## How? Loop over track states after the propagation extract the needed information for the update. Before we used the `collector()` inside the actor for this task. The collector was too confining, since we had to convert all n-dimensional measurements into n 1-dimensional measurements, removing all non-diagonal information from their covariances. Blocked by: - acts-project#3247 - acts-project#3248
paulgessinger
pushed a commit
to paulgessinger/acts
that referenced
this pull request
Jun 14, 2024
This was missed in: - acts-project#3248
Matthewharri
pushed a commit
to Matthewharri/acts
that referenced
this pull request
Jun 18, 2024
…ct#3248) Have *almost* same settings for GX2F and KF makes it easier to understand in the physmon output and check if our results are good. ## Why *almost*? Had also 3 FPEs which fixed here, but did not choose this approach: - acts-project#3254 This issue only occurred for `2.5 < |eta| < 3.0`. To get better physmon until the problem is fixed (maybe in a later stage of the GX2F-development), the eta-range is reduced. The full eta-range could be used after solving: - acts-project#3267
Matthewharri
pushed a commit
to Matthewharri/acts
that referenced
this pull request
Jun 18, 2024
Use a temporary track container for the propagation during the updates, to ensure the original track container stays untouched. Until now, we were clearing the original container with `trackContainer.clear()`. This is fine, until we have more tracks in our container. blocked by: - acts-project#3248 edit: After acts-project#3248 the physmon does not change, since we switched from 2 particles/event to 1. Therefore, we also have only one track/container. Before we could see the effect stronger, like in the graphic. However, I don't see a reason to change physmon to catch this. 
Matthewharri
pushed a commit
to Matthewharri/acts
that referenced
this pull request
Jun 18, 2024
This PR allows to fully support measurements with non-diagonal covariance matrices. ## How? Loop over track states after the propagation extract the needed information for the update. Before we used the `collector()` inside the actor for this task. The collector was too confining, since we had to convert all n-dimensional measurements into n 1-dimensional measurements, removing all non-diagonal information from their covariances. Blocked by: - acts-project#3247 - acts-project#3248
Matthewharri
pushed a commit
to Matthewharri/acts
that referenced
this pull request
Jun 18, 2024
This was missed in: - acts-project#3248
timadye
pushed a commit
to andiwand/acts
that referenced
this pull request
Jun 27, 2024
This PR allows to fully support measurements with non-diagonal covariance matrices. ## How? Loop over track states after the propagation extract the needed information for the update. Before we used the `collector()` inside the actor for this task. The collector was too confining, since we had to convert all n-dimensional measurements into n 1-dimensional measurements, removing all non-diagonal information from their covariances. Blocked by: - acts-project#3247 - acts-project#3248
timadye
pushed a commit
to andiwand/acts
that referenced
this pull request
Jun 27, 2024
This was missed in: - acts-project#3248
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.
Have almost same settings for GX2F and KF makes it easier to understand in the physmon output and check if our results are good.
Why almost?
Had also 3 FPEs which fixed here, but did not choose this approach:
This issue only occurred for
2.5 < |eta| < 3.0. To get better physmon until the problem is fixed (maybe in a later stage of the GX2F-development), the eta-range is reduced. The full eta-range could be used after solving:pathCorrection=infon cylinderSurface #3267