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

fix: TrackSelector remove broken m_noEtaCuts #3640

Merged
merged 8 commits into from
Oct 18, 2024

Conversation

timadye
Copy link
Contributor

@timadye timadye commented Sep 23, 2024

m_noEtaCuts wasn't set correctly, and can be removed. Also check that eta-binned cuts don't themselves cut on eta. This was only tested previously in the case of a single bin.

Also added binIndexNoCheck(eta) which is generally useful and saves having to check eta instead of the index. I think it would be better to have getCuts(eta) not throw but just return the first/last bin, but that would change the API - unless you added a no-throw variant. binIndexNoCheck at least helps in the meantime.

@github-actions github-actions bot added Component - Core Affects the Core module Track Finding labels Sep 23, 2024
@timadye timadye marked this pull request as draft September 23, 2024 18:48
@timadye timadye changed the title draft: fix: TrackSelector remove broken m_noEtaCuts fix: TrackSelector remove broken m_noEtaCuts Sep 23, 2024
Copy link

github-actions bot commented Sep 23, 2024

📊: Physics performance monitoring for 1486e75

Full contents

physmon summary

Copy link
Contributor

@andiwand andiwand left a comment

Choose a reason for hiding this comment

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

some minor comments

Core/include/Acts/TrackFinding/TrackSelector.hpp Outdated Show resolved Hide resolved
@timadye
Copy link
Contributor Author

timadye commented Sep 24, 2024

@paulgessinger, can you also take a look at this PR. I didn't understand the original logic around m_noEtaCuts, which looked wrong. But maybe it was something too clever for me.

@timadye timadye marked this pull request as ready for review October 17, 2024 13:43
@paulgessinger paulgessinger added this to the next milestone Oct 17, 2024
Copy link

sonarcloud bot commented Oct 18, 2024

@kodiakhq kodiakhq bot merged commit 66d1854 into acts-project:main Oct 18, 2024
42 checks passed
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 Track Finding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants