Channels
#mathematics
Title
# mathematics
z
Zhelenskiy
05/12/2021, 8:51 AM@altavir Let the discussion about the core change be here. I suggested inventing fast power method for Rings.
Here is the current implementation:
Copy code
public fun <T> Ring<T>.pow(base: T, exponent: ULong): T = when {
this == zero && exponent > 0UL -> zero
this == one -> base
this == -one -> powWithoutOptimization(base, exponent % 2UL)
else -> powWithoutOptimization(base, exponent)
}
private fun <T> Ring<T>.powWithoutOptimization(base: T, exponent: ULong): T = when (exponent) {
0UL -> one
1UL -> base
else -> {
val pre = powWithoutOptimization(base, exponent shr 1).let { it * it }
if (exponent and 1UL == 0UL) pre else pre * base
}
} Copy code
public fun pow(other: ULong): BigInt = BigIntField { pow(this@BigInt, other) }a
altavir
05/12/2021, 9:38 AMThe problem with methods in core algebras is that it will be mandatory for all descendants. It makes kind of sense, still it requires much more thinking. For example, why ULong and not UInt
z
Zhelenskiy
05/12/2021, 9:44 AMI know and agree about necessity of further discussion of the core changes.
As I explained in Github, it is useless for BigInts (because of overflow) but may be useful for other Rings.
However, it may be better to create also
Copy code
fun <T> Ring<T>.pow(base: T, exponent: UInt) = pow(base, exponent.toULong())a
altavir
05/12/2021, 7:04 PMThe problem is that onece we add something to the core, it will be really hard to remove it. An extension function could be marked experimental and removed without harm later.
z
Zhelenskiy
05/12/2021, 10:01 PMOk, I will add that. Are there any arguments against the current implementation (being extension function, having pow with ulong and with uint that calls ulong version, having function in BigInt that calls Ring version of pow, etc)?
And which annotation do you suggest?
kotlin.Experimentalpowa
altavir
05/13/2021, 5:54 AMUnstableKMathAPI
z
Zhelenskiy
05/13/2021, 10:30 AMI added it and published the changes
Do I need to do anything else there?
a
altavir
05/13/2021, 10:42 AMI've re-assigned the review so we will do it as soon as all accept changes.
z
Zhelenskiy
05/13/2021, 10:43 AMThank you! 😀
a
altavir
05/13/2021, 4:59 PMOK, looks good, but I found out that we already have those extensions in a separate file, so you should just replace implementations in them instead of creating new functions.
Also be sure to resolve conflicts
z
Zhelenskiy
05/13/2021, 9:42 PMConflicts of names - of course. I have done everything.
a
altavir
05/14/2021, 6:13 AMMerged. There were some problems there still, but I've fixed them on merge
z
Zhelenskiy
05/14/2021, 7:01 AMThank you!
Well, I checked. You replaced Int -> UInt in Field. However,
Fielda
altavir
05/14/2021, 7:13 AMIt was a mistake, I've reverted it
there was a problem in extension, it took this instead of arg. I've fixed it. Also I've changed a long-running random test since it took too much time on native.
z
Zhelenskiy
05/14/2021, 7:18 AMI mean the following code
(second part)
a
altavir
05/14/2021, 7:19 AMYeah, it was my mistake, i've reverted it. See the current state in
devz
Zhelenskiy
05/14/2021, 7:20 AMAlso, where can I check the test that was reduced?
a
altavir
05/14/2021, 7:21 AMMore specifically here: https://github.com/mipt-npm/kmath/commit/dc9ec54644c4d7b9675bfd9959ee2e1f84829152
z
Zhelenskiy
05/14/2021, 7:27 AMThis one. Right?
a
altavir
05/14/2021, 7:28 AMyes. The library is already quite heavy so we can't allow long running tests. Maybe it is a good idea to do a separate module with optional tests later.
z
Zhelenskiy
05/14/2021, 7:29 AMOk. (Or it was possible to reduce repeat number to 20 - 50)
a
altavir
05/14/2021, 7:30 AMIt is not quite good idea to do fully random tests because it could cause error reproducability issues. It is better to add those tests to examples and run them manually.
z
Zhelenskiy
05/14/2021, 7:31 AMI expected that to be a stress test
a
altavir
05/14/2021, 7:33 AMIndeed. But stress tests should not be run with unit tests. Otherwise there will be problems with CI. You can add stress tests to examples module as stand-alone programs.
z
Zhelenskiy
05/14/2021, 7:35 AMPerhaps, it is worth adding a module with stress tests as there is one with benchmarks.
a
altavir
05/14/2021, 7:48 AMgood idea
z
Zhelenskiy
05/14/2021, 7:50 AMhttps://github.com/mipt-npm/kmath/pull/334 I removed todo
And I crated a feature request for the stress-tests: https://github.com/mipt-npm/kmath/issues/335
👍 1
3 Views