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

Change the default smoother to two-filters smoother #842

Merged
merged 2 commits into from
Feb 5, 2025

Conversation

beomki-yeo
Copy link
Contributor

@beomki-yeo beomki-yeo commented Feb 5, 2025

This will fix the issue on bad matching rate between CPU and GPU, reported by @stephenswat and @krasznaa

@beomki-yeo
Copy link
Contributor Author

beomki-yeo commented Feb 5, 2025

Yeah it's not working for the drift chamber where the navigation can get unstable.
Fitting with wire chamber won't be OK until we implement (1) direct navigator and (2) deterministic annealing filter.
The current tests with the wire chamber should not be taken seriously.

Copy link

sonarqubecloud bot commented Feb 5, 2025

Copy link
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

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

Okay... Let's treat this as a quick fix to the observed issue.

What I'll (or somebody else will) do is to introduce traccc::opts::track_finding, to make this (and the other track finding options) configurable from the command line. As it really should be...

@krasznaa krasznaa merged commit ae79eb3 into acts-project:main Feb 5, 2025
29 checks passed
@beomki-yeo
Copy link
Contributor Author

This kills the performance a bit but it is expected as propagation runs twice..

@krasznaa
Copy link
Member

krasznaa commented Feb 6, 2025

This kills the performance a bit but it is expected as propagation runs twice..

"A bit" may be a bit of an understatement unfortunately. 😭 Lately our code has slowed down a lot... Something to be discussed in tomorrow's meeting.

@beomki-yeo
Copy link
Contributor Author

It's a drop from correcting the mathematics so... take it as a necessary evil..

@beomki-yeo
Copy link
Contributor Author

beomki-yeo commented Feb 6, 2025

And KF will be trivial once we implement the ambinsolver after CKF.

If i am correct, KF is used in refitting only but it is not a part of ITk chain

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants