fix!: Use ActsScalar in binning tools

[stephenswat]

The current binning tools use floats everywhere, which is prone to conversion warnings. This commit converts that code to use the ActsScalar type alias instead.

[paulgessinger]

Looks good overall, waiting for v37 to merge this.

[github-actions]

📊: Physics performance monitoring for 05f8805

Full contents

physmon summary

• ✅ Particles fatras
• ✅ Particles geant4
• ✅ Particles ttbar
• ✅ Vertices ttbar
• ✅ Truth tracking (KF)
• ✅ Truth tracking (GSF)
• ✅ Truth tracking (GX2F)
• ✅ CKF | trackfinding | single muon | truth smeared seeding
• ✅ Track Summary CKF | trackfinding | single muon | truth smeared seeding
• ✅ Seeding trackfinding | single muon | truth estimated seeding
• ✅ CKF | trackfinding | single muon | truth estimated seeding
• ✅ Track Summary CKF | trackfinding | single muon | truth estimated seeding
• ✅ Seeding trackfinding | single muon | default seeding
• ✅ CKF | trackfinding | single muon | default seeding
• ✅ Track Summary CKF | trackfinding | single muon | default seeding
• ✅ Seeding trackfinding | single muon | orthogonal seeding
• ✅ CKF | trackfinding | single muon | orthogonal seeding
• ✅ Track Summary CKF | trackfinding | single muon | orthogonal seeding
• ✅ Seeding trackfinding | 4 muon x 50 vertices | default seeding
• ✅ CKF | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ Track Summary CKF | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ Ambisolver | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ IVF notime | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ AMVF gauss notime | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ AMVF grid time | trackfinding | 4 muon x 50 vertices | default seeding
• 🔴 Seeding trackfinding | ttbar with 200 pileup | default seeding
• 🔴 CKF | trackfinding | ttbar with 200 pileup | default seeding
• 🔴 Track Summary CKF | trackfinding | ttbar with 200 pileup | default seeding
• 🔴 Ambisolver | trackfinding | ttbar with 200 pileup | default seeding
• 🔴 AMVF gauss notime | trackfinding | ttbar with 200 pileup | default seeding
• 🔴 AMVF grid time | trackfinding | ttbar with 200 pileup | default seeding

[sonarqubecloud]

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
81.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[andiwand]

always interesting to see what triggers the 3 ttbar events to change

[AJPfleger]

I was looking at the physmon output. Some of the plots have random spikes. The outliers in CKF | trackfinding | ttbar with 200 pileup | default seeding change a lot.

The use of double instead of float seems correct, but I think we should investigate the underlying cause first before getting this in. What do you think?

[stephenswat]

Huh, that's peculiar. 🤔

[andiwand]

Ultimately I think this changed the seeding a bit which then changes the track finding a bit. The plots don't scare me too much since we have very low stats there.

What I am not sure about is IF we actually want to change the seeding here? @stephenswat @paulgessinger @asalzburger

If that is clear I am happy to approve and bring this in.

[AJPfleger]

Regarding the IF, I have no strong opinion.

It's annoying for the low stats, but maybe we should re-run with better stats. I will check the logs, how expensive the individual tests are, but if we are lucky we could upscale them at a cost of an extra minute or so :3

[paulgessinger]

Why this affects the seeding is not entirely clear to me.

I guess it could affect Fatras and then subsequently reconstruction.

[AJPfleger]

The current physmon tests take around 11-18 minutes. In the trackfinding_ttbar_pu200 we spend around 75-135 seconds. I think, we could double the number of events from 3 to 6, but everything else would take too much time. I tried it with 30 events and it takes 850s on github. As expected, we scale here linearly.

[andiwand]

Why this affects the seeding is not entirely clear to me.

I guess it could affect Fatras and then subsequently reconstruction.

Isn't

• Core/include/Acts/Utilities/BinUtility.hpp
• Core/include/Acts/Utilities/BinningData.hpp
  something that is used in seeding?

The current physmon tests take around 11-18 minutes. In the trackfinding_ttbar_pu200 we spend around 75-135 seconds. I think, we could double the number of events from 3 to 6, but everything else would take too much time. I tried it with 30 events and it takes 850s on github. As expected, we scale here linearly.

I think we would need 100-1000 events so I would rather keep it as it is for now if we cannot reach that

[paulgessinger]

I was under the impression it uses the Grid for the internal binning, but I could be wrong.

[CarloVarni]

In the seeding we use the Acts::Grid, but I'm not familiar with neither of Core/include/Acts/Utilities/BinUtility.hpp or Core/include/Acts/Utilities/BinningData.hpp and I don't know if they may be use internally somewhere

[stephenswat]

Invalidated by #3873.

[AJPfleger]

we could still have this with double instead of the floats, no?