feat: safe divide

[AJPfleger]

Prevents FPEs by giving a good estimate, when i comes to a potentially dangerous division.

Example use case is the pathCorrection calculation for the cylinderSurface. We do not want to fail the navigation, and instead give an good estimate for the correction:

• fix: add safeDivide to cylinderSurface pathCorrection #3254

[andiwand]

Is an exception really better than an FPE?

[AJPfleger]

At least it is not undefined behaviour. Alternatively I thought about
return std::numeric_limits<T>::quiet_NaN(); or boldly using something like +-inf.

Third option could be using something like in safeInverse and return std::nullopt; or so in case we cannot divide.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47.66%. Comparing base (80a03ea) to head (a5454d7).
Report is 284 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3253      +/-   ##
==========================================
+ Coverage   47.65%   47.66%   +0.01%     
==========================================
  Files         507      507              
  Lines       29205    29212       +7     
  Branches    14010    14011       +1     
==========================================
+ Hits        13917    13924       +7     
  Misses       5264     5264              
  Partials    10024    10024              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

📊: Physics performance monitoring for a5454d7

Full contents

physmon summary

• ✅ CKF truth_smeared
• ✅ IVF truth_smeared
• ✅ AMVF truth_smeared
• ✅ Track Summary CKF truth_smeared
• ✅ Seeding truth_estimated
• ✅ CKF truth_estimated
• ✅ IVF truth_estimated
• ✅ AMVF truth_estimated
• ✅ Track Summary CKF truth_estimated
• ✅ Seeding seeded
• ✅ CKF seeded
• ✅ IVF seeded
• ✅ AMVF seeded
• ✅ AMVF (+grid seeder) seeded
• ✅ Track Summary CKF seeded
• ✅ Seeding orthogonal
• ✅ CKF orthogonal
• ✅ IVF orthogonal
• ✅ AMVF orthogonal
• ✅ Track Summary CKF orthogonal
• ✅ Ambisolver seeded
• ✅ Ambisolver orthogonal
• ✅ Seeding ttbar
• ✅ CKF ttbar
• ✅ Ambisolver
• ✅ Track Summary CKF ttbar
• ✅ AMVF ttbar
• ✅ AMVF (+grid seeder) ttbar
• ✅ Truth tracking (GSF)
• ✅ Truth tracking
• ✅ Truth tracking (GX2F)
• ✅ Particles fatras
• ✅ Particles geant4

[andiwand]

Well but it is the kind of UB which always behaves nicely in reality in my experience 😄

I think if we go hard on all UB we have to change quite some code in Acts

The idea of the other helpers was to return optional values in case of a problem. But I have a hard time seeing the use case here

[paulgessinger]

I'm also not entirely convinced.

With the FPE monitoring, we have the ability to fix faulty operations only where they occur.
At the very least, we should only apply these functions in cases where we have observed FPEs.

In case of the ROOT writers, I think we'd either want to manually handle them, in which case an exception is not necessarily the best way to go, or fix the underlying cause for the FPE. The optional return imo has little benefit over checking the denominator manually at the call site.

Also, is FLTDIV even UB? I thought it always results in inf which is specified.

EDIT: I don't think it's UB. IEEE 754 specifies that any nonzero float divided by zero is +- infinity. So imo we should really only work around that if it causes concrete issues.

[paulgessinger]

In conclusion, I think I'm okay having this and using it in cases where we've observed FPE FLTDIVs. But I would not start applying these functions everywhere without a reason.

[AJPfleger]

sounds good to me. I made this, since I had the concrete case in the linked PR :)

[andiwand]

I am still not convinced by this. The only use we have is in #3255 and it looks to me like treating symptoms. Instead of returning a valid inf value we return an arbitrary high number. For the user of this function it is not clear if the threshold was hit or not.

Comparing a potential use how we would do it today

const double a = nomindator;
const double b = denominator;
if (std::abs(b) < some physical value) {
  do something
}
const double c = a / b;

Against the proposed solution

const double a = nomindator;
const double b = denominator;
const double c = safeDiv(a, b)

We lose lose the condition and we lose the physical cutoff value.

[AJPfleger]

Currently not needed anymore, since the only usecase

• fix: add safeDivide to cylinderSurface pathCorrection #3254

was abandoned.