elmcraft / core-extra

Utility functions for an improved experience with elm/core
https://package.elm-lang.org/packages/elmcraft/core-extra/latest/
Other
22 stars 10 forks source link

`Float.Extra.aboutEqual` weirdnesses #41

Open sebsheep opened 4 months ago

sebsheep commented 4 months ago

Describe the bug The test for aboutEqual doesn't pass, plus it makes some floats equal whereas there are clearly not.

To Reproduce

Running 1 test. To reproduce these results, run: elm-test --fuzz 10000 --seed 225971905022788

↓ FloatTests
✗ makes numbers about equal even after some operations

Given 8.98846567431158e+307

    False
    ╷
    │ Expect.equal
    ╵
    True

in the repl:

> (1.00002 + 1.00002) |> Float.Extra.aboutEqual 2.00003
True : Bool

Expected behavior I'm not sure about what should be the "good behavior". The fuzz test shows that aboutEqual is "too strict", whereas the 1.00002 + 1.00002 is "too loose".

Maybe we should provide multiple functions aboutEqual function like aboutEqualReallyStrict/aboutEqualStrict/aboutEqualLoose/aboutEqualReallyLoose, or adding an integer parameter indicating the "strictness" of this function.

Maybe it's even not a core lib's library to make such choices, so we shouldn't provide this function at all.

Additional context

We could consider the following implementation : https://github.com/sindresorhus/float-equal/blob/main/index.js

Translated in elm with the "strictness" parameter (which we can be the number of significant bits, with 52 as the maximum precision):

{-| Corresponds to Number.EPSILON.
See: <https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/EPSILON>
-}
epsilon : Float
epsilon =
    2 ^ -52

aboutEqual : { significantBits : Int } -> Float -> Float -> Bool
aboutEqual { significantBits } a b =
    if isInfinite a then
        isInfinite b

    else if isInfinite b then
        False

    else
        let
            diff =
                abs (a - b)
        in
        (diff < epsilon)
            || (diff <= (2.0 ^ toFloat (52 - significantBits)) * epsilon * min (abs a) (abs b))
gampleman commented 4 months ago
(1.00002 + 1.00002) |> Float.Extra.aboutEqual 2.00003

This is expected. That's what "about equality" is about (sorry).

You can also read in the docs:

Note: this is unlikely to be appropriate if you are performing computations much smaller than one.

Although that could be phrased a bit more clearly. Either way, we should probably also expose a function that allows you to control the relevant parameters. In our implementation there are 2 - the absolute and relative difference. They are set to 1e-5 and 1e-8 respectively, but if you wanted to detect 1.00002 + 1.00002 /= 2.00003, you would probably need to set the absolute to 1e-7 or so.

I'll look into the failing test, seems like you found a good seed.

gampleman commented 4 months ago

OK, looked into the test. The value it finds is very high and generally above the sort of common range this function is designed. I'll modify it to be something like:

testAboutEqual : Test
testAboutEqual =
    Test.only <|
        fuzz (Fuzz.floatRange -1.0e100 1.0e100) "makes numbers about equal even after some operations" <|
            \a ->
                ((a + 10 + a - 10 - a) * 2 / 2)
                    |> aboutEqual a
                    |> Expect.equal True