Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Algebra in core #254

Merged
merged 17 commits into from
Mar 12, 2022
Merged

Algebra in core #254

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
3d2bede
port Add to tl algebra
erikerlandson Mar 6, 2022
db7cf20
port Sub to tl algebra
erikerlandson Mar 6, 2022
2296aff
port Mul to tl algebra
erikerlandson Mar 6, 2022
fe55800
Div with TruncatedDivision is failing
erikerlandson Mar 6, 2022
e78652a
/ and tquot
erikerlandson Mar 7, 2022
da94832
neg
erikerlandson Mar 7, 2022
746798b
pow and tpow
erikerlandson Mar 7, 2022
79993dd
remove algebra.scala
erikerlandson Mar 7, 2022
ec6d576
explicit truncating conversions
erikerlandson Mar 7, 2022
540299e
populate some coulomb.ops.algebra instances
erikerlandson Mar 10, 2022
b30eae2
refactor use of algebras
erikerlandson Mar 10, 2022
a72a408
scala.math.Ordering -> cats.kernel.Order
erikerlandson Mar 10, 2022
1ee822f
refactor si, si.prefixes, mks, mksa to use export
erikerlandson Mar 11, 2022
c39fc91
tweak directories
erikerlandson Mar 11, 2022
15b3dd5
refactor algebra to use export
erikerlandson Mar 12, 2022
3b56b9d
factor value-specific optimizations to separate import
erikerlandson Mar 12, 2022
6d2c43c
unit test optimized operations
erikerlandson Mar 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ lazy val core = crossProject(JVMPlatform, JSPlatform/*, NativePlatform*/)
.in(file("core"))
.settings(name := "coulomb-core")
.settings(commonSettings :_*)
.settings(libraryDependencies += "org.typelevel" %%% "algebra" % "2.7.0")

lazy val units = crossProject(JVMPlatform, JSPlatform/*, NativePlatform*/)
.crossType(CrossType.Pure)
Expand Down
6 changes: 6 additions & 0 deletions core/src/main/scala/coulomb/conversion/conversion.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ import scala.annotation.implicitNotFound
@implicitNotFound("No value conversion in scope for value types {VF} => {VT}")
abstract class ValueConversion[VF, VT] extends (VF => VT)

@implicitNotFound("No truncating value conversion in scope for value types {VF} => {VT}")
abstract class TruncatingValueConversion[VF, VT] extends (VF => VT)

/** Convert a value of type V from implied units UF to UT */
@implicitNotFound("No unit conversion in scope for value type {V}, unit types {UF} => {UT}")
abstract class UnitConversion[V, UF, UT] extends (V => V)

@implicitNotFound("No truncating unit conversion in scope for value type {V}, unit types {UF} => {UT}")
abstract class TruncatingUnitConversion[V, UF, UT] extends (V => V)
20 changes: 9 additions & 11 deletions core/src/main/scala/coulomb/conversion/standard/standard.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package coulomb.conversion.standard

import coulomb.conversion.{ValueConversion, UnitConversion}
import coulomb.conversion.*
import coulomb.{coefficient, withUnit, Quantity}
import coulomb.policy.{AllowTruncation, AllowImplicitConversions}

Expand Down Expand Up @@ -50,16 +50,16 @@ inline given ctx_VC_Long[VF](using num: Integral[VF]): ValueConversion[VF, Long]
new ValueConversion[VF, Long]:
def apply(v: VF): Long = num.toLong(v)

inline given ctx_VC_Long_tr[VF](using num: Fractional[VF], at: AllowTruncation): ValueConversion[VF, Long] =
new ValueConversion[VF, Long]:
inline given ctx_TVC_Long[VF](using num: Fractional[VF]): TruncatingValueConversion[VF, Long] =
new TruncatingValueConversion[VF, Long]:
def apply(v: VF): Long = num.toLong(v)
Comment on lines +53 to 55
Copy link
Contributor

@armanbilge armanbilge Mar 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bikeshed for another issue/PR, but I'm concerned that inline combined with new ... is sub-optimal and can lead to a proliferation of anonymous classes in the jar. It depends on whether the compiler will automatically recognize this is a SAM and use invokedynamic instead. It might be better to implement this as a lambda e.g. v => num.toLong(v) to encourage this optimization.

I'm less familiar with Scala.js internals but this could be especially bad for JavaScript actually.

See typelevel/cats#3871 for a discussion of this topic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it's not obvious to me why inline is needed here?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not assume I'm using these only when they are needed 😁

I'm still working out my intuitions about when inline and transparent inline are necessary. Situations where a typeclass involved dependent typing seem unambiguous - they need transparent inline. Others are less obvious (to me).

Some places I am definitely using inline for efficiency gains, but other places it may be a bad trade

At any rate, if inline is problematic, I'm happy to clean all that up where it is not needed. Having some decent unit test coverage should make this a bit easier now. If something introduces a compile error, it ought to show up pretty fast.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I would use inline where needed only for its compile-time semantics.

For runtime performance, both the JVM and the Scala.js linker backend do a fantastic job of inlining themselves.

We can always revisit inline opportunities in the future with a benchmark suite.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have had bad experiences with macro generated classes in the past. When defining boopickle generated serialized it was very easy to endup generating thousands of instances that would bloat the resulting js size. It wasn't really noticeable on the jvm thus it was quite hard to spot

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall I expect scala-3 coulomb to be better in this regard. I am using metaprogramming in multiple places where I was using intermediate typeclasses, and so these intermediate typeclasses should no longer appear in byte code. However, some of this may be offset by use of transparent inline, so it will definitely be helpful to remove any unimportant usages of inline


inline given ctx_VC_Int[VF](using num: Integral[VF]): ValueConversion[VF, Int] =
new ValueConversion[VF, Int]:
def apply(v: VF): Int = num.toInt(v)

inline given ctx_VC_Int_tr[VF](using num: Fractional[VF], at: AllowTruncation): ValueConversion[VF, Int] =
new ValueConversion[VF, Int]:
inline given ctx_TVC_Int_tr[VF](using num: Fractional[VF]): TruncatingValueConversion[VF, Int] =
new TruncatingValueConversion[VF, Int]:
def apply(v: VF): Int = num.toInt(v)

// unit conversions that discard fractional values can be imported from
Expand All @@ -76,19 +76,17 @@ inline given ctx_UC_Float[UF, UT]:
new UnitConversion[Float, UF, UT]:
def apply(v: Float): Float = c * v

inline given ctx_UC_Long[UF, UT](using AllowTruncation):
UnitConversion[Long, UF, UT] =
inline given ctx_TUC_Long[UF, UT]: TruncatingUnitConversion[Long, UF, UT] =
val nc = coulomb.conversion.infra.coefficientNumDouble[UF, UT]
val dc = coulomb.conversion.infra.coefficientDenDouble[UF, UT]
// using nc and dc is more efficient than using Rational directly in the conversion function
// but still gives us 53 bits of integer precision for exact rational arithmetic, and also
// graceful loss of precision if nc*v exceeds 53 bits
new UnitConversion[Long, UF, UT]:
new TruncatingUnitConversion[Long, UF, UT]:
def apply(v: Long): Long = ((nc * v) / dc).toLong

inline given ctx_UC_Int[UF, UT](using AllowTruncation):
UnitConversion[Int, UF, UT] =
inline given ctx_TUC_Int[UF, UT]: TruncatingUnitConversion[Int, UF, UT] =
val nc = coulomb.conversion.infra.coefficientNumDouble[UF, UT]
val dc = coulomb.conversion.infra.coefficientDenDouble[UF, UT]
new UnitConversion[Int, UF, UT]:
new TruncatingUnitConversion[Int, UF, UT]:
def apply(v: Int): Int = ((nc * v) / dc).toInt
18 changes: 18 additions & 0 deletions core/src/main/scala/coulomb/infra/meta.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,24 @@ object meta:
report.error(s"type expr ${typestr(TypeRepr.of[E])} is not a non-negative Int")
'{ new coulomb.rational.typeexpr.NonNegInt[E] { val value = 0 } }

def teToPosInt[E](using Quotes, Type[E]): Expr[coulomb.rational.typeexpr.PosInt[E]] =
import quotes.reflect.*
val rationalTE(v) = TypeRepr.of[E]
if ((v.d == 1) && (v.n > 0) && (v.n.isValidInt)) then
'{ new coulomb.rational.typeexpr.PosInt[E] { val value = ${Expr(v.n.toInt)} } }
else
report.error(s"type expr ${typestr(TypeRepr.of[E])} is not a positive Int")
'{ new coulomb.rational.typeexpr.PosInt[E] { val value = 0 } }

def teToInt[E](using Quotes, Type[E]): Expr[coulomb.rational.typeexpr.AllInt[E]] =
import quotes.reflect.*
val rationalTE(v) = TypeRepr.of[E]
if ((v.d == 1) && (v.n.isValidInt)) then
'{ new coulomb.rational.typeexpr.AllInt[E] { val value = ${Expr(v.n.toInt)} } }
else
report.error(s"type expr ${typestr(TypeRepr.of[E])} is not an Int")
'{ new coulomb.rational.typeexpr.AllInt[E] { val value = 0 } }

