typelevel / algebra

Experimental project to lay out basic algebra type classes
https://typelevel.org/algebra/
Other
378 stars 69 forks source link

Test field laws for Float/Double/BigDecimal. #192

Closed tixxit closed 7 years ago

tixxit commented 7 years ago

This tests the Field instances for Float, Double and BigDecimal in a bit of a sneaky way. Rather than test them directly, it uses a FPApprox wrapper type. This exercises the underlying Field to handle the regular operations, but also maintains an error bound on the approximation. The Eq instance for FPApprox[A] will return true for any 2 numbers that could be equal, given their error bounds. Of course, there are more subtleties than that:

The numbers occasionally underflow, which causes all sorts of annoying problems. So, we also have to occasionally check if values after multiplications are near-zero and, if so, bump them back up to a very small, but useable number.

The numbers occasionally overflow to infinity, at which point the game is lost, since we soon start getting NaNs. So, we also return true if a comparison gets us into non-finite number territory.

tixxit commented 7 years ago

For whatever reason, the fromDouble test is failing for FPApprox[Float] and FPApprox[Double] on JS only. The values I'm getting make no sense and it seems suspiciously like a bug in BigInt. Need to investigate further...

codecov-io commented 7 years ago

Current coverage is 73.99% (diff: 92.30%)

Merging #192 into master will increase coverage by 3.36%

@@             master       #192   diff @@
==========================================
  Files            36         36          
  Lines           766        769     +3   
  Methods         713        708     -5   
  Messages          0          0          
  Branches         53         61     +8   
==========================================
+ Hits            541        569    +28   
+ Misses          225        200    -25   
  Partials          0          0          

Powered by Codecov. Last update baa32ef...22a3743

tixxit commented 7 years ago

So, this is because the fromDouble test uses BigDecimal, which is apparently broken on scalajs. I'm going to skip the tests if Platform.isJVM is false, which should be OK for now. If BigDecimal gets unbroken, we can re-enable.

erik-stripe commented 7 years ago

👍