Adding GCD rings and Euclidean rings, along with instances for BigInt #246
Conversation
| if (EuclideanRing[A].isZero(b)) a else euclid(b, EuclideanRing[A].emod(a, b)) | ||
| } | ||
|
|
||
| /* @tailrec final def extendedEuclid[@sp(Int, Long, Float, Double) A: Eq: EuclideanRing](a: A, b: A): (A, A, A) = { |
There was a problem hiding this comment.
Sure. Or I'll implement the extended GCD algorithm.
| A.isZero(r) || (A.euclideanFunction(r) < A.euclideanFunction(y)) | ||
| } | ||
| }, | ||
| "submultiplicative function" -> forAll { (x: A, y: A) => |
There was a problem hiding this comment.
I didn't see this comment in trait. Could we add it so people get more intuition for the euclideanFunction?
| trait DivisionRing[@sp(Byte, Short, Int, Long, Float, Double) A] extends Any with Ring[A] with MultiplicativeGroup[A] { | ||
| self => | ||
|
|
||
| def fromDouble(a: Double): A = Field.defaultFromDouble[A](a)(self, self) |
There was a problem hiding this comment.
Is there any law on this? Seems odd to me a division ring only has this extra method. We don't have any other division like operation?
There was a problem hiding this comment.
Field has it too. I should have fromDouble in DivisionRing and make Field inherit DivisionRing.
Basically, if you have a type A with a function BigInt -> A and division, you have fromDouble.
(Should algebra incorporate DivisionRing, or is it too specialized? Note that DivisionRing covers the quaternions)
| * quot(x, y) = q | ||
| * mod(x, y) = r | ||
| */ | ||
| trait EuclideanRing[@sp(Int, Long, Float, Double) A] extends Any with GCDRing[A] { self => |
There was a problem hiding this comment.
If we have specialized on these types can we also add the implementations?
There was a problem hiding this comment.
... Float and Double are taken charge of by Field (euclidean division is trivial).
For Byte/Short/Int/Long, the instances are not lawful as overflow is undefined behaviour territory (which was not the case for just CommutativeRing, the laws pass).
We could:
- Provide no instances
- Provide instances, but not test them
- Port the
forAllSafemachinery from Spire that enables tests with abort-on-overflow
What do you think?
This aligns the commutative rings/field hierarchy with the one available in Spire.
I haven't provided instances for non-exact types such as Long, Int, etc, because their overflow behaviour doesn't work with the laws.
(Spire tests such instances by aborting the test in case of an overflow -- which is then considered undefined behaviour).