object rationalTE:
def unapply(using Quotes)(tr: quotes.reflect.TypeRepr): Option[Rational] =
import quotes.reflect.*
Expand Down
18 changes: 15 additions & 3 deletions core/src/main/scala/coulomb/ops/ops.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ import scala.annotation.implicitNotFound

import coulomb.*

@implicitNotFound("Negation not defined in scope for Quantity[${V}, ${U}]")
abstract class Neg[V, U]:
def apply(v: V): V

@implicitNotFound("Addition not defined in scope for Quantity[${VL}, ${UL}] and Quantity[${VR}, ${UR}]")
abstract class Add[VL, UL, VR, UR]:
type VO
Expand All @@ -44,16 +48,24 @@ abstract class Div[VL, UL, VR, UR]:
type UO
def apply(vl: VL, vr: VR): VO

@implicitNotFound("Negation not defined in scope for Quantity[${V}, ${U}]")
abstract class Neg[V, U]:
def apply(v: V): V
@implicitNotFound("Truncating Division not defined in scope for Quantity[${VL}, ${UL}] and Quantity[${VR}, ${UR}]")
abstract class TQuot[VL, UL, VR, UR]:
type VO
type UO
def apply(vl: VL, vr: VR): VO

@implicitNotFound("Power not defined in scope for Quantity[${V}, ${U}] ^ ${P}")
abstract class Pow[V, U, P]:
type VO
type UO
def apply(v: V): VO

@implicitNotFound("Truncating Power not defined in scope for Quantity[${V}, ${U}] ^ ${P}")
abstract class TPow[V, U, P]:
type VO
type UO
def apply(v: V): VO

@implicitNotFound("Ordering not defined in scope for Quantity[${VL}, ${UL}] and Quantity[${VR}, ${UR}]")
abstract class Ord[VL, UL, VR, UR]:
def apply(vl: VL, vr: VR): Int
Expand Down
18 changes: 10 additions & 8 deletions core/src/main/scala/coulomb/ops/standard/add.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package coulomb.ops.standard

import scala.util.NotGiven

import algebra.ring.AdditiveSemigroup

import coulomb.ops.{Add, ValueResolution}
import coulomb.conversion.{ValueConversion, UnitConversion}
import coulomb.policy.AllowImplicitConversions
Expand Down Expand Up @@ -58,24 +60,24 @@ transparent inline given ctx_add_1V1U[VL, UL, VR, UR](using
// https://github.com/lampepfl/dotty/issues/14585
eqv: VR =:= VL,
equ: UR =:= UL,
alg: CanAdd[VL]
alg: AdditiveSemigroup[VL]
): Add[VL, UL, VR, UR] =
new Add[VL, UL, VR, UR]:
type VO = VL
type UO = UL
def apply(vl: VL, vr: VR): VL = alg.add(vl, eqv(vr))
def apply(vl: VL, vr: VR): VL = alg.plus(vl, eqv(vr))

transparent inline given ctx_add_1V2U[VL, UL, VR, UR](using
ice: AllowImplicitConversions,
eqv: VR =:= VL,
neu: NotGiven[UR =:= UL],
ucv: UnitConversion[VL, UR, UL],
alg: CanAdd[VL]
alg: AdditiveSemigroup[VL]
): Add[VL, UL, VR, UR] =
new Add[VL, UL, VR, UR]:
type VO = VL
type UO = UL
def apply(vl: VL, vr: VR): VL = alg.add(vl, ucv(eqv(vr)))
def apply(vl: VL, vr: VR): VL = alg.plus(vl, ucv(eqv(vr)))

