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

Record track quality to the CKF tracks #844

Merged
merged 2 commits into from
Feb 20, 2025

Conversation

beomki-yeo
Copy link
Contributor

track quality is the struct for the NDF, chi2 and the number of holes.
This has been recorded in the KF but we can also calculate it for the CKF.

This will allow us to apply the ambiguity resolution to the CKF output directly, and reduce the KF time significantly.

@beomki-yeo beomki-yeo marked this pull request as draft February 6, 2025 02:06
@beomki-yeo beomki-yeo force-pushed the chi2-ckf branch 3 times, most recently from 335392a to 140e39a Compare February 9, 2025 10:16
@beomki-yeo beomki-yeo marked this pull request as ready for review February 9, 2025 10:19
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.

I'm generally on board. Code-wise I like how you did things.

But do you have some measurements to what it does to our runtime performance? I understand that the KF should become faster (not necessary?) after this. But do you have any quantitative numbers on that? 🤔

@beomki-yeo
Copy link
Contributor Author

beomki-yeo commented Feb 10, 2025

I understand that the KF should become faster (not necessary?) after this.

No this PR is not for performance improvement.
Instead, this PR will allow us moving the ambi-solver, which needs chi2 and number of holes information, before the KF. Then, KF will fit less number of tracks.

@niermann999
Copy link
Contributor

I understand that the KF should become faster (not necessary?) after this.

No this PR is not for performance improvement. Instead, this PR will allow us moving the ambi-solver, which needs chi2 information, before the KF. Then, KF will fit less tracks.

Sorry if this is a stupid question, but do we have/plan to add a GPU ambi solver or is this a CPU-only development?

@stephenswat
Copy link
Member

Sorry if this is a stupid question, but do we have/plan to add a GPU ambi solver or is this a CPU-only development?

If we end up running the ambiguity resolution between the CKF and the KF we will want to build a GPU ambiguity resolver.

@beomki-yeo
Copy link
Contributor Author

I will move the current cpu amisolver to between ckf and kf, and implement the gpu ambl-solver in a similar fasion

@beomki-yeo
Copy link
Contributor Author

beomki-yeo commented Feb 19, 2025

Can I go with the PR? There are still some PRs left before I can move the ambiguity resolver between the CKF and KF.

There is no big difference in performance. Anyway this PR itself is not for performance improvement

Main

byeo@hermes:/mnt/nvme0n1/byeo/projects/traccc/traccc_build$ ./bin/traccc_benchmark_cuda 
Please be patient. It may take some time to generate the simulation data.
2025-02-10T14:40:40-08:00
Running ./bin/traccc_benchmark_cuda
Run on (32 X 3000 MHz CPU s)
CPU Caches:
  L1 Data 32 KiB (x16)
  L1 Instruction 32 KiB (x16)
  L2 Unified 512 KiB (x16)
  L3 Unified 16384 KiB (x8)
Load Average: 1.35, 1.96, 1.93
***WARNING*** CPU scaling is enabled, the benchmark real time measurements may be noisy and will incur extra overhead.
WARNING: No entries in volume finder

Detector check: OK
------------------------------------------------------------------------------------
Benchmark                          Time             CPU   Iterations UserCounters...
------------------------------------------------------------------------------------
ToyDetectorBenchmark/CUDA 1.2447e+10 ns   1.2446e+10 ns            1 event_throughput_Hz=8.03452/s

This PR

byeo@hermes:/mnt/nvme0n1/byeo/projects/traccc/traccc_build$ ./bin/traccc_benchmark_cuda 
Please be patient. It may take some time to generate the simulation data.
2025-02-10T14:28:21-08:00
Running ./bin/traccc_benchmark_cuda
Run on (32 X 3000 MHz CPU s)
CPU Caches:
  L1 Data 32 KiB (x16)
  L1 Instruction 32 KiB (x16)
  L2 Unified 512 KiB (x16)
  L3 Unified 16384 KiB (x8)
Load Average: 1.10, 1.63, 1.51
***WARNING*** CPU scaling is enabled, the benchmark real time measurements may be noisy and will incur extra overhead.
WARNING: No entries in volume finder

Detector check: OK
------------------------------------------------------------------------------------
Benchmark                          Time             CPU   Iterations UserCounters...
------------------------------------------------------------------------------------
ToyDetectorBenchmark/CUDA 1.2514e+10 ns   1.2513e+10 ns            1 event_throughput_Hz=7.99149/s

@beomki-yeo beomki-yeo merged commit efb743d into acts-project:main Feb 20, 2025
29 checks passed
beomki-yeo added a commit to beomki-yeo/traccc that referenced this pull request Feb 20, 2025
beomki-yeo added a commit to beomki-yeo/traccc that referenced this pull request Feb 20, 2025
beomki-yeo added a commit to beomki-yeo/traccc that referenced this pull request Feb 20, 2025
beomki-yeo added a commit to beomki-yeo/traccc that referenced this pull request Feb 20, 2025
beomki-yeo added a commit to beomki-yeo/traccc that referenced this pull request Feb 20, 2025
stephenswat added a commit that referenced this pull request Feb 21, 2025
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.

4 participants