typelevel/spire

`RealScalaCheckSuite.x.pow(k).nroot(k) = x` failed

Closed this issue · 3 comments

https://github.com/typelevel/spire/runs/4087917370?check_suite_focus=true#step:8:1039

==> X spire.math.RealScalaCheckSuite.x.pow(k).nroot(k) = x  0.049s munit.FailException: /home/runner/work/spire/spire/tests/shared/src/test/scala/spire/math/RealScalaCheckSuite.scala:124
123:    }
124:  }
125:

Failing seed: IhNYEXMVtFv8DyJdCDjhw3OngO0cs8m_RcJEQuIvjaG=
You can reproduce this failure by adding the following override to your suite:

  override val scalaCheckInitialSeed = "IhNYEXMVtFv8DyJdCDjhw3OngO0cs8m_RcJEQuIvjaG="

Falsified after 9 passed tests.
> ARG_0: -2759766405657757/1532495540865888858358347027150309183618739122183602176
> ARG_1: Sized(6)
    at munit.Assertions.fail(Assertions.scala:283)
    at munit.Assertions.fail$(Assertions.scala:15)
    at munit.FunSuite.fail(FunSuite.scala:11)
    at munit.ScalaCheckSuite.propToTry$$anonfun$1(ScalaCheckSuite.scala:104)
    at scala.util.Try$.apply(Try.scala:210)
    at munit.ScalaCheckSuite.propToTry(ScalaCheckSuite.scala:104)
    at munit.ScalaCheckSuite.$init$$$anonfun$2$$anonfun$1$$anonfun$1(ScalaCheckSuite.scala:46)
    at scala.concurrent.impl.Promise$Transformation.run(Promise.scala:439)
    at munit.Suite$$anon$1.execute(Suite.scala:26)
    at scala.concurrent.impl.Promise$Transformation.submitWithValue(Promise.scala:393)
    at scala.concurrent.impl.Promise$DefaultPromise.submitWithValue(Promise.scala:302)
    at scala.concurrent.impl.Promise$DefaultPromise.dispatchOrAddCallbacks(Promise.scala:276)
    at scala.concurrent.impl.Promise$DefaultPromise.transform(Promise.scala:128)
    at munit.internal.FutureCompat$ExtensionFuture.transformCompat(FutureCompat.scala:16)
    at munit.ScalaCheckSuite.$init$$$anonfun$3$$anonfun$2(ScalaCheckSuite.scala:48)
    at munit.GenericTest.withBodyMap$$anonfun$1(GenericTest.scala:33)
    at munit.MUnitRunner.$anonfun$8(MUnitRunner.scala:296)

It appears to be acting on Rational under the hood:

  test("rational val") {
    import spire.math.Fractional.RationalIsFractional
    val x =  Rational(2759766405657757L, SafeLong(BigInt("1532495540865888858358347027150309183618739122183602176")))

    assertEquals(x.pow(3).nroot(3), x) // Rational fails at 3, Real fails at 6
  }

This equality fails for sqrt too if we look large enough:

  test("rational val") {
    import spire.math.Fractional.RationalIsFractional
    val x =  Rational(2759766405657757L, SafeLong(BigInt(2).pow(1000)))
    assertEquals(x.pow(2).nroot(2), x) // fails
  }

nroot on Rational has a comment that it occurs at Double precision. This is clearly pushing the boundary. I think reasonable solutions include attempting to up the nroot precision (at cost of performance), or just updating the test to only look at rationalFromLongs.
Docs could be updated to be more accurate on this too.

Thanks for looking into this! It seems strange to make that NRoot instance available by default since it is only approximate. I wonder if it should be moved out of implicit scope so that it has to be imported explicitly, do you have any opinions?

Testing with the rationalFromLongs generator sounds good to me, as does updating the docs. PRs welcome! :)

It wouldn't be the first time someone suggested NRoot[Rational] looks suspicious. I'm for making it an explicit import.

I'll try getting around to a PR for these changes in the next couple days!