Add monic polynomial

[jmh530]

Monic is a specialization of Polynomial assuming the leading coefficient equals to 1. I want to add my own version of log1p (because phobos documentation lies about how it works) and the CEPHES implementation uses the equivalent of this.

The tester fails, but I believe it is unrelated. I tested it with a separate version identifier and tests pass.

[9il]

The Polynomial is a high-level stuff with RC arrays. For simple things like math functions, it is better to not to allocate memory at all.

[jmh530]

@9il Happy to make the required changes. I was following how Polynomial worked more than anything. The phobos version of poly doesn't make any allocations. It would be a breaking change if making mir's polynomial function work similarly (since it returns a Polynomial!F), but we could introduce a new poly function and then have Polynomial's opCall use that.

[jmh530]

@9il Changes in fada1ee

[9il]

but we could introduce a new poly function and then have Polynomial's opCall use that.

Looks good. In that case, you don't need monic because it will be compiled-time optimized from poly. It has to be.

[jmh530]

@9il Coefficients are parameters that can be provided at run-time. This means that the compiler-optimized version of poly has to be able to handle any value for the leading coefficient. monic should be able to avoid some calculations. Though it would take some testing to determine how much an impact that is.

[jmh530]

@9il Made some changes and added the poly function. The only challenge with respect to calling poly from Polynomial.opCall would be that I can't use a core.lifetime.move. So I left the current implementation for now.

[9il]

• overload for slice isn't needed, arrays are fine
• I don't get why move may be needed. someRCArray[] returns an array view, now need to move it.
• Why do we have to have monic? Why not to use poly instead?

[jmh530]

• overload for slice isn't needed, arrays are fine

This makes things a little more awkward when calling it with slices. If I remove those overloads, then I got a whole bunch of template errors when calling it with either poly(value, x) or poly(value, x[]). I have to do poly(value, x.array), which results in a GC allocation.

* I don't get why `move` may be needed. `someRCArray[]` returns an array view, now need to move it. 

move was only used in Slice version. I only included it because it was used in the original. I don't always know when to use move or not.

* Why do we have to have monic? Why not to use poly instead? 

The motivation was originally that a version of monic is used in Cephes' version of log1p, presumably because there are performance benefits (as I mentioned above, the run-time calls of the function can't optimize away the operations in poly that monic gets rid of). I can do some profiling though to check it out.

[9il]

The motivation was originally that a version of monic is used in Cephes' version of log1p, presumably because there are performance benefits (as I mentioned above, the run-time calls of the function can't optimize away the operations in poly that monic gets rid of). I can do some profiling though to check it out.

I am 99% sure it will be optimised. godbold.org could help.

This makes things a little more awkward when calling it with slices. If I remove those overloads, then I got a whole bunch of template errors when calling it with either poly(value, x) or poly(value, x[]). I have to do poly(value, x.array), which results in a GC allocation.

Do you mean in unit tests or your private code?

[jmh530]

I am 99% sure it will be optimised. godbold.org could help.

Is there any way to include dependencies with that?

Do you mean in unit tests or your private code?

I mean adding a unittest here that makes it a little clearer how it works.

[9il]

Is there any way to include dependencies with that?

Yes, it allows to inclusion mir libs. However, you don't need that to check if you do a little code update.

I mean adding a unittest here that makes it a little clearer how it works.

Just use arrays.

[jmh530]

Here's the godbolt: https://godbolt.org/z/cvvx48GeE

Looks like the polyImpl version uses some additional instructions.

I'm happy to just use arrays for my application of these functions. My point about lightConst UT examples was just so that I have the example of how it works somewhere so that I don't forget it. It's orthogonal to using arrays...

[9il]

Looks like the polyImpl version uses some additional instructions.

https://godbolt.org/z/Mj1q7rf1h

[jmh530]

@9il So it looks like if polyImpl is used within another function and the compiler knows about the parameters, then it will inline everything away. I suppose that is good enough for my case.

[jmh530]

@9il pushed new commit. Feel free to squash before merging.

[9il]

Please

• make a single type API with X = F = T two types like in the Polynomial structure
• unify the code in a single function
• unify code with the Polynomial structure

[jmh530]

@9il Addressed them in latest. First two are pretty simple. Last one you might check is how you want it (see: 3d761cc)

[9il]

scope const F

[jmh530]

@9il That change results in the error below. It seems as if F.init for const(double)[] is null. I changed it to ForeachType!F.init and it worked. Though I'm happy to make some other adjustment instead.

source\mir\polynomial.d(100,12): Error: incompatible types for `(null) * (0)`: `const(double)[]` and `int` source\mir\polynomial.d(44,37): Error: template instance `mir.polynomial.poly!0u.poly!(int, const(double)[])` error instantiating source\mir\math\func\hermite.d(73,7): instantiated from here: `opCall!int` source\mir\polynomial.d(100,12): Error: incompatible types for `(null) * (nan)`: `const(double)[]` and `double` dmd failed with exit code 1. 

[codecov-commenter]

Codecov Report

Base: 91.56% // Head: 91.59% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (f55bec8) compared to base (d20245f).
Patch coverage: 100.00% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@ Coverage Diff @@ ## master #453 +/- ## ========================================== + Coverage 91.56% 91.59% +0.03%  ========================================== Files 79 79 Lines 18739 18812 +73 ========================================== + Hits 17158 17231 +73  Misses 1581 1581 

Impacted Files	Coverage Δ
source/mir/polynomial.d	98.70% <100.00%> (+2.14%)	⬆️
source/mir/math/sum.d	96.31% <0.00%> (+0.15%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[jmh530]

@9il Well it looks like my motivation for adding this is a bit mute as I was able to prod the phobos team to fix log1p. dlang/phobos#8701