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

refactor!: Do not use geometry extent during seeding #3688

Merged
merged 8 commits into from
Oct 4, 2024

Conversation

CarloVarni
Copy link
Collaborator

@CarloVarni CarloVarni commented Oct 3, 2024

In the seeding we only need the radius range of the space points (and only when the range is computed by the code and not provided by the user). As of now, when filling the grid we waste a lot of time to fill the Acts::Extent object (for all the possible Acts::BinningValue even if we don't need them all).

I have refactored the code so that we do not do this expensive operation. The user will have to do it by them selves or provide a validity search window via JO.

No changes expected from this PR

@CarloVarni CarloVarni added this to the v37.0.0 milestone Oct 3, 2024
@github-actions github-actions bot added Component - Core Affects the Core module Component - Examples Affects the Examples module Seeding Track Finding labels Oct 3, 2024
Copy link

github-actions bot commented Oct 3, 2024

📊: Physics performance monitoring for 8b944f4

Full contents

physmon summary

@CarloVarni
Copy link
Collaborator Author

don't mind sonarclound. The issues would require extensive changes. I'd like to decouple that fro this PR @paulgessinger

@CarloVarni CarloVarni requested a review from AJPfleger October 4, 2024 09:00
paulgessinger
paulgessinger previously approved these changes Oct 4, 2024
Copy link
Member

@paulgessinger paulgessinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

AJPfleger
AJPfleger previously approved these changes Oct 4, 2024
@CarloVarni
Copy link
Collaborator Author

I had to switch to float for the computation (also downstream expects floats)

@paulgessinger paulgessinger merged commit 81e129b into acts-project:main Oct 4, 2024
40 of 41 checks passed
@github-actions github-actions bot removed the automerge label Oct 4, 2024
@CarloVarni CarloVarni deleted the NoExtent branch October 4, 2024 13:04
Copy link

sonarqubecloud bot commented Oct 4, 2024

@acts-project-service acts-project-service added Breaks Athena build This PR breaks the Athena build Fails Athena tests This PR causes a failure in the Athena tests labels Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaks Athena build This PR breaks the Athena build Component - Core Affects the Core module Component - Examples Affects the Examples module Fails Athena tests This PR causes a failure in the Athena tests Seeding Track Finding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants