Abstracting Polynomials

[mileslucas]

This is a pretty big pull request with a lot of moving parts, I'll do my best to document and update my changes not only within the code but within this PR.

1. Create an AbstractPolynomial type and interface
Allows creation of an abstract interface which requires minor implementations for each type of polynomial.

• Define AbstractPolynomial type
  Note There are some ambiguities that occur when treating AbstractPolynomial <: Number, mainly in the arithmetic it is hard to define +(p::AbstractPolynomial, n::Number) because those types cause an ambiguous function error (related to Should Polynomials subtype Number ? #173, add toml files; v1.0 forward; make subtype of Number close #173 #174)
• Define inspection (length, size, etc.)
• Define conversions and promotions
  The existing promotion and conversion rules will stand, in addition the promote type for two AbstractPolynomial will be Polynomial, since every special polynomial can be (somewhat easily) converted to a standard polynomial.
• Define arithmetic operations
• Define indexing operations
• Define number-like interface zero, one, etc.
• Plotting
• Showing

2. The common interface
All polynomials should have the following common methods. This is very similar to the way Distributions.jl handles its abstract types

• fromroots(::Type{P}, ....) create polynomial of type P from roots. Defaults to Polynomial
  this calls _fromroots(::Type{P}, ...) in the concrete implementation
• fit(::Type{P}, x, y, ...) fit data using polynomial of type P. This relies on _vander(::Type{P}...) only.
• roots(p) return the roots of the polynomial. This relies on _companion(p) only.
• coeffs(p) wrapper for p.coeffs
• domain(p), domain(::Type{P}) return the domain of the indeterminate, This relies on _domain.
• vander(::Type{P}) just a wrapper for _vander
• companion(p) just a wrapper for _companion
• scale_to_domain(p, x) scale_to_domain(::Type{P}, x), return x scaled to the domain of the polynomial
• variable(p) variable(::Type{P}) get the indeterminate of the polynomial.
• derivative(p, k=1) return derivate of the polynomial
• integral(p, k=0) return integral of the polynomial
• `integrate(p, a, b)1 return definite integral of the polynomial from a to b
• truncate truncate! round off and chop leading zeros
• chop chop! chop leading zeros
• degree and order
• norm and conjugate
  This matches most of the previous implementation plus some extra. By defining fit as the least-squares solution of the vandermonde matrix _vander, we only have to implement the latter for each concrete type (same with roots and _companion).

3. Deprecate Old functionality

• Deprecate Poly -> Polynomial
• Deprecate polyX -> appropriate new implementation

4. New polynomials

• ChebyshevT
• ChebyshevU
• Legendre
• Hermite
• HermiteHe
• Laguerre
• Lagrange

5. Documentation
Document everything with doctests and plots :)

6. Tests
Make sure there are no breaking changes via deprecation tests and maintain an equal or greater coverage of functionality

[mileslucas]

At this point all of the existing implementation is re-written and properly deprecated.

[olof3]

@jverzani Something like Polynomial(roots=[1,2,3])) looks good.

For many (typically academic) applications of charpoly I guess that one would not like to go through eigvals (if it is a matrix of Ints or Syms) but compute the polynomial directly so that it has coefficients of type eltype(A). If this is done it is would be a reasonable function for that odd time that you'd like to check a linear algebra exercise or something similar.

Edit: Seems like SymPy has this functionality: A = Sym.(rand(1:9, 3, 3)); s = Sym("s"); A.charpoly(s), so perhaps not necessary to implement it in Polynomials.
It could possibly be helpful with some kind of pointer to SymPy and Polynomial(roots=eigvals(A))) if someone comes looking for this in Polynomials.

[jverzani]

I'm sorry @olof3, I think I misread your comments. You are talking about two things, right:

• instead of fromroots, using a keyword argument to the Polynomial constructor
• in addition, possibly adding a charpoly constructor. I don't think this is necessary, as my understanding is that it would mostly be for pedagogical uses, in which case maybe more is to be learned by calling eigvals directly.

[mileslucas]

Preface: I don't have any academic knowledge of advanced polynomials and my only previous usage has been with the numpy.polynomials package.

• The ChebyshevT type is based in this wikipedia entry and this numpy implementation. The exigence for creating this abstract implementation was to better accommodate having different types of polynomials (eg ChebyshevT are not defined in any other package as far as I could see when I looked a few months ago).

• Polynomial(;roots=) looks good semantically, but it will require a few extra lines to enable usage like Polynomial{Float64}(roots=Float32[1, 2, 3]).

• I have no problem removing the default definition for fit; my overall approach was to default everything to Polynomial.

• The default order for fit existed before I rewrote the code

• I originally chose integral over integrate because integral seems less surprising to return another AbstractPolynomial whereas integrate ought to return a scalar. Mathematica, does this, I don't know about other examples. derivative matches the grammar, no other reason.

other comments
Again, I wanted to try and create a framework that allows us to start to have more coverage of polynomial types like in numpy. This is important because if people are trying to switch from Python, they will regard a JuliaMath repo as reliable, but will be unsatisfied if a certain type is not available in Julia. I also feel like it is easier to keep these types within the single Polynomials.jl package, because the end-user interface is extremely similar for all of them and after initial implementation I don't see how there would be much need for independent and parallel development of different types.

[jverzani]

Im not sure about roots=[...] or using a flag, as in roots=true. The latter allows the order of values, symbol without using names. It might also open up specifying other basis by name. Thoughts? I’d leave the default fit. I think that’s a documentation fix, if it is needed. I prefer integrate (a verb) and only one name for both the definite and indefinite integral. Much better to use dispatch for the two cases. I also prefer diff which sounds like a verb. Matching SymPy is my preference. I also think how you handle the Poly case through the framework is much better than my suggestion to make Poly{T} an abstract super type. Have you had time to benchmark? If not, I’ll do some, but not until the new year.

On Sun, Dec 22, 2019 at 10:50 PM Miles Lucas ***@***.***> wrote: Preface: I don't have any academic knowledge of advanced polynomials and my only previous usage has been with the numpy.polynomials package. - The ChebyshevT type is based in this wikipedia entry <https://en.wikipedia.org/wiki/Chebyshev_polynomials#First_kind> and this numpy implementation <https://docs.scipy.org/doc/numpy/reference/routines.polynomials.chebyshev.html>. The exigence for creating this abstract implementation was to better accommodate having different types of polynomials (eg ChebyshevT are not defined in any other package as far as I could see when I looked a few months ago). - Polynomial(;roots=) looks good semantically, but it will require a few extra lines to enable usage like Polynomial{Float64}(roots=Float32[1, 2, 3]). - I have no problem removing the default definition for fit; my overall approach was to default everything to Polynomial. - The default order for fit existed before I rewrote the code - I originally chose integral over integrate because integral seems less surprising to return another AbstractPolynomial whereas integrate ought to return a scalar. Mathematica, does this, I don't know about other examples. derivative matches the grammar, no other reason. *other comments* Again, I wanted to try and create a framework that allows us to start to have more coverage of polynomial types like in numpy. This is important because if people are trying to switch from Python, they will regard a JuliaMath repo as reliable, but will be unsatisfied if a certain type is not available in Julia. I also feel like it is easier to keep these types within the single Polynomials.jl package, because the end-user interface is extremely similar for all of them and after initial implementation I don't see how there would be much need for independent and parallel development of different types. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#179?email_source=notifications&email_token=AADG6TESUJNZGCOLCTEFQJTQ2AYPTA5CNFSM4I45Z422YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHQEBLQ#issuecomment-568344750>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADG6TFKR7D2ABPSTGZEQFTQ2AYPTANCNFSM4I45Z42Q> .

-- John Verzani Department of Mathematics College of Staten Island, CUNY verzani@math.csi.cuny.edu

[mileslucas]

Hm, I don't really like Polynomial([1, 2, 3], roots=true). That feels like a pythonic approach to the problem as opposed to a julian approach (ie. dispatch within a single function using kwargs instead of multiple dispatch). We should be able to dispatch fine using Polynomial(; roots=[1, 2, 3]), since it has no positional arguments, we can make the roots kwarg required, and still be able to set a different varname. I feel like this is the least surprising in terms of inputs and outputs.

I'll go ahead and make changes to integrate and diff.

I haven't done any benchmarks. I can write some basic ones up. What do you think of these-

• Do we want to compare to numpy?
• Polynomial evaluation across large num of coeffs and large num of inputs
• Polynomial rootfinding
• Polynomial fitting

I can put these into a /bench folder with their own environment.

[mileslucas]

Also, in terms of breaking builds, I've made sure every single previous test still passes. Fortunately, this ensures we maintain full coverage in our testing. Unfortunately, that means we can only be as sure about not breaking as our unit tests are. I've also made sure that things like poly.A show a dep warning and recommend using coeffs(poly) or poly.coeffs, so the name changes should show deprecation warnings, too.

[mileslucas]

@jverzani Base exports diff, which gets the difference element-wise in an array. Should we duck this or find a different word?

E: I've left it at derivative now, my first attempt to duck caused a bunch of wacky test failures

[olof3]

@mileslucas Have you checked out https://github.com/JuliaApproximation/ and in particular https://github.com/JuliaApproximation/ApproxFunOrthogonalPolynomials.jl "This package implements orthogonal polynomial based spaces to ApproxFun, including Chebyshev, Jacobi, Laguerre, and Hermite.".

I also feel like it is easier to keep these types within the single Polynomials.jl package,

For packages that include Polynomials.jl I guess it is nicer with a lightweight Polynomials.jl package that only includes typical functionality for dealing with polynomials?

[mileslucas]

I don’t see in that package where I could construct a Chebyshev series directly, which is the exact use case that led me to write this PR. Edit: I see you can create it with Fun(Chebyshev(), coeffs).

This means the only value we’re adding is a simpler interface for directly manipulating series and fitting arbitrary data.

In terms of lightening the package, I truly don’t think having the orthogonal polynomials in here will bloat the package. Each implementation is going to around 300 lines of code or fewer, and the common interface makes it easier to keep things consistent.

At most, I’d consider making three repos: AbstractPolynomials.jl, Polynomials.jl, SpecialPolynomials.jl, which takes the current organizational structure but streamlines the Polynomials.jl package. The trade-off is 3x more organizational and logistical work by devs.

[mileslucas]

So if we decide to not include the special polynomials then 90% of this rewrite is moot, since there’s no sense in having an abstract type for only one concrete subtype.

I have no issues with scrapping this, so don’t feel like my sunk time should influence this decision. What do you both think

[olof3]

I'm not sure what most sensible approach is. I think it could be a good idea if some more people weighed in on this, but perhaps it would be useful to clarify exactly what the questions are.

Definitely a number of things that needed fixing, and great that someone got around to do it! I don't see why the Chebyshev expansions can't be in another package if they don't end up in Polynomials.jl.

[jverzani]

Let’s see how many special cases get done, and see if another package is warranted. It isn’t so hard to split off. As for diff, we use that in SymPy and having a base function is actually good, as we do t get name collisions. As for benchmarks, I was just thinking about comparing to the current release.

On Mon, Dec 23, 2019 at 4:24 PM olof3 ***@***.***> wrote: I'm not sure what most sensible approach is. I think it could be a good idea if some more people weighed in on this, but perhaps it would be useful to clarify exactly what the questions are. Definitely a number of things that needed fixing, and great that someone got around to do it! I don't see why the Chebyshev expansions can't be in another package if they don't end up in Polynomials.jl. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#179?email_source=notifications&email_token=AADG6TANUJAA2267PR4OI6DQ2EUCNA5CNFSM4I45Z422YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHR6XLY#issuecomment-568585135>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADG6TDRHKZHZ64LZE76YSTQ2EUCNANCNFSM4I45Z42Q> .

-- John Verzani Department of Mathematics College of Staten Island, CUNY verzani@math.csi.cuny.edu

[jverzani]

In #188 it is seen that this should be n-1, or we could use degree in place of length

[jverzani]

Hi @mileslucas . Sorry, I took a hiatus from this that took longer than I thought it would. But I would like to get your nice work merged. Here are some current thoughts. Hopefully you haven't moved on altogether from this work.

• In the long run, your idea of having three packages (AbstractPolynomials, Polynomials, and SpecialPolynomials) may be the proper separation, but until there is a wider collection of special polynomials, let's keep as one. Though maybe a new subdirectory would be useful for the special polynomials.

• I'd like to have a) merge this and tag a release(but then remove the deprecations, and a few other modifications) b) add deprecations and bump the version number c) release 1.0 with new names.

• I'd still like to have a better name for fromroots. I don't really like poly for this, but prefer no change than fromroots (poly is from MatLab). Python (numpy) appears to use a logical positional argument to indicate this, which I don't like. You had reservations about a keyword argument (which I now share -- as I don't like Polynomial(; roots=[...]) anymore. Maybe just a deprecation warning to use prod(variable() .- rts) or some such construct is better.

• I played around with this to see what adding a new polynomial basis would look like. The results are here: https://github.com/jverzani/Polynomials.jl/blob/bernstein/src/polynomials/Bernstein.jl
  In writing this, a number of methods default to this process: convert to Polynomial{T}, work there, convert back to Bernstein{T}. (For example p(x) and * do this.) Should this be a default, so that if there are no faster algorithms available, the user doesn't have to define it this way? Also, it seems the showterm method should pass an instance of AbstractPolynomial{T} not a subtype, as in the Bernstein example, access to N would be useful.

• The roots function has some issues arising from the truncate call. We need to remove this. For example, here we have roots(fromroots(1.0:12)) has issues; but eigvals(companion(fromroots(1.0:12))) does not.

• we should add the domain information into the plot recipe.

[mileslucas]

Hey, no worries, this has also been on the back-burrner for me.

I'm happy to make any changes you see fit to make this work better not only for you as a maintainer but for the community as a whole, too.

For the new polynomial basis, I think having the fallback option be convert to polynomial -> operate -> convert back is part of the benefit of abstracting these things.

Let's work off your list here and put down concrete steps to get this merged. I fear if we sit too long this will end up dying in PR purgatory. How would you like to organize those changes?

[jverzani]

Great. I have a few things going on: a branch that includes some changes against your PR and the start of a SpecialPolynomials.jl package. I'll push later today. Would love your feedback.

[jverzani]

I have a few repositories to look at:

• https://github.com/jverzani/Polynomials.jl/tree/miles-lucas-refactor-delta

which contains some suggested changes to this main pull request.

• https://github.com/jverzani/SpecialPolynomials.jl

which contains some implementations of special polynomials.

My game plan with this was the following:

Version 0.7

• merge in @mileslucas work

• make minor adjustments:

✓ hold off on deprecations
✓ adjust roots; doc note to alternatives
✓ drop sorting of roots output. Following issue #185, use a keyword to pass to eigvals
✓ fix variable definition (return monomial x in the polynomial basis)
✓ iteration; add basis method
✓ add round (to go with truncate, chop)
✓ remove order as synonym for length; use order term only for derivative
✓ For fit, make deg positional, not a keyword (like Matlab, as I don't like deg for degree as keyword.)
✓ change to iszero from == zero(T) in showterm instance
✓ Close issue #189 for p(A::Array) for Polynomial and Poly type. Not for ChebyshevT (as x ∉ domain(p) isn't satisfied
✓ adjust type calculation for derivative; adjust derivative(::ChebyshevT...) to use recursion for order > 1

--> Plots
✓ plot recipe respects domain if finite
✓ tighten range of plot recipe
✓ adjust tests for change of tightening

--> Chebyshev
✓ ChebyshevT: fix derivative if order > degree
✓ ChebyshevT: make copy, as derivative was modifying p
✓ ChebyshevT: make basis more explicit when printing

--> Docs
✓ docs to emphasize basis
✓ change doc order -> degree

--> run some benchmarks
✓ add Base.:+(p::Polynomial{T}, c::S) where {T,S<:Number} method
✓ add @inbounds for p*q; use one(x) in places
✓ fit: use vand \ y in place of pinv(vand) * y
✓ divrem: avoid R.(num[0:n]), as it is slower than typed comprehension
✓ slightly different format for companion

works with Unitful.jl?

X pass, needed changes to + and * are cumbersome

Version 0.8

• add deprecations? Make a compat module? I'm now leaning towards the latter.

Version 1.0

• make Compat module loadable by the user

[mileslucas]

This sounds great. Let me know how/where I should help! Perhaps a good way would be to merge this PR into a diff branch and make PRs off that and then have a final PR when that branch is finished?

[jverzani]

Why don't I make a PR against yours, and you can resolve. Once done, we can merge your branch. That will be 0.7. I'm still unclear about 0.8, but am heavily leaning towards putting the Poly part in a separate module along with the MATLAB names and exporting. That gives us the option to remove, deprecate, or keep (which I'm currently inclined to do for 1.0, but perhaps could be convinced otherwise).

[jverzani]

Thanks so much @mileslucas !!