spire icon indicating copy to clipboard operation
spire copied to clipboard

Eq[Polynomial[C]] throws ArrayIndexOutOfBoundsException for negative exponents

Open LukaJCB opened this issue 6 years ago • 7 comments

Reproduce:

scala> def x: Polynomial[Int] = PolySparse(List(Term(-1642198702, -1)))
x: spire.math.Polynomial[Int]
scala> x === x
java.lang.ArrayIndexOutOfBoundsException: -1
  at scala.runtime.ScalaRunTime$.array_update(ScalaRunTime.scala:76)
  at spire.math.poly.PolySparse.coeffsArray(PolySparse.scala:54)
  at spire.math.PolynomialEq.eqv(Polynomial.scala:556)
  at spire.math.PolynomialEq.eqv$(Polynomial.scala:555)
  at spire.math.PolynomialInstances0$$anon$14.eqv(Polynomial.scala:568)
  ... 36 elided

LukaJCB avatar Jan 09 '19 13:01 LukaJCB

I'm not sure polynomials were designed to accept negative exponents. For example, the parse method on the companion object does not support negative exponents (see the termRe regexp).

@tixxit: can you comment on this?

denisrosset avatar Jan 11 '19 00:01 denisrosset

They were not - I think it is probably a bug that we don't throw an exception during construction in this case and we should fix that bug.

tixxit avatar Jan 16 '19 14:01 tixxit

Then we must also fix this line right here, which generates random exponents that could all be negative :) https://github.com/non/spire/blob/master/laws/src/main/scala/spire/laws/gen.scala#L100

LukaJCB avatar Jan 16 '19 14:01 LukaJCB

😬

tixxit avatar Jan 16 '19 15:01 tixxit

Submitted a quick fix for that particular problem.

LukaJCB avatar Jan 16 '19 15:01 LukaJCB

If polynomials aren't supposed to have negative exponents, should Term have UInt for exponent instead of Int?

kschwarz1116 avatar May 19 '22 03:05 kschwarz1116

@kschwarz1116 that's a good question. I wonder if UInt interacts poorly with specialization, and so would be boxed, which is why Int is preferable for performance. But I'm not sure about that without looking at bytecode.

armanbilge avatar May 19 '22 04:05 armanbilge