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

perf: std::sqrt over std::hypot in Core #3694

Merged
merged 12 commits into from
Oct 10, 2024

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented Oct 5, 2024

Replace std::hypot with std::sqrt in a couple of occasions in Core. This is numerically less accurate but much faster which is preferable for seeding and propagation.

blocked by


This pull request focuses on optimizing mathematical calculations by replacing std::hypot with more efficient alternatives like std::sqrt and Eigen's norm method. These changes aim to improve performance and maintain code consistency across various files.

Mathematical Optimization:

Additional changes include similar optimizations in other files to ensure consistency and performance improvements across the codebase.

@andiwand andiwand added this to the next milestone Oct 5, 2024
Copy link

github-actions bot commented Oct 5, 2024

📊: Physics performance monitoring for 48367a2

Full contents

physmon summary

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.

Sounds good. Did you measure this by any chance?

@andiwand
Copy link
Contributor Author

andiwand commented Oct 5, 2024

seems to give a 3% speedup on the seeding
image

AJPfleger
AJPfleger previously approved these changes Oct 8, 2024
@github-actions github-actions bot added Infrastructure Changes to build tools, continous integration, ... Changes Performance labels Oct 9, 2024
@andiwand andiwand marked this pull request as ready for review October 9, 2024 06:49
@andiwand andiwand added the 🛑 blocked This item is blocked by another item label Oct 9, 2024
@andiwand andiwand removed the 🛑 blocked This item is blocked by another item label Oct 9, 2024
@acts-project-service acts-project-service added the Fails Athena tests This PR causes a failure in the Athena tests label Oct 9, 2024
@asalzburger
Copy link
Contributor

I think that should go in.

@andiwand
Copy link
Contributor Author

Just kicked off one more Athena CI run but the failure above seems unrelated

@andiwand andiwand removed the Fails Athena tests This PR causes a failure in the Athena tests label Oct 10, 2024
@acts-project-service acts-project-service added the Fails Athena tests This PR causes a failure in the Athena tests label Oct 10, 2024
@andiwand andiwand added automerge and removed Fails Athena tests This PR causes a failure in the Athena tests labels Oct 10, 2024
Copy link

sonarcloud bot commented Oct 10, 2024

@kodiakhq kodiakhq bot merged commit 0fa82af into acts-project:main Oct 10, 2024
42 checks passed
@acts-project-service acts-project-service added the Breaks Athena build This PR breaks the Athena build label Oct 10, 2024
@andiwand andiwand deleted the perf-sqrt-over-hypot branch October 10, 2024 15:56
@acts-project-service acts-project-service added the Fails Athena tests This PR causes a failure in the Athena tests label Oct 11, 2024
@paulgessinger paulgessinger modified the milestones: next, v37.1.0 Oct 16, 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 Changes Performance Component - Core Affects the Core module Component - Examples Affects the Examples module Event Data Model Fails Athena tests This PR causes a failure in the Athena tests Infrastructure Changes to build tools, continous integration, ... Seeding Track Finding Vertexing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants