Add shouldBeNear expectation

[erikd]

Comparing floating point numbers for equalty is a bad idea. Instead
they should tested to see if they are close enough to one another.

[erikd]

Ping?

[sol]

Hey, sorry for the delay. For this to be useful this would at least need tests and more detailed documentation. Specifically I don't understand

• why is the first case necessary
• what happens on division by zero, e.g. when comparing 1e-20 and -1e-20

[erikd]

For this to be useful this would at least need tests

There's no tests listed in the cabal file. I've found the tests in the repo and I'm adding tests, but how do I run them?

why is the first case necessary

This is short circuit evaluation for the case where they are actually equal.

what happens on division by zero, e.g. when comparing 1e-20 and -1e-20

The code for the third case says (abs actual + abs expected) which could only be zero if both values are zero and that would be caught be the second case.

[sol]

I use ghci or sensei to run tests

Sent from mobile

On 3 Sep 2016, at 4:53 PM, Erik de Castro Lopo [email protected] wrote:

For this to be useful this would at least need tests

There's not tests listed on the cabal file. How do I run them?

why is the first case necessary

This is short circuit evaluation for the case where they are actually equal.

what happens on division by zero, e.g. when comparing 1e-20 and -1e-20

The code for the third case says (abs actual + abs expected) which could only be zero if both values are zero and that would be caught be the second case.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

[sol]

Tests are running on Travis now. Can you rebase on current master and make sure the build is green.

I also wouldn't mind if you remove some of the code duplication, e.g. by factoring common stuff out into a where-clause.

Then I would love to discuss the actual stuff, that is the formula and the chosen value for epsilon.

[erikd]

Ok, the build has passed.

Since the current code uses guards, its not possible to refactor common code into a where clause, but I could make it a separate top level function (and then not export it).

[sol]

I think you can use a where clause with guards, no?

Sent from mobile

On 6 Sep 2016, at 5:03 PM, Erik de Castro Lopo [email protected] wrote:

Ok, the build has passed.

Since the current code uses guards, its not possible to refactor common code into a where clause, but I could make it a separate top level function (and then not export it).

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

[erikd]

It works! Fixed, rebased and pushed.

[erikd]

For some reason I couldn't get one the tests working with ghc 7.10 even though it passed on all the other versions.

[sol]

I fixed this on master, so this is no longer necessary.

[erikd]

Ok, dropped that patch.

[sol]

source code comments are an anti-pattern, rather than commenting make your code more expressive, e.g.

otherwise = reportFail (relativeDifference < epsilon)

[erikd]

Agreed, that is neater.

[sol]

for me it's much easier to work with code that does not use abbreviations, the reason for this is that I have to use speech recognition for programming due to RSI. for that reason I would much prefer absoluteDifference here

[sol]

now finally regarding the interesting stuff, the formula, looking at the following example

x `shouldBeNear` 1e-300

I haven't tried, but what happens if x is

1. 0.9e-300
2. 0

would (1) fail and (2) pass?

should we always normalize against the expected value instead?

actual `shouldBeNear` expected
  | delta < epsilon = return ()
  | otherwise = actual `shouldBe` expected
  where
    epsilon = 1e-15
    absoluteDifference = abs (expected - actual)
    relativeDifference = absoluteDifference / abs expected
    delta
      | expected == 0 = absoluteDifference
      | otherwise = relativeDifference

[erikd]

I haven't tried, but what happens if x is

1. 0.9e-300
2. 0

would (1) fail and (2) pass?

That is what currently happens, and I agree, at first sight, doesn't make much sense, but this is the usual recommended way to compare floating point numbers.

However, now that I think about it,

1e-300 `shouldBeNear` 0.0

should pass (because actual is close to expected), but I think maybe

0.0 `shouldBeNear` 1e-300

should fail.

Let me think about this some more.

[erikd]

I have encoded my expectations in the tests.

[erikd]

@sol Does this seem ok?

[declension]

+1 on this change BTW!

Wouldn't it be nicer to report things like expected 123 ± 1% but got 125 or similar, as the expectation is of it being within a tolerance of the value not being the value?

[istathar]

@erikd is this still a valid approach or have you found a different way to tackle comparing Doubles?

[erikd]

@istathar I stopped using hspec about 6 years ago, in favor of hedgehog. However, I suspect this may still be a valid approach.