Feature: Polynomials and rational functions #469

2022-03-16T18:36:06+03:00

lounres commented

2022-03-16 18:36:06 +03:00

(Migrated from github.com)

This is implementation of different kinds of polynomials and rational functions.
It should close issues #462, #336
See also: #39, #446,

Comment: files systematisation

Each kind of polynomials and corresponding polynomial spaces are placed in corresponding file named after type of polynomials.
The same goes for rational functions: each kind of rational functions and corresponding rational functions' spaces are placed in corresponding file named after type of polynomials.
Utilities for each kind of polynomials and rational functions are placed in file named after corresponding type of polynomials/rational functions with additional Util suffix.
Constructors (and building DSLs) for each kind of polynomials and rational functions are placed in file named after corresponding type with additional Constructors suffix.
Abstract entities are places in Polynomial.kt and RationalFunction.kt.

Problems and questions

Is it worth to change Map<Symbol, UInt> to android.util.ArrayMap<Symbo, UInt> or something similar?
This class is designed to be more memory efficient for small numbers of entries which is exactly our use case.
What is better way to implement interoperability with integers?
As fact, additive (abelian) groups has natural product operation with integers: product of element c and integer n is just:
- sum of n copies of c if n is positive,
- sum of -n copies of -c if n is negative (or negate of sum of -n copies of c, which is the same),
- zero (neutral element) of the group if n is zero.
The same goes for multiplicative groups and power. (As a matter of fact, rings do not provide multiplicative (abelian) groups, but multiplicative (abelian) monoids, so the power can be defined not for integers, but for non-negative integers.) In context of rings we also have addition of ring members and integers: for real, sum of element c and integer n is just c + n * one (where one is unit of the ring). And multiplication and exponentiation are very helpful and frequently used.

For example, when you calculate derivative of polynomial you multiply coefficients of terms to corresponding degrees (which are positive integers), so to implement derivative you have to implement multiplication yourself (see algebraicStub.kt) or require ability to convert integer to ring member (see Polynomial<C>.derivative that preserves former logic). Both variants are bad: manual multiplication usually slower then multiplication that could be provided by corresponding ring, and requiring convertibility (by ScaleOperations or even NumericAlgebra because there are no alternatives) makes it impossible to use it in common rings (say hi to rational numbers again).

In my opinion, the best way is to declare the interactions in GroupOps/Group (for multiplication) and Ring (for exponentiation that is already declared and addition) interfaces and implement them in all current algebraic structures. (It's very easy, even is a bit of boilerplate.) It's even inconsistent in some way that exponentiation is declared in Ring, but multiplication by Int/UInt (which is the same operation not in multiplicative group/monoid, but in additive group/monoid) is not.

So the trivial question here: what should I do here?
Maybe, it is better to move first argument ring of utility functions (substitute, derivative, etc.) to context (when context receivers will be available), isn't it?
Is there need in other functionality?
For example, string representation: it is already implemented in my other repositories, it remains only to copy it with minimal modification.
Should RFs spaces be implemented as field?
If yes, then what division operations should be implemented? (For example, what should c / c return? And when field over C is provided?) And how to implement current Number-oriented functions of Field?
Should cases for A (the type of provided ring of constants) being Field be considered as well?
I mean that function operator fun Polynomial<C>.div(other: C): Polynomial<C> makes sense only in context of of field over C. And it looks helpful. So should I implement something like this? (But of course, I'll be able to implement something like this when context receivers will be released. For now I can sketch it, comment it and add TODO mark to it.)

TODO list

Write optimized algorithms for substitution.
Anything else? (Look at inline TODOs.)

This is implementation of different kinds of polynomials and rational functions. It should close issues #462, #336 See also: #39, #446, ## Comment: files systematisation 1. Each kind of polynomials and corresponding polynomial spaces are placed in corresponding file named after type of polynomials. 2. The same goes for rational functions: each kind of rational functions and corresponding rational functions' spaces are placed in corresponding file named after type of polynomials. 3. Utilities for each kind of polynomials and rational functions are placed in file named after corresponding type of polynomials/rational functions with additional `Util` suffix. 4. Constructors (and building DSLs) for each kind of polynomials and rational functions are placed in file named after corresponding type with additional `Constructors` suffix. 5. Abstract entities are places in `Polynomial.kt` and `RationalFunction.kt`. ## Problems and questions 1. **Is it worth to change `Map<Symbol, UInt>` to [`android.util.ArrayMap<Symbo, UInt>`](https://developer.android.com/reference/kotlin/android/util/ArrayMap) or something similar?** This class is designed to be more memory efficient for small numbers of entries which is exactly our use case. 1. **What is better way to implement interoperability with integers?** As fact, additive (abelian) groups has natural product operation with integers: product of element `c` and integer `n` is just: - sum of `n` copies of `c` if `n` is positive, - sum of `-n` copies of `-c` if `n` is negative (or negate of sum of `-n` copies of `c`, which is the same), - zero (neutral element) of the group if `n` is zero. The same goes for multiplicative groups and power. (As a matter of fact, rings do not provide multiplicative (abelian) groups, but multiplicative (abelian) monoids, so the power can be defined not for integers, but for non-negative integers.) In context of rings we also have addition of ring members and integers: for real, sum of element `c` and integer `n` is just `c + n * one` (where `one` is unit of the ring). And multiplication and exponentiation are very helpful and frequently used. For example, when you calculate derivative of polynomial you multiply coefficients of terms to corresponding degrees (which are positive integers), so to implement derivative you have to implement multiplication yourself (see `algebraicStub.kt`) or require ability to convert integer to ring member (see `Polynomial<C>.derivative` that preserves former logic). Both variants are bad: manual multiplication usually slower then multiplication that could be provided by corresponding ring, and requiring convertibility (by `ScaleOperations` or even `NumericAlgebra` because there are no alternatives) makes it impossible to use it in common rings (say hi to rational numbers again). In my opinion, the best way is to declare the interactions in `GroupOps`/`Group` (for multiplication) and `Ring` (for exponentiation that is already declared and addition) interfaces and implement them in all current algebraic structures. (It's very easy, even is a bit of boilerplate.) It's even inconsistent in some way that exponentiation is declared in `Ring`, but multiplication by `Int`/`UInt` (which is the same operation not in multiplicative group/monoid, but in additive group/monoid) is not. So the trivial question here: what should I do here? 1. **Maybe, it is better to move first argument `ring` of utility functions (`substitute`, `derivative`, etc.) to context (when context receivers will be available), isn't it?** 1. **Is there need in other functionality?** For example, string representation: it is already implemented in my other repositories, it remains only to copy it with minimal modification. 1. **Should RFs spaces be implemented as field?** If yes, then what division operations should be implemented? (For example, what should `c / c` return? And when field over `C` is provided?) And how to implement current `Number`-oriented functions of `Field`? 1. **Should cases for `A` (the type of provided ring of constants) being `Field` be considered as well?** I mean that function `operator fun Polynomial<C>.div(other: C): Polynomial<C>` makes sense only in context of of field over `C`. And it looks helpful. So should I implement something like this? (But of course, I'll be able to implement something like this when context receivers will be released. For now I can sketch it, comment it and add TODO mark to it.) ## TODO list - [ ] Write optimized algorithms for substitution. - [ ] Anything else? (Look at inline TODOs.)

CommanderTvis (Migrated from github.com) requested changes 2022-03-16 18:52:17 +03:00

lounres (Migrated from github.com) reviewed 2022-03-16 23:21:17 +03:00

lounres (Migrated from github.com) reviewed 2022-03-16 23:52:31 +03:00

lounres (Migrated from github.com) reviewed 2022-03-17 00:04:49 +03:00

lounres (Migrated from github.com) reviewed 2022-03-17 00:54:31 +03:00

CommanderTvis (Migrated from github.com) reviewed 2022-03-17 22:17:07 +03:00

lounres (Migrated from github.com) reviewed 2022-03-17 23:35:57 +03:00

CommanderTvis (Migrated from github.com) reviewed 2022-03-18 13:56:18 +03:00

lounres commented

2022-06-18 02:08:39 +03:00

(Migrated from github.com)

So now the main feature is finally ready. It only remains to add tests and write optimized algorithms for substitutions (with tests and benchmarks). (Maybe I should move algorithm optimization to another pull request and/or another branch.)

I also have questions about some details which I formulated in "Problems and questions" section in the pull request description message.

So now the main feature is finally ready. It only remains to add tests and write optimized algorithms for substitutions (with tests and benchmarks). (Maybe I should move algorithm optimization to another pull request and/or another branch.) I also have questions about some details which I formulated in "Problems and questions" section in the pull request description message.

altavir commented

2022-06-18 16:43:59 +03:00

(Migrated from github.com)

This is a large PR, so review will take some time. In order for this to be accepted, you need to add some examples in the example directory making it obvious how to use it. Tests are also necessary. I think that we will require a design note (in docs directory) on all new feature PRs in order to make sure different features play well together. You can probably use your very thorough (👍 ) PR text for that.

This is a large PR, so review will take some time. In order for this to be accepted, you need to add some examples in the example directory making it obvious how to use it. Tests are also necessary. I think that we will require a design note (in `docs` directory) on all new feature PRs in order to make sure different features play well together. You can probably use your very thorough (👍 ) PR text for that.

lounres commented

2022-06-18 18:43:20 +03:00

(Migrated from github.com)

OK. I am going to be busy for now. So I think I'll write examples and design notes some time later (about 1 to 2 weeks later). And after that will deal with tests and rest stuff.

lounres commented

2022-06-26 12:37:53 +03:00

(Migrated from github.com)

I added examples and design notes. (Although, I am not sure if they came out good.) Also decided to move optimizations to new refactoring branch (because I found out that there a lot of algorithms to refactor).

Now I am going to gradually add tests...

I added examples and design notes. (Although, I am not sure if they came out good.) Also decided to move optimizations to new refactoring branch (because I found out that there a lot of algorithms to refactor). Now I am going to gradually add tests...

altavir (Migrated from github.com) reviewed 2022-07-08 13:09:53 +03:00

lounres (Migrated from github.com) reviewed 2022-07-08 15:35:19 +03:00

lounres (Migrated from github.com) reviewed 2022-07-08 22:34:20 +03:00

lounres (Migrated from github.com) reviewed 2022-07-08 22:47:28 +03:00

lounres (Migrated from github.com) reviewed 2022-07-08 23:04:27 +03:00

lounres (Migrated from github.com) reviewed 2022-07-08 23:18:21 +03:00

altavir (Migrated from github.com) requested changes 2022-07-16 10:46:55 +03:00

altavir (Migrated from github.com) left a comment

Please review necessity of test modules. Otherwise, we will merge the module and then refine the API post-merge

lounres (Migrated from github.com) reviewed 2022-07-16 21:24:50 +03:00

lounres commented

2022-07-16 22:22:51 +03:00

(Migrated from github.com)

Done.

altavir (Migrated from github.com) approved these changes 2022-07-19 14:44:19 +03:00

altavir commented

2022-07-19 14:45:03 +03:00

(Migrated from github.com)

@CommanderTvis waiting for your review

pklimai (Migrated from github.com) requested changes 2022-07-19 23:38:49 +03:00

pklimai (Migrated from github.com) left a comment

My first comment would be - it's a huge module without README.md in the directory...

pklimai (Migrated from github.com) approved these changes 2022-07-21 13:15:32 +03:00

altavir commented

2022-07-21 17:34:34 +03:00

(Migrated from github.com)

Seems to be failing on Native. @lounres, have you done the full build locally?

altavir commented

2022-07-21 17:35:20 +03:00

(Migrated from github.com)

Maybe better to remove native plugin from polynomials for now. We can always add it later.

lounres commented

2022-07-21 20:11:08 +03:00

(Migrated from github.com)

@altavir Just cleanly rebuilt it (:clean + :build as-is and with kover disabled). Everything has been built successfully (and without any problems nor warnings in kmath-functions and kmath-polynomial). You can see logs here. It's Windows 10 x64.

By the way, a lot of on-push GitHub builds in this PR failed. Most of them failed due to bug in kover (seems like this issue) and some of them failed due to disappearance of daemon.

@altavir Just cleanly rebuilt it (`:clean` + `:build` as-is and with kover disabled). Everything has been built successfully (and without any problems nor warnings in `kmath-functions` and `kmath-polynomial`). You can see logs [here](https://gist.github.com/lounres/d84596524a1b10c5e887b50326065e9c). It's Windows 10 x64. By the way, a lot of on-push GitHub builds in this PR failed. Most of them failed due to bug in kover (seems like [this issue](https://github.com/Kotlin/kotlinx-kover/issues/183)) and some of them failed due to disappearance of daemon.

altavir commented

2022-07-23 11:36:04 +03:00

(Migrated from github.com)

It seems like it requires to much resources to run in the github container. I will try to fix it post-merge. Waiting for @CommanderTvis review now.

CommanderTvis (Migrated from github.com) requested changes 2022-07-27 10:56:21 +03:00

CommanderTvis (Migrated from github.com) left a comment

The first problem is that Polynomial interface is duplicated in two modules:
kmath-functions: https://github.com/lounres/kmath/blob/feature/polynomials/kmath-functions/src/commonMain/kotlin/space/kscience/kmath/functions/Polynomial.kt#L22
kmath-polynomial: https://github.com/lounres/kmath/blob/feature/polynomials/kmath-polynomial/src/commonMain/kotlin/space/kscience/kmath/functions/Polynomial.kt#L17

The first problem is that `Polynomial` interface is duplicated in two modules: kmath-functions: https://github.com/lounres/kmath/blob/feature/polynomials/kmath-functions/src/commonMain/kotlin/space/kscience/kmath/functions/Polynomial.kt#L22 kmath-polynomial: https://github.com/lounres/kmath/blob/feature/polynomials/kmath-polynomial/src/commonMain/kotlin/space/kscience/kmath/functions/Polynomial.kt#L17

CommanderTvis commented

2022-07-27 10:59:39 +03:00

(Migrated from github.com)

Another doubt is about the type parameter in interface Polynomial in kmath-polynomial. It can be just removed without any changes in semantics.

Another doubt is about the type parameter in `interface Polynomial` in kmath-polynomial. It can be just removed without any changes in semantics.

CommanderTvis commented

2022-07-27 11:02:15 +03:00

(Migrated from github.com)

Also, about type parameters, many classes in this PR can have out or in declaration site variances e.g., ListPolynomial<out C>.

Also, about type parameters, many classes in this PR can have `out` or `in` declaration site variances e.g., ListPolynomial<***out*** C>.

CommanderTvis commented

2022-07-27 11:11:44 +03:00

(Migrated from github.com)

Some implementations of operators in the contexts can be shortened:

    public override operator fun C.plus(other: Int): C = ring { addMultipliedByDoubling(this@plus, one, other) }
    public override operator fun C.minus(other: Int): C = ring { addMultipliedByDoubling(this@minus, one, -other) }


    public override operator fun C.plus(other: Int): C = ring { addMultipliedByDoubling(this@plus, one, other) }
    public override operator fun C.minus(other: Int): C =  this@minus + (-other)

Some implementations of operators in the contexts can be shortened: ```kotlin public override operator fun C.plus(other: Int): C = ring { addMultipliedByDoubling(this@plus, one, other) } public override operator fun C.minus(other: Int): C = ring { addMultipliedByDoubling(this@minus, one, -other) } ``` ```kotlin public override operator fun C.plus(other: Int): C = ring { addMultipliedByDoubling(this@plus, one, other) } public override operator fun C.minus(other: Int): C = this@minus + (-other) ```

lounres commented

2022-07-27 11:52:10 +03:00

(Migrated from github.com)

Another doubt is about the type parameter in interface Polynomial in kmath-polynomial. It can be just removed without any changes in semantics.

OK. Will be removed.

> Another doubt is about the type parameter in `interface Polynomial` in kmath-polynomial. It can be just removed without any changes in semantics. OK. Will be removed.

👍 1

lounres commented

2022-07-27 11:57:37 +03:00

(Migrated from github.com)

Also, about type parameters, many classes in this PR can have out or in declaration site variances e.g., ListPolynomial<out C>.

If "many" means "just ListPolynomial, NumberedPolynomial, LabeledPolynomial", then I agree. Because I don't see any other possible variance cases. This three will be provided with variance.

> Also, about type parameters, many classes in this PR can have `out` or `in` declaration site variances e.g., ListPolynomial<_**out**_ C>. If "many" means "just `ListPolynomial`, `NumberedPolynomial`, `LabeledPolynomial`", then I agree. Because I don't see any other possible variance cases. This three will be provided with variance.

👍 1

lounres commented

2022-07-27 12:00:42 +03:00

(Migrated from github.com)

The first problem is that Polynomial interface is duplicated in two modules: kmath-functions: https://github.com/lounres/kmath/blob/feature/polynomials/kmath-functions/src/commonMain/kotlin/space/kscience/kmath/functions/Polynomial.kt#L22 kmath-polynomial: https://github.com/lounres/kmath/blob/feature/polynomials/kmath-polynomial/src/commonMain/kotlin/space/kscience/kmath/functions/Polynomial.kt#L17

Watching discussion in telegram. Waiting for a final verdict.

> The first problem is that `Polynomial` interface is duplicated in two modules: kmath-functions: https://github.com/lounres/kmath/blob/feature/polynomials/kmath-functions/src/commonMain/kotlin/space/kscience/kmath/functions/Polynomial.kt#L22 kmath-polynomial: https://github.com/lounres/kmath/blob/feature/polynomials/kmath-polynomial/src/commonMain/kotlin/space/kscience/kmath/functions/Polynomial.kt#L17 Watching discussion in telegram. Waiting for a final verdict.

👀 1

lounres commented

2022-07-27 12:18:16 +03:00

(Migrated from github.com)

Some implementations of operators in the contexts can be shortened:

    public override operator fun C.plus(other: Int): C = ring { addMultipliedByDoubling(this@plus, one, other) }
    public override operator fun C.minus(other: Int): C = ring { addMultipliedByDoubling(this@minus, one, -other) }

    public override operator fun C.plus(other: Int): C = ring { addMultipliedByDoubling(this@plus, one, other) }
    public override operator fun C.minus(other: Int): C =  this@minus + (-other)

Yes, it's true. Although, I don't like it in some way because I don't fully control the minus operation's logic but delegate it to another operation. (It was driven by principal that I should control number of the ring's used operations because there can be algebraic contexts with very slow operations, so a + (-b) would be twice slower than a - b. Fortunately, it's not the case in the example.)

But, yeah, I'll fix it.

> Some implementations of operators in the contexts can be shortened: > > ```kotlin > public override operator fun C.plus(other: Int): C = ring { addMultipliedByDoubling(this@plus, one, other) } > public override operator fun C.minus(other: Int): C = ring { addMultipliedByDoubling(this@minus, one, -other) } > ``` > > ```kotlin > public override operator fun C.plus(other: Int): C = ring { addMultipliedByDoubling(this@plus, one, other) } > public override operator fun C.minus(other: Int): C = this@minus + (-other) > ``` Yes, it's true. Although, I don't like it in some way because I don't fully control the `minus` operation's logic but delegate it to another operation. (It was driven by principal that I should control number of the ring's used operations because there can be algebraic contexts with very slow operations, so `a + (-b)` would be twice slower than `a - b`. Fortunately, it's not the case in the example.) But, yeah, I'll fix it.

❤️ 1

CommanderTvis commented

2022-07-27 12:46:06 +03:00

(Migrated from github.com)

Also, about type parameters, many classes in this PR can have out or in declaration site variances e.g., ListPolynomial<out C>.

If "many" means "just ListPolynomial, NumberedPolynomial, LabeledPolynomial", then I agree. Because I don't see any other possible variance cases. This three will be provided with variance.

Yes, actually I meant them.

> > Also, about type parameters, many classes in this PR can have `out` or `in` declaration site variances e.g., ListPolynomial<_**out**_ C>. > > If "many" means "just `ListPolynomial`, `NumberedPolynomial`, `LabeledPolynomial`", then I agree. Because I don't see any other possible variance cases. This three will be provided with variance. Yes, actually I meant them.

lounres commented

2022-07-28 12:20:43 +03:00

(Migrated from github.com)

I removed Polynomial and fixed variance (and everything seems OK there). But I am currently struggling with code "shortening" (see neighbour branch for current progress). I suspect KT-31420. So it seems to me that some changes should be applied a bit later (when the issue will be fixed, which is planned on 1.8.0).

I removed `Polynomial` and fixed variance (and everything seems OK there). But I am currently struggling with code "shortening" (see [neighbour branch](https://github.com/lounres/kmath/tree/feature/polynomial-shortening) for current progress). I suspect [KT-31420](https://youtrack.jetbrains.com/issue/KT-31420). So it seems to me that some changes should be applied a bit later (when the issue will be fixed, which is planned on 1.8.0).

CommanderTvis (Migrated from github.com) approved these changes 2022-07-28 15:50:34 +03:00

Sign in to join this conversation.

No reviewers

altavir

pklimai

CommanderTvis