https://kotlinlang.org logo
#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
    }
}
I also invented function for the big integer to make it easier to access power:
Copy code
public fun pow(other: ULong): BigInt = BigIntField { pow(this@BigInt, other) }
a

altavir

05/12/2021, 9:38 AM
The 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 AM
I 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 PM
The 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 PM
Ok, 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.Experimental
is not applicable to target 'top level' function' which
pow
actually is.
a

altavir

05/13/2021, 5:54 AM
UnstableKMathAPI
z

Zhelenskiy

05/13/2021, 10:30 AM
I added it and published the changes
Do I need to do anything else there?
a

altavir

05/13/2021, 10:42 AM
I've re-assigned the review so we will do it as soon as all accept changes.
z

Zhelenskiy

05/13/2021, 10:43 AM
Thank you! 😀
a

altavir

05/13/2021, 4:59 PM
OK, 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 PM
Conflicts of names - of course. I have done everything.
a

altavir

05/14/2021, 6:13 AM
Merged. There were some problems there still, but I've fixed them on merge
z

Zhelenskiy

05/14/2021, 7:01 AM
Thank you!
Well, I checked. You replaced Int -> UInt in Field. However,
Field
can accept negative values. That was even in previous implementation. And there is a check that you still left in code. I haven't checked, but that is probably not compilable now.
a

altavir

05/14/2021, 7:13 AM
It 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 AM
I mean the following code
(second part)
a

altavir

05/14/2021, 7:19 AM
Yeah, it was my mistake, i've reverted it. See the current state in
dev
z

Zhelenskiy

05/14/2021, 7:20 AM
Also, where can I check the test that was reduced?
a

altavir

05/14/2021, 7:21 AM
z

Zhelenskiy

05/14/2021, 7:27 AM
This one. Right?
a

altavir

05/14/2021, 7:28 AM
yes. 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 AM
Ok. (Or it was possible to reduce repeat number to 20 - 50)
a

altavir

05/14/2021, 7:30 AM
It 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 AM
I expected that to be a stress test
a

altavir

05/14/2021, 7:33 AM
Indeed. 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 AM
Perhaps, it is worth adding a module with stress tests as there is one with benchmarks.
a

altavir

05/14/2021, 7:48 AM
good idea
z

Zhelenskiy

05/14/2021, 7:50 AM
And I crated a feature request for the stress-tests: https://github.com/mipt-npm/kmath/issues/335
👍 1
3 Views