Skip to content

Commit

Permalink
Make Field inherit from DivisionRing
Browse files Browse the repository at this point in the history
  • Loading branch information
armanbilge authored and pikinier20 committed Oct 25, 2021
1 parent 8ffb393 commit 4df5ad0
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 28 deletions.
10 changes: 9 additions & 1 deletion algebra-core/src/main/scala/algebra/ring/DivisionRing.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,15 @@ import scala.{specialized => sp}
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)
/**
* This is implemented in terms of basic Ring ops. However, this is
* probably significantly less efficient than can be done with a
* specific type. So, it is recommended that this method be
* overriden.
*
* This is possible because a Double is a rational number.
*/
def fromDouble(a: Double): A = DivisionRing.defaultFromDouble[A](a)(self, self)

}

Expand Down
18 changes: 8 additions & 10 deletions algebra-core/src/main/scala/algebra/ring/Field.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ package ring

import scala.{specialized => sp}

trait Field[@sp(Int, Long, Float, Double) A] extends Any with EuclideanRing[A] with MultiplicativeCommutativeGroup[A] {
trait Field[@sp(Int, Long, Float, Double) A]
extends Any
with EuclideanRing[A]
with DivisionRing[A]
with MultiplicativeCommutativeGroup[A] {
self =>

// default implementations for GCD
Expand All @@ -19,15 +23,9 @@ trait Field[@sp(Int, Long, Float, Double) A] extends Any with EuclideanRing[A] w
def emod(a: A, b: A): A = zero
override def equotmod(a: A, b: A): (A, A) = (div(a, b), zero)

/**
* This is implemented in terms of basic Field ops. However, this is
* probably significantly less efficient than can be done with a
* specific type. So, it is recommended that this method be
* overriden.
*
* This is possible because a Double is a rational number.
*/
def fromDouble(a: Double): A = Field.defaultFromDouble(a)(self, self)
// needed for bin-compat
override def fromDouble(a: Double): A =
DivisionRing.defaultFromDouble[A](a)(self, self)

}

Expand Down
41 changes: 24 additions & 17 deletions algebra-laws/shared/src/main/scala/algebra/laws/RingLaws.scala
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,29 @@ trait RingLaws[A] extends GroupLaws[A] { self =>
}
)

def divisionRing(implicit A: DivisionRing[A]) = new RingProperties(
name = "division ring",
al = additiveCommutativeGroup,
ml = multiplicativeGroup,
parents = Seq(ring),
"fromDouble" -> forAll { (n: Double) =>
if (Platform.isJvm) {
// TODO: BigDecimal(n) is busted in scalajs, so we skip this test.
val bd = new java.math.BigDecimal(n)
val unscaledValue = new BigInt(bd.unscaledValue)
val expected =
if (bd.scale > 0) {
A.div(A.fromBigInt(unscaledValue), A.fromBigInt(BigInt(10).pow(bd.scale)))
} else {
A.fromBigInt(unscaledValue * BigInt(10).pow(-bd.scale))
}
DivisionRing.fromDouble[A](n) ?== expected
} else {
Prop(true)
}
}
)

// boolean rings

def boolRng(implicit A: BoolRng[A]) = RingProperties.fromParent(
Expand Down Expand Up @@ -285,23 +308,7 @@ trait RingLaws[A] extends GroupLaws[A] { self =>
name = "field",
al = additiveCommutativeGroup,
ml = multiplicativeCommutativeGroup,
parents = Seq(euclideanRing),
"fromDouble" -> forAll { (n: Double) =>
if (Platform.isJvm) {
// TODO: BigDecimal(n) is busted in scalajs, so we skip this test.
val bd = new java.math.BigDecimal(n)
val unscaledValue = new BigInt(bd.unscaledValue)
val expected =
if (bd.scale > 0) {
A.div(A.fromBigInt(unscaledValue), A.fromBigInt(BigInt(10).pow(bd.scale)))
} else {
A.fromBigInt(unscaledValue * BigInt(10).pow(-bd.scale))
}
Field.fromDouble[A](n) ?== expected
} else {
Prop(true)
}
}
parents = Seq(euclideanRing, divisionRing)
)

// Approximate fields such a Float or Double, even through filtered using FPFilter, do not work well with
Expand Down

0 comments on commit 4df5ad0

Please sign in to comment.