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

feat: Validate track parameters #3756

Merged
merged 20 commits into from
Oct 29, 2024

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented Oct 17, 2024

Adds two free functions which allow checking BoundVectors and FreeVectorss for valid track parameters. This is then used as asserts in the track parameter EDM.

blocked by

@andiwand andiwand added this to the next milestone Oct 17, 2024
@github-actions github-actions bot added Component - Core Affects the Core module Event Data Model labels Oct 17, 2024
@andiwand
Copy link
Contributor Author

0a02484 contains your feedback @paulgessinger

@github-actions github-actions bot added the Component - Plugins Affects one or more Plugins label Oct 19, 2024
@andiwand
Copy link
Contributor Author

This fires basically everywhere. It would take quite some time to fix every single problem. I think we can just add the functionality and not assert on it for now. The question is if we really want to assert on it. $\phi $ being out of range usually does not hurt. With $\theta$ it gets trickier.

Copy link

github-actions bot commented Oct 21, 2024

📊: Physics performance monitoring for 371692d

Full contents

physmon summary

@andiwand andiwand marked this pull request as ready for review October 22, 2024 09:16
@andiwand andiwand marked this pull request as draft October 23, 2024 08:38
@andiwand
Copy link
Contributor Author

This is a bit frustrating: we have a bunch of invalid track parameters in the code which are generated randomly in different ways. I think it would be good to have one mechanism of generating random track parameters which has to be parameterized before we can continue with this PR.

@andiwand andiwand added the 🛑 blocked This item is blocked by another item label Oct 23, 2024
kodiakhq bot pushed a commit that referenced this pull request Oct 24, 2024
- Reworks random bound and free track parameter generation
  - This should guarantee to obtain valid parameters #3756
- Split general random parameter and covariance generation
- Also, don't use covariance to draw parameters in general random generation
@andiwand andiwand removed the 🛑 blocked This item is blocked by another item label Oct 24, 2024
@andiwand
Copy link
Contributor Author

Since the angle helper changes are quite a stretch I pulled the changes here @timadye #3788

@andiwand andiwand marked this pull request as ready for review October 29, 2024 07:43
Copy link

@kodiakhq kodiakhq bot merged commit 9c0bc47 into acts-project:main Oct 29, 2024
42 checks passed
@acts-project-service acts-project-service added the Fails Athena tests This PR causes a failure in the Athena tests label Oct 29, 2024
@andiwand andiwand deleted the feat-validate-track-params branch October 29, 2024 14:04
@paulgessinger paulgessinger modified the milestones: next, v37.4.0, v37.3.0 Nov 8, 2024
Rosie-Hasan pushed a commit to Rosie-Hasan/acts that referenced this pull request Nov 13, 2024
…ject#3777)

- Reworks random bound and free track parameter generation
  - This should guarantee to obtain valid parameters acts-project#3756
- Split general random parameter and covariance generation
- Also, don't use covariance to draw parameters in general random generation
Rosie-Hasan pushed a commit to Rosie-Hasan/acts that referenced this pull request Nov 13, 2024
Adds two free functions which allow checking `BoundVector`s and `FreeVectors`s for valid track parameters. This is then used as `assert`s in the track parameter EDM.

blocked by
- acts-project#3777
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Core Affects the Core module Component - Plugins Affects one or more Plugins Event Data Model Fails Athena tests This PR causes a failure in the Athena tests Vertexing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants