Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Float.Extra.aboutEqual weirdnesses #41

Open
sebsheep opened this issue Feb 26, 2024 · 2 comments
Open

Float.Extra.aboutEqual weirdnesses #41

sebsheep opened this issue Feb 26, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@sebsheep
Copy link

sebsheep commented Feb 26, 2024

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))
@sebsheep sebsheep added the bug Something isn't working label Feb 26, 2024
@gampleman
Copy link
Collaborator

(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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants