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: Use Core CKF extrapolation after inwards extension in Examples #3195

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented May 16, 2024

After changes in the Core CKF we are now able to use the extrapolated parameters from within the CKF and don't need to redo the work.

Pulled out of #3188 to capture the reference change and the performance

blocked by

@andiwand andiwand added this to the next milestone May 16, 2024
@github-actions github-actions bot added Component - Core Affects the Core module Component - Examples Affects the Examples module Event Data Model Track Finding labels May 16, 2024
@andiwand andiwand added the 🛑 blocked This item is blocked by another item label May 16, 2024
@andiwand andiwand force-pushed the refactor-examples-track-finding-use-ckf-extrapolation branch from 7078f22 to d29ae01 Compare July 23, 2024 08:23
@github-actions github-actions bot removed Component - Core Affects the Core module Event Data Model labels Jul 23, 2024
@andiwand andiwand marked this pull request as draft July 23, 2024 08:56
@andiwand
Copy link
Contributor Author

image

@andiwand andiwand changed the title refactor: Use Core CKF extrapolation after inwards extension refactor: Use Core CKF extrapolation after inwards extension in Examples Jul 30, 2024
noemina
noemina previously approved these changes Jul 30, 2024
Copy link
Contributor

@noemina noemina left a comment

Choose a reason for hiding this comment

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

Looks good to me. Approving.

@andiwand andiwand marked this pull request as ready for review July 30, 2024 08:55
Copy link
Contributor

@timadye timadye left a comment

Choose a reason for hiding this comment

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

I can do my usual ITk ttbar_pu200 test, but I don't think we have much in the way of resolution tests there.

@timadye
Copy link
Contributor

timadye commented Jul 30, 2024

The ITk ttbar_pu200 efficiency etc are not significantly changed by this PR. I can't speak to the resolutions. However I do see a 4.7% speedup :)

It's a long time since I looked into it, but would it be a big addition to add the performance analysis plots (where we can check resolutions) to the full_chain*.py tests? I think many people trying out ACTS could benefit from more detailed results.

ATLAS can test this out in Athena, so I guess it's not a top priority for us.

@andiwand
Copy link
Contributor Author

It's a long time since I looked into it, but would it be a big addition to add the performance analysis plots (where we can check resolutions) to the full_chain*.py tests? I think many people trying out ACTS could benefit from more detailed results.

I think that makes sense yeah - I am reworking the root outputting a bit which somehow ended up at the very end of my current list of tasks but generally I would like to have finder and fitter performance for the CKF instead of "CKF performance" and track summary.

@andiwand andiwand requested a review from timadye July 30, 2024 11:16
Copy link

sonarcloud bot commented Jul 30, 2024

@kodiakhq kodiakhq bot merged commit 555e6ce into acts-project:main Jul 30, 2024
48 checks passed
@andiwand andiwand deleted the refactor-examples-track-finding-use-ckf-extrapolation branch July 30, 2024 14:00
@acts-project-service acts-project-service added the Fails Athena tests This PR causes a failure in the Athena tests label Jul 30, 2024
@paulgessinger paulgessinger modified the milestones: next, v36.1.0 Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes Performance Component - Examples Affects the Examples module Fails Athena tests This PR causes a failure in the Athena tests Track Finding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants