Either drop cooperative equality or write better tests for it

[non]

So @rklaehn discovered that Rational(1, 2) == 0L (despite Rational(1, 2) != 0).

I'm pretty sure this is a bug in Scala's cooperative equality, but obviously we don't have tests in place to catch this. We need to either have better tests and coverage for this feature, or we should drop cooperative equality.

[non]

Make sure to audit the following:

• Algebraic
• Complex
• FixedPoint
• Jet
• Natural
• Number
• Quaternion
• Rational
• Real
• SafeLong
• UByte
• UShort
• UInt
• ULong

[kevinmeredith]

I've written ScalaCheck tests at work for a DSL, as well using QuickCheck for verifying Functor laws - https://github.com/kevinmeredith/brentyorgey_upenn_class_hw/blob/master/Typeclassopedia/EvilFunctor.hs.

I'd be interested in writing up tests. Could you please tell me where to read more about cooperative equality? I googled for it, but found nothing.

[non]

I'm not sure there is a single article, but cooperative equality refers to the universal equals and hashCode methods that all Java objects (and thus all Scala objects) support.

def equals(other: Any): Boolean
def hashCode: Int

(If you are coming from Haskell this is obviously a somewhat uncomfortable and unfortunate situation, but it's the one we have.)

The general thing that Scala tries to achieve with cooperative equality is that you can compare different number types and they will compare equal (and hash to the same value) in most cases (e.g. if both values represent the same signed 32-bit integer).

The task here is pretty big since naïvely it means testing the cartesian product of all the types listed (plus Scala's built-in types, e.g. Int or BigInt). But I feel like we can diagonalize our testing, at least in terms of priority. Also, I think it's OK to punt on "hard" comparisons and just return false.

The things we have to ensure:

• Commutativity
• Transitivity
• Consistency between hashCode and equals
• Consistency between conversions and equals
• ...and so on.

Does this make sense?

[kevinmeredith]

I believe that I understand what you're looking for.

Here's my first commit for adding commutativity tests - kevinmeredith@86c654a.

sbt output:

[info] Compiling 1 Scala source to C:\Users\k\Workspace\spire\tests\target\scala-2.11\test-classes...
[info] + Algebraic.commutative +: OK, passed 100 tests.
[info] + Algebraic.commutative *: OK, passed 100 tests.

I'm expecting to write a set of properties, against which all of your above types may be tested.

My plan is to finish adding tests for Algebraic. Then, figure out how to write a method that takes a type, i.e. Algebraic, Complex, etc., and then either (1) returns a set of Properties or (2) returns Unit, checking all of the properties.

Please let me know if I'm on the right path or not. Thanks

[non]

That sounds good -- one suggestion would be to mention equality in the test name, since we also expect addition, multiplication, etc. to be commutative.

[non]

Haha nevermind, I was misreading the output, you are one step ahead of me.

I think this is the right approach. Thanks!

[kevinmeredith]

Thanks for this and continued guidance!

I got help on ScalaCheck's mailing list about how to handle multiple implicits - directly use a Gen[T] - https://groups.google.com/forum/#!topic/scalacheck/YWZGn0Wrtws.

Then, I updated the Algebraic property-based tests:

https://github.com/kevinmeredith/spire/blob/07d9b32b2ee9b429cd466499520d94b69a89a7d3/tests/src/test/scala/spire/laws/CooperativeEquality.scala

For these tests, it showed output that indicates there's not cooperative equality with Double.

Note - I should annotate it, but each of the 3 tests that run are:

• commutative addition
• commutative multiplication
• transitivity

EDIT - replacing with latest output

[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] No tests to run for core/test:testOnly
Checking Algebraic's Commutativity for + and *, as well as Transitivity, for Int
+ OK, passed 100 tests.
+ OK, passed 100 tests.
+ OK, passed 100 tests.
Checking Algebraic's Commutativity for + and *, as well as Transitivity, for Long
+ OK, passed 100 tests.
+ OK, passed 100 tests.
+ OK, passed 100 tests.
Checking Algebraic's Commutativity for + and *, as well as Transitivity, for Double
! Falsified after 6 passed tests.
> ARG_0: ~1.44246860E-278
> ARG_1: ~-3.48686543E-307
! Falsified after 0 passed tests.
> ARG_0: ~4.20103387E-187
> ARG_1: ~-1.63397008E-241
+ OK, passed 100 tests.
Checking Algebraic's Commutativity for + and *, as well as Transitivity, for BigInt
+ OK, passed 100 tests.
! Falsified after 2 passed tests.
> ARG_0: -1
> ARG_1: ~-6.49037107E+32
+ OK, passed 100 tests.
Checking Algebraic's Commutativity for + and *, as well as Transitivity, for Rational
+ OK, passed 100 tests.
+ OK, passed 100 tests.
+ OK, passed 100 tests.

When running with BigDecimal, I halted the tests after waiting ~2 minutes. That's why it does not show up in the output above.

Please let me know if, in the above 3 tests, if I'm missing anything.

[kevinmeredith]

Issued #436.

For Algebraic and 1-2 other types, using arbitrary BigDecimal values (in property-based tests) did not complete after an hour, so I excluded BigDecimal for most tests.

This may not be acceptable, but documenting it.

[kevinmeredith]

Hmm, I'm confused why the Travis CI build failed:

[error] /home/travis/build/non/spire/tests/src/test/scala/spire/laws/cooperative_equality/AlgebraicCoopEqProp.scala:23: type mismatch;
[error]  found   : spire.math.Rational
[error]  required: spire.math.Algebraic
[error]   } yield Rational(num, den)
[error]                   ^
[error] one error found
[error] (tests/test:compileIncremental) Compilation failed

I git pull origin master into my working branch, deleted my ~/.ivy2 directory, and then re-built successfully with:

cd spire
sbt
project core
test:compile