transparent inline given ctx_add_2V1U[VL, UL, VR, UR](using
ice: AllowImplicitConversions,
Expand All @@ -84,12 +86,12 @@ transparent inline given ctx_add_2V1U[VL, UL, VR, UR](using
vres: ValueResolution[VL, VR],
vlvo: ValueConversion[VL, vres.VO],
vrvo: ValueConversion[VR, vres.VO],
alg: CanAdd[vres.VO]
alg: AdditiveSemigroup[vres.VO]
): Add[VL, UL, VR, UR] =
new Add[VL, UL, VR, UR]:
type VO = vres.VO
type UO = UL
def apply(vl: VL, vr: VR): VO = alg.add(vlvo(vl), vrvo(vr))
def apply(vl: VL, vr: VR): VO = alg.plus(vlvo(vl), vrvo(vr))

transparent inline given ctx_add_2V2U[VL, UL, VR, UR](using
ice: AllowImplicitConversions,
Expand All @@ -99,10 +101,10 @@ transparent inline given ctx_add_2V2U[VL, UL, VR, UR](using
vlvo: ValueConversion[VL, vres.VO],
vrvo: ValueConversion[VR, vres.VO],
ucvo: UnitConversion[vres.VO, UR, UL],
alg: CanAdd[vres.VO]
alg: AdditiveSemigroup[vres.VO]
): Add[VL, UL, VR, UR] =
new Add[VL, UL, VR, UR]:
type VO = vres.VO
type UO = UL
def apply(vl: VL, vr: VR): VO = alg.add(vlvo(vl), ucvo(vrvo(vr)))
def apply(vl: VL, vr: VR): VO = alg.plus(vlvo(vl), ucvo(vrvo(vr)))

104 changes: 0 additions & 104 deletions core/src/main/scala/coulomb/ops/standard/algebra.scala
View file
Open in desktop

This file was deleted.

10 changes: 6 additions & 4 deletions core/src/main/scala/coulomb/ops/standard/div.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,13 @@ package coulomb.ops.standard

import scala.util.NotGiven

import coulomb.{`*`, `/`, `^`}
import algebra.ring.MultiplicativeGroup

import coulomb.`/`
import coulomb.ops.{Div, SimplifiedUnit, ValueResolution}
import coulomb.conversion.{ValueConversion, UnitConversion}
import coulomb.policy.AllowImplicitConversions
import coulomb.policy.AllowTruncation

transparent inline given ctx_div_Double_2U[UL, UR](using su: SimplifiedUnit[UL / UR]):
Div[Double, UL, Double, UR] =
Expand All @@ -40,7 +43,7 @@ transparent inline given ctx_div_Float_2U[UL, UR](using su: SimplifiedUnit[UL /
transparent inline given ctx_div_1V2U[VL, UL, VR, UR](using
// https://github.com/lampepfl/dotty/issues/14585
eqv: VR =:= VL,
alg: CanDiv[VL],
alg: MultiplicativeGroup[VL],
su: SimplifiedUnit[UL / UR]
): Div[VL, UL, VR, UR] =
new Div[VL, UL, VR, UR]:
Expand All @@ -54,11 +57,10 @@ transparent inline given ctx_div_2V2U[VL, UL, VR, UR](using
vres: ValueResolution[VL, VR],
vlvo: ValueConversion[VL, vres.VO],
vrvo: ValueConversion[VR, vres.VO],
alg: CanDiv[vres.VO],
alg: MultiplicativeGroup[vres.VO],
su: SimplifiedUnit[UL / UR]
): Div[VL, UL, VR, UR] =
new Div[VL, UL, VR, UR]:
type VO = vres.VO
type UO = su.UO
def apply(vl: VL, vr: VR): VO = alg.div(vlvo(vl), vrvo(vr))

12 changes: 7 additions & 5 deletions core/src/main/scala/coulomb/ops/standard/mul.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ package coulomb.ops.standard

import scala.util.NotGiven

import coulomb.{`*`, `/`, `^`}
import algebra.ring.MultiplicativeSemigroup

import coulomb.`*`
import coulomb.ops.{Mul, SimplifiedUnit, ValueResolution}
import coulomb.conversion.{ValueConversion, UnitConversion}
import coulomb.policy.AllowImplicitConversions
Expand All @@ -40,25 +42,25 @@ transparent inline given ctx_mul_Float_2U[UL, UR](using su: SimplifiedUnit[UL *
transparent inline given ctx_mul_1V2U[VL, UL, VR, UR](using
// https://github.com/lampepfl/dotty/issues/14585
eqv: VR =:= VL,
alg: CanMul[VL],
alg: MultiplicativeSemigroup[VL],
su: SimplifiedUnit[UL * UR]
): Mul[VL, UL, VR, UR] =
new Mul[VL, UL, VR, UR]:
type VO = VL
type UO = su.UO
def apply(vl: VL, vr: VR): VL = alg.mul(vl, eqv(vr))
def apply(vl: VL, vr: VR): VL = alg.times(vl, eqv(vr))

transparent inline given ctx_mul_2V2U[VL, UL, VR, UR](using
nev: NotGiven[VL =:= VR],
ice: AllowImplicitConversions,
vres: ValueResolution[VL, VR],
vlvo: ValueConversion[VL, vres.VO],
vrvo: ValueConversion[VR, vres.VO],
alg: CanMul[vres.VO],
alg: MultiplicativeSemigroup[vres.VO],
su: SimplifiedUnit[UL * UR]
): Mul[VL, UL, VR, UR] =
new Mul[VL, UL, VR, UR]:
type VO = vres.VO
type UO = su.UO
def apply(vl: VL, vr: VR): VO = alg.mul(vlvo(vl), vrvo(vr))
def apply(vl: VL, vr: VR): VO = alg.times(vlvo(vl), vrvo(vr))

Loading