Fixed unsafe floating point comparisons

added 1 commit

f428d784 - Fixed unsafe floating point comparisons

Compare with previous version

added 19 commits

f428d784...2ec6ac1b - 9 earlier commits
160c6a6f - Dropped useless (and problematic) operator== in HistoDef
06c646a6 - Fixed float comparisons in ParticlePropertySvc.cpp
21a24596 - Attempt to make HistoDump close to readable. No code change in principle
d1fd8b28 - Fixed float comparisons in HistoDump.cpp
dc01dfc1 - Do not define operator== for floating point based properties
f80c0925 - Fixed float comparisons in PropertyAlg.cpp
4c528b20 - Fixed float comparisons in EvtCollectionSelector.cpp
1a508906 - Fixed float comparisons in CPUCruncher.cpp
f8723d34 - Fixed float comparisons in CPUCrunchSvc.cpp
24b4fb43 - Fixed float comparisons in HepRndmGenerators.cpp

Compare with previous version

changed the description

added 7 commits

80ced213 - Fixed float comparisons in HistoDump.cpp
8b34df6e - Do not define operator== for floating point based properties
2659b743 - Fixed float comparisons in PropertyAlg.cpp
0bb8f744 - Fixed float comparisons in EvtCollectionSelector.cpp
498e83e2 - Fixed float comparisons in CPUCruncher.cpp
3930793e - Fixed float comparisons in CPUCrunchSvc.cpp
027d488d - Fixed float comparisons in HepRndmGenerators.cpp

Compare with previous version

std::numeric_limits<double>::epsilon() (2.22045e-16) might be good to compare doubles in the range 1-10, but it fails when we compare very small numbers where we may prefer std::numeric_limits<double>::min() (2.22507e-308) or, even better, a normalized difference (e.g. abs((a-b)/b) < epsilon()).

Said that, I'm still not convinced that all x == 0.0 must be changed. Sometimes they mean * x is small*, but sometimes it's really a x is exactly 0.

For the record, I think there are plenty of cases where == is 'just fine', or, even stronger, what is really intended.

For example, when a value is initialized with a given (default) value, which is then subsequently (bitwise) copied, it makes sense to compare the (potentially modified) value with the known default using ==. Basically, in cases like that, the two values to be compared are not the result of 'equivalent' computations which may eg. round differently.

Let me try to answer all conversations in one go :

thanks @clemenci for spotting that I messed up on line 368. Indeed it should have been

        } while ( abs( v ) < std::numeric_limits<double>::epsilon() * m_specs->cut() );

where epsilon is used with respect with the order of magnitude we are dealing with. I've fixed it locally and will push soon

neither epsilon, nor min are fine without an order of magnitude next to them, meaning it indeed needs normalization
the only case where == can make sense for floats is indeed when you "compare the (potentially modified) value with the known default". But in such cases, why not e.g. use a boolean to check whether the thing was modified at all ? Or an std::optional ?
and as usual the value of this is not in the trivial, annoying cases. But in the few cases where we really have bugs. And we need clean output for finding these

added 1 commit

f4db703d - Fixed float comparisons in HepRndmGenerators.cpp

Compare with previous version

resolved all threads

added 49 commits

f4db703d...2dd6efd7 - 29 commits from branch master
2dd6efd7...01046254 - 10 earlier commits
351c2d0c - Dropped useless (and problematic) operator== in HistoDef
36f4f0bf - Fixed float comparisons in ParticlePropertySvc.cpp
e8d1cef6 - Attempt to make HistoDump close to readable. No code change in principle
bb943d90 - Fixed float comparisons in HistoDump.cpp
24cbbfee - Do not define operator== for floating point based properties
cdeffe1c - Fixed float comparisons in PropertyAlg.cpp
eaa2a87a - Fixed float comparisons in EvtCollectionSelector.cpp
748a9102 - Fixed float comparisons in CPUCruncher.cpp
47c49947 - Fixed float comparisons in CPUCrunchSvc.cpp
34dd600d - Fixed float comparisons in HepRndmGenerators.cpp

Compare with previous version

added 1 commit

484181ac - Fixed float comparisons in HepRndmGenerators.cpp

Compare with previous version

There was actually another reason for the infinite loop : I had used abs instead of std::abs. Ans somehow, with all the using namespace in that code, the abs function I was using always returned 0.... Fixed now. Tests are back green.

added 20 commits

484181ac...d121a4b4 - 10 earlier commits
4ab43e5a - Dropped useless (and problematic) operator== in HistoDef
cfba0cea - Fixed float comparisons in ParticlePropertySvc.cpp
c815a34d - Attempt to make HistoDump close to readable. No code change in principle
fe3d9c22 - Fixed float comparisons in HistoDump.cpp
3f02752e - Do not define operator== for floating point based properties
4758baf6 - Fixed float comparisons in PropertyAlg.cpp
d87d332f - Fixed float comparisons in EvtCollectionSelector.cpp
2d6baa53 - Fixed float comparisons in CPUCruncher.cpp
dba8f945 - Fixed float comparisons in CPUCrunchSvc.cpp
f722e7de - Fixed float comparisons in HepRndmGenerators.cpp