We use compare_approx when comparing appraisal metrics so that assets with approximately equal metrics are considered equal (then falling back to a series of secondary criteria, such as favouring existing assets over new ones). This is to ensure that we don't get changes in investment decisions caused by tiny floating point differences (e.g. on different operating systems).
Unfortunately, there's a flaw here. Consider the following three assets (real example from #1402):
- Asset 27:
26.871084337032230
- Asset 28:
26.871084337032244
- Asset 32:
26.871084337032247
Using compare_approx, you get the following:
- 27 vs 28 -> equal
- 28 vs 32 -> equal
- 27 vs 32 -> less than
The transitivity rule is that if a == b and b == c, then a == c must also be true.
If there's an inconsistency and this transitivity rule doesn't hold, then sort_by (called here) will detect this inconsistency and panic.
This is hopefully quite a rare problem, but it's cropping up in #1402, and something that we absolutely need to address. It's also platform dependent - the example for #1402 isn't panicking for me on my mac, as the appraisal metrics are slightly different so transitivity isn't broken, but is panicking for Adam on WSL.
A naive approach might be to use slightly lower precision in compare_approx. That doesn't fix the underlying problem, but would hopefully make it rarer. Alternatively we could round to fixed precision before comparing. There's probably a better solution though. This slightly reminds me of some of the issues with #1296, so there may be some inspiration there.
We use
compare_approxwhen comparing appraisal metrics so that assets with approximately equal metrics are considered equal (then falling back to a series of secondary criteria, such as favouring existing assets over new ones). This is to ensure that we don't get changes in investment decisions caused by tiny floating point differences (e.g. on different operating systems).Unfortunately, there's a flaw here. Consider the following three assets (real example from #1402):
26.87108433703223026.87108433703224426.871084337032247Using
compare_approx, you get the following:The transitivity rule is that if
a == bandb == c, thena == cmust also be true.If there's an inconsistency and this transitivity rule doesn't hold, then
sort_by(called here) will detect this inconsistency and panic.This is hopefully quite a rare problem, but it's cropping up in #1402, and something that we absolutely need to address. It's also platform dependent - the example for #1402 isn't panicking for me on my mac, as the appraisal metrics are slightly different so transitivity isn't broken, but is panicking for Adam on WSL.
A naive approach might be to use slightly lower precision in
compare_approx. That doesn't fix the underlying problem, but would hopefully make it rarer. Alternatively we could round to fixed precision before comparing. There's probably a better solution though. This slightly reminds me of some of the issues with #1296, so there may be some inspiration there.