-
Notifications
You must be signed in to change notification settings - Fork 9
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.
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
Algebra in core #254
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
3d2bede
port Add to tl algebra
erikerlandson db7cf20
port Sub to tl algebra
erikerlandson 2296aff
port Mul to tl algebra
erikerlandson fe55800
Div with TruncatedDivision is failing
erikerlandson e78652a
/ and tquot
erikerlandson da94832
neg
erikerlandson 746798b
pow and tpow
erikerlandson 79993dd
remove algebra.scala
erikerlandson ec6d576
explicit truncating conversions
erikerlandson 540299e
populate some coulomb.ops.algebra instances
erikerlandson b30eae2
refactor use of algebras
erikerlandson a72a408
scala.math.Ordering -> cats.kernel.Order
erikerlandson 1ee822f
refactor si, si.prefixes, mks, mksa to use export
erikerlandson c39fc91
tweak directories
erikerlandson 15b3dd5
refactor algebra to use export
erikerlandson 3b56b9d
factor value-specific optimizations to separate import
erikerlandson 6d2c43c
unit test optimized operations
erikerlandson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
/* | ||
* Copyright 2022 Erik Erlandson | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package coulomb.ops.algebra | ||
|
||
import scala.annotation.implicitNotFound | ||
|
||
// there is no typelevel community typeclass that expresses the concept | ||
// "supports raising to fractional powers, without truncation" | ||
// The closest thing is spire NRoot, but it is also defined on truncating integer types, | ||
// so it is not helpful for distinguishing "pow" from "tpow", and in any case requires spire | ||
// https://github.com/typelevel/spire/issues/741 | ||
|
||
@implicitNotFound("Fractional power not defined for value type ${V}") | ||
abstract class FractionalPower[V]: | ||
/** returns v^e */ | ||
def pow(v: V, e: Double): V | ||
|
||
@implicitNotFound("Truncating power not defined for value type ${V}") | ||
abstract class TruncatingPower[V]: | ||
/** returns v^e, truncated to integer value (toward zero) */ | ||
def tpow(v: V, e: Double): V |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
/* | ||
* Copyright 2022 Erik Erlandson | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package coulomb.ops.algebra | ||
|
||
object all: | ||
export coulomb.ops.algebra.int.{*,given} | ||
export coulomb.ops.algebra.long.{*,given} | ||
export coulomb.ops.algebra.float.{*,given} | ||
export coulomb.ops.algebra.double.{*,given} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
/* | ||
* Copyright 2022 Erik Erlandson | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package coulomb.ops.algebra | ||
|
||
import algebra.ring.TruncatedDivision | ||
|
||
object double: | ||
given ctx_Double_is_FractionalPower: FractionalPower[Double] with | ||
def pow(v: Double, e: Double): Double = math.pow(v, e) | ||
|
||
given ctx_Double_is_TruncatingPower: TruncatingPower[Double] with | ||
def tpow(v: Double, e: Double): Double = math.pow(v, e).toLong.toDouble | ||
|
||
given ctx_Double_is_TruncatedDivision: TruncatedDivision[Double] with | ||
def tquot(x: Double, y: Double): Double = (x / y).toLong.toDouble | ||
// I don't care about these | ||
def tmod(x: Double, y: Double): Double = ??? | ||
def fquot(x: Double, y: Double): Double = ??? | ||
def fmod(x: Double, y: Double): Double = ??? | ||
def abs(a: Double): Double = ??? | ||
def additiveCommutativeMonoid: algebra.ring.AdditiveCommutativeMonoid[Double] = ??? | ||
def order: cats.kernel.Order[Double] = ??? | ||
def signum(a: Double): Int = ??? |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
/* | ||
* Copyright 2022 Erik Erlandson | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package coulomb.ops.algebra | ||
|
||
import algebra.ring.TruncatedDivision | ||
|
||
object float: | ||
given ctx_Float_is_FractionalPower: FractionalPower[Float] with | ||
def pow(v: Float, e: Double): Float = math.pow(v.toDouble, e).toFloat | ||
|
||
given ctx_Float_is_TruncatingPower: TruncatingPower[Float] with | ||
def tpow(v: Float, e: Double): Float = math.pow(v.toDouble, e).toLong.toFloat | ||
|
||
given ctx_Float_is_TruncatedDivision: TruncatedDivision[Float] with | ||
def tquot(x: Float, y: Float): Float = (x / y).toLong.toFloat | ||
// I don't care about these | ||
def tmod(x: Float, y: Float): Float = ??? | ||
def fquot(x: Float, y: Float): Float = ??? | ||
def fmod(x: Float, y: Float): Float = ??? | ||
def abs(a: Float): Float = ??? | ||
def additiveCommutativeMonoid: algebra.ring.AdditiveCommutativeMonoid[Float] = ??? | ||
def order: cats.kernel.Order[Float] = ??? | ||
def signum(a: Float): Int = ??? |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
/* | ||
* Copyright 2022 Erik Erlandson | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package coulomb.ops.algebra | ||
|
||
import algebra.ring.TruncatedDivision | ||
|
||
object int: | ||
given ctx_Int_is_TruncatingPower: TruncatingPower[Int] with | ||
def tpow(v: Int, e: Double): Int = math.pow(v.toDouble, e).toInt | ||
|
||
given ctx_Int_is_TruncatedDivision: TruncatedDivision[Int] with | ||
def tquot(x: Int, y: Int): Int = x / y | ||
// I don't care about these | ||
def tmod(x: Int, y: Int): Int = ??? | ||
def fquot(x: Int, y: Int): Int = ??? | ||
def fmod(x: Int, y: Int): Int = ??? | ||
def abs(a: Int): Int = ??? | ||
def additiveCommutativeMonoid: algebra.ring.AdditiveCommutativeMonoid[Int] = ??? | ||
def order: cats.kernel.Order[Int] = ??? | ||
def signum(a: Int): Int = ??? | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
/* | ||
* Copyright 2022 Erik Erlandson | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package coulomb.ops.algebra | ||
|
||
import algebra.ring.TruncatedDivision | ||
|
||
object long: | ||
given ctx_Long_is_TruncatingPower: TruncatingPower[Long] with | ||
def tpow(v: Long, e: Double): Long = math.pow(v.toDouble, e).toLong | ||
|
||
given ctx_Long_is_TruncatedDivision: TruncatedDivision[Long] with | ||
def tquot(x: Long, y: Long): Long = x / y | ||
// I don't care about these | ||
def tmod(x: Long, y: Long): Long = ??? | ||
def fquot(x: Long, y: Long): Long = ??? | ||
def fmod(x: Long, y: Long): Long = ??? | ||
def abs(a: Long): Long = ??? | ||
def additiveCommutativeMonoid: algebra.ring.AdditiveCommutativeMonoid[Long] = ??? | ||
def order: cats.kernel.Order[Long] = ??? | ||
def signum(a: Long): Int = ??? |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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 withnew ...
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 useinvokedynamic
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.
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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
andtransparent inline
are necessary. Situations where a typeclass involved dependent typing seem unambiguous - they needtransparent 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 tradeAt 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.There was a problem hiding this comment.
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.There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 ofinline