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

how to compare floating point numbers? #337

Open
sfindeisen opened this issue Jun 16, 2022 · 13 comments
Open

how to compare floating point numbers? #337

sfindeisen opened this issue Jun 16, 2022 · 13 comments

Comments

@sfindeisen
Copy link

Hi, what is the recommended way to go about comparing floating point numbers? I am aware of @?= and @=? operators, which route to assertEqual, but this of course doesn't work. What would be nice to have is assertClose (or similar) to compare 2 floating point numbers with respect to some predefined ε, i.e.: abs(a-b) < ε. Ideally ε would be configurable (somehow). What do you think? Will you accept a patch?

@andreasabel
Copy link
Collaborator

Can't you use a newtype with a suitable Eq instance? (Ignoring that epsilon-distance isn't actually transitive, so it is not an equivalence relation.)

newtype Precision5 = Precision5 Double

instance Eq Precision5 where
  Precision5 x == Precision5 y = abs (x-y) < 0.00001

@sfindeisen
Copy link
Author

Technically speaking we could get away with this, yes, but why do you consider violating Eq transitivity to be a recommended practice? Plus, this complicates client code due to the extra type.

I will be happy to implement a patch for you, if this makes sense?

@Bodigrim
Copy link
Collaborator

If you try using assertClose, you'll quickly discover that sometimes you want to check abs(a-b) < ε, sometimes abs((a-b)/a) < ε, sometimes both, sometimes any. It's a job for a separate opinionated package, not for the core.

@andreasabel
Copy link
Collaborator

Yeah, maybe defining your own assert... functions/operators on top of assertFailure might be the way to go. Unless there is a clear consensus what the most common operators would be for floats, it does not make sense to add them here.

@sfindeisen
Copy link
Author

How about just a single, unary operator for floats: abs(x) < ε. Would it cover all the cases?

@Bodigrim
Copy link
Collaborator

To be honest, abs(x) < ε looks even more ad-hoc.

FWIW when it comes to this kind of tests, I find it more expressive to use QuickCheck instead hunit.

@Mikolaj
Copy link

Mikolaj commented Jun 17, 2022

I suppose, if we define our own operator using tasty primitives, we can then make a tasty PR from it so that tasty devs can decide without blocking us. And if not enough primitives are exposed or they don't have the desired semantics, we can complain regardless of whether the operator is for our own library or for extending Tasty.Hunit.

@sfindeisen
Copy link
Author

We eventually came up with such a design: https://github.com/sfindeisen/horde-ad/blob/sfindeisen/fix-46/test/common/TestCommonEqEpsilon.hs , see AssertClose class and its several instances. It uses HUnit-approx. Any interest?

Right now I am polishing this up and will delete assertion messages because we don't need them in our project.

@Mikolaj
Copy link

Mikolaj commented Jul 2, 2022

I was actually quite surprised https://hackage.haskell.org/package/HUnit-approx could be made to work with tasty. That's either a happy coincidence or the power of good Haskell APIs. And, after @sfindeisen proved it to be the case and my world-view reconfigured, I'm now surprised it's not yet integrated with tasty. [edit: "now", not "not"]

@Bodigrim
Copy link
Collaborator

Bodigrim commented Jul 2, 2022

That's no coincedence, tasty has a modular architecture, and @UnkindPartition always argued that anything which can be implemented as a plugin and maintained independently should be a separate package, spreading load away from core maintainers.

@Mikolaj
Copy link

Mikolaj commented Jul 3, 2022

Plugins such as https://hackage.haskell.org/package/tasty-quickcheck and https://hackage.haskell.org/package/tasty-hunit? So would tasty-hunit-approx be a sub-plugin of https://hackage.haskell.org/package/tasty-hunit or a separate plugin of tasty?

@Bodigrim
Copy link
Collaborator

Bodigrim commented Jul 3, 2022

Probably a separate plugin, implementing a test provider for HUnit-approx.

sfindeisen added a commit to Mikolaj/horde-ad that referenced this issue Jul 4, 2022
* implement eq-epsilon option

* implement support for eq-epsilon in dumbMnistTests and sgdTestCase

* apply eq-epsilon command line option to some test cases

* implement Precision5

UnkindPartition/tasty#337 (comment)

* implement assertClose

* replace Precision5 with assertClose

* make eq-epsilon a global variable using IORef and unsafePerformIO

This is almost identical to the current standard idiom for doing this: https://wiki.haskell.org/Top_level_mutable_state .

* add 2 comments

* implement assertCloseMulti

* replace @?= with assertCloseMulti

* simplify

* implement assertCloseElem

* replace elem with assertCloseElem

* fix msg

* add more comments

* polish up

* implement eqEpsilonDefault; rename assertCloseMulti

* whitespace fix

* implement hlint suggestions

* fix

* move eq-epsilon related stuff to a new file (TestCommonEqEpsilon.hs)

* move even more eq-epsilon related stuff to TestCommonEqEpsilon.hs

* implement support for eq-epsilon

* implement support for eq-epsilon

* whitespace fix

* implement (@?~) infix operator; generalize

* replace (@?=) with (@?~)

* replace assertClose with (@?~)

* fix cmdline syntax

* remove redundant brackets (hlint)

* fix

* fix warning

* plug in HUnit-approx/Test.HUnit.Approx

* define AssertClose class

* implement AssertClose instance for Traversable

* implement AssertClose instance ([a],a)

* relax TestSimpleDescent checks (with eq-epsilon)

* make it more generic

* refactor TestOutdated

* implement AssertClose instance for pairs

1. foldr on pairs does not work as expected: https://stackoverflow.com/questions/72798587/haskell-foldr-foldl-on-pairs
2. performance (skip asList)

* whitespace fix

* relax TestSingleGradient checks (with eq-epsilon)

* apply hlint

* resolve 2x TODO

* implement AssertClose instance for Storable vectors

* get rid of assertion message (not used)

* remove unused imports

Co-authored-by: Stanislaw Findeisen <[email protected]>
@Mikolaj
Copy link

Mikolaj commented Jul 6, 2022

Thanks. If somebody ever needs the feature, stumbles upon this ticket and wants to implement the plugin, we have all the essential parts and some example code using it in our library and the meat is in HUnit-approx, so this shouldn't be too hard. Committing to maintainership may be a more serious undertaking, but the tasty API doesn't move fast these days, so that's chasing GHCs mostly, probably?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants