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

Seed Parameter Estimation Fix, main branch (2025.02.10.) #848

Merged

Conversation

krasznaa
Copy link
Member

This is a follow-up about the issue discussed (amongst other places) in #842. That in RelWithDebInfo mode we would see the following output from traccc_seq_example_cuda:

[bash][Celeborn]:out > ./build/cuda-fp64/bin/traccc_seq_example_cuda --compare-with-cpu

Running Full Tracking Chain Using CUDA

Detector Options:
├ Detector file:                   geometries/odd/odd-detray_geometry_detray.json
├ Material file:                   geometries/odd/odd-detray_material_detray.json
├ Surface grid file:               geometries/odd/odd-detray_surface_grids_detray.json
├ Use detray detector:             true
└ Digitization file:               geometries/odd/odd-digi-geometric-config.json
...
WARNING: @traccc::io::csv::read_cells: 28 duplicate cells found in /home/krasznaa/ATLAS/projects/traccc/traccc/data/odd/geant4_10muon_10GeV/event000000000-cells.csv
===>>> Event 0 <<<===
Number of measurements: 631 (host), 631 (device)
  Matching rate(s):
    - 100% at 0.01% uncertainty
    - 100% at 0.1% uncertainty
    - 100% at 1% uncertainty
    - 100% at 5% uncertainty
Number of spacepoints: 451 (host), 451 (device)
  Matching rate(s):
    - 100% at 0.01% uncertainty
    - 100% at 0.1% uncertainty
    - 100% at 1% uncertainty
    - 100% at 5% uncertainty
Number of seeds: 181 (host), 181 (device)
  Matching rate(s):
    - 100% at 0.01% uncertainty
    - 100% at 0.1% uncertainty
    - 100% at 1% uncertainty
    - 100% at 5% uncertainty
Number of track parameters: 181 (host), 181 (device)
  Matching rate(s):
    - 0% at 0.01% uncertainty
    - 0% at 0.1% uncertainty
    - 0% at 1% uncertainty
    - 0% at 5% uncertainty
Number of track candidates (header): 183 (host), 183 (device)
  Matching rate(s):
    - 0% at 0.01% uncertainty
    - 0% at 0.1% uncertainty
    - 0% at 1% uncertainty
    - 0% at 5% uncertainty
  Track candidates (item) matching rate: 100%
Number of track states: 183 (host), 183 (device)
  Matching rate(s):
    - 98.9071% at 0.01% uncertainty
    - 98.9071% at 0.1% uncertainty
    - 98.9071% at 1% uncertainty
    - 98.9071% at 5% uncertainty
...

(Note that in these examples I built the code with FP64 precision, to make the point even more strongly.)

With this small update, this changes to:

[bash][Celeborn]:out > ./build/cuda-fp64/bin/traccc_seq_example_cuda --compare-with-cpu

Running Full Tracking Chain Using CUDA

Detector Options:
├ Detector file:                   geometries/odd/odd-detray_geometry_detray.json
├ Material file:                   geometries/odd/odd-detray_material_detray.json
├ Surface grid file:               geometries/odd/odd-detray_surface_grids_detray.json
├ Use detray detector:             true
└ Digitization file:               geometries/odd/odd-digi-geometric-config.json
...
WARNING: @traccc::io::csv::read_cells: 28 duplicate cells found in /home/krasznaa/ATLAS/projects/traccc/traccc/data/odd/geant4_10muon_10GeV/event000000000-cells.csv
===>>> Event 0 <<<===
Number of measurements: 631 (host), 631 (device)
  Matching rate(s):
    - 100% at 0.01% uncertainty
    - 100% at 0.1% uncertainty
    - 100% at 1% uncertainty
    - 100% at 5% uncertainty
Number of spacepoints: 451 (host), 451 (device)
  Matching rate(s):
    - 100% at 0.01% uncertainty
    - 100% at 0.1% uncertainty
    - 100% at 1% uncertainty
    - 100% at 5% uncertainty
Number of seeds: 181 (host), 181 (device)
  Matching rate(s):
    - 100% at 0.01% uncertainty
    - 100% at 0.1% uncertainty
    - 100% at 1% uncertainty
    - 100% at 5% uncertainty
Number of track parameters: 181 (host), 181 (device)
  Matching rate(s):
    - 100% at 0.01% uncertainty
    - 100% at 0.1% uncertainty
    - 100% at 1% uncertainty
    - 100% at 5% uncertainty
Number of track candidates (header): 183 (host), 183 (device)
  Matching rate(s):
    - 100% at 0.01% uncertainty
    - 100% at 0.1% uncertainty
    - 100% at 1% uncertainty
    - 100% at 5% uncertainty
  Track candidates (item) matching rate: 100%
Number of track states: 183 (host), 183 (device)
  Matching rate(s):
    - 100% at 0.01% uncertainty
    - 100% at 0.1% uncertainty
    - 100% at 1% uncertainty
    - 100% at 5% uncertainty
...

Though without backward filtering, the agreement is indeed not perfect. So that part is still as we discussed earlier. 🤔

[bash][Celeborn]:out > ./build/cuda-fp64/bin/traccc_seq_example_cuda --compare-with-cpu --fit-use-backward-filter=false

Running Full Tracking Chain Using CUDA

Detector Options:
├ Detector file:                   geometries/odd/odd-detray_geometry_detray.json
├ Material file:                   geometries/odd/odd-detray_material_detray.json
├ Surface grid file:               geometries/odd/odd-detray_surface_grids_detray.json
├ Use detray detector:             true
└ Digitization file:               geometries/odd/odd-digi-geometric-config.json
...
WARNING: @traccc::io::csv::read_cells: 28 duplicate cells found in /home/krasznaa/ATLAS/projects/traccc/traccc/data/odd/geant4_10muon_10GeV/event000000000-cells.csv
===>>> Event 0 <<<===
Number of measurements: 631 (host), 631 (device)
  Matching rate(s):
    - 100% at 0.01% uncertainty
    - 100% at 0.1% uncertainty
    - 100% at 1% uncertainty
    - 100% at 5% uncertainty
Number of spacepoints: 451 (host), 451 (device)
  Matching rate(s):
    - 100% at 0.01% uncertainty
    - 100% at 0.1% uncertainty
    - 100% at 1% uncertainty
    - 100% at 5% uncertainty
Number of seeds: 181 (host), 181 (device)
  Matching rate(s):
    - 100% at 0.01% uncertainty
    - 100% at 0.1% uncertainty
    - 100% at 1% uncertainty
    - 100% at 5% uncertainty
Number of track parameters: 181 (host), 181 (device)
  Matching rate(s):
    - 100% at 0.01% uncertainty
    - 100% at 0.1% uncertainty
    - 100% at 1% uncertainty
    - 100% at 5% uncertainty
Number of track candidates (header): 183 (host), 183 (device)
  Matching rate(s):
    - 100% at 0.01% uncertainty
    - 100% at 0.1% uncertainty
    - 100% at 1% uncertainty
    - 100% at 5% uncertainty
  Track candidates (item) matching rate: 100%
Number of track states: 183 (host), 183 (device)
  Matching rate(s):
    - 80.3279% at 0.01% uncertainty
    - 83.6066% at 0.1% uncertainty
    - 84.153% at 1% uncertainty
    - 84.153% at 5% uncertainty
...

It took me a little while to realize that we're not actually using detray::bound_parameters_vector in that part of the code, but just the underlying algebra-plugins "vector type" directly. So the params variable was in an undefined state over there. Leading to its time property ending up being undefined. (As it's not set to anything explicitly by either the host or the device code.) And this undefined time value made its way all the way down to the final fitted tracks. Leading to an imperfect (though not 0%) agreement even there. 🤔

@krasznaa krasznaa added the bug Something isn't working label Feb 10, 2025
@krasznaa
Copy link
Member Author

Note that one could also just set the time parameter to 0.f explicitly, instead of using matrix::zero. Which may be an optimization for the future... 🤔

@krasznaa krasznaa force-pushed the ParamsEstimationFix-main-20250210 branch from c9f9e68 to 84eaeca Compare February 10, 2025 12:15
@stephenswat stephenswat merged commit 701f55b into acts-project:main Feb 10, 2025
23 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants