New method for derivative of constant ImmutableDensePolynomial

[iojea]

Currently derivative does not work properly on a constant ImmutablePolynomial.
In the example

p = ImmutablePolynomial(1.) # defines a constant polynomial
dp = derivative(p)

the returned dp is of type ImmutablePolynomial{Union{},:x,0}, which is unusable (cannot be shown nor evaluated).

This pull request fixes this issue by introducing a new method:

function derivative(p::ImmutableDensePolynomial{B,T,X,1}) where {B<:StandardBasis,T,X}
    ImmutableDensePolynomial{B,T,X,1}(zero(T))
end

This returns the zero ImmutablePolynomial, that works fine in all my tests: it evaluates to zero(T) for every input, it produces the zero ImmutablePolynomial (or Polynomial) when multiplied by any other polynomial and it integrates to itself.

I think that there maybe some other issue in the other variants of derivative. In particular, I fail to see the meaning of ImmutablePolynomial{T,X,N} when N=0, which is treated in a special derivative method but is also considered in the main one. As far as I understand the code, all references to the case N=0 should be removed, and the zero polynomial should be interpreted as having N=1, since it has one coefficient. I have not make any changes in this direction, however, just in case I am missing something.

[jverzani]

Thanks for the contribution. Let me see. I think your observations are good ones.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.65%. Comparing base (607acca) to head (7ee9cdc).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #615      +/-   ##
==========================================
+ Coverage   76.64%   76.65%   +0.01%     
==========================================
  Files          38       38              
  Lines        4068     4070       +2     
==========================================
+ Hits         3118     3120       +2     
  Misses        950      950              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[KlausC]

I am not happy with that solution, as I expect the derivative of a constant polynomial the the zero polynomial, and that would be more naturally of type ImmutablePolynomial{B,T,X,0}. where the parameters B,T,X are copied from the derived polynomial. The derivative should just return zero(p) for the N === 1 case.

[KlausC]

The test @test derivative(p) === zero(p) should succeed.

[iojea]

That is a very good point. I was not aware of the type of zero(p) for p an ImmutablePolynomial, but it makes sense that zero(p) has N=0 (even though it is not formed by an empty Tuple). This also explains the existing method for N=0. I changed the method as you suggested, so it now returns zero(p). I also added
N==0 && return zero(p)
to the main derivative method.

[jverzani]

It seems like rather than add a case, we can remove the N==0 case which is captured in the special case above.

[KlausC]

I think, both cases (N == 0 and N == 1) are covered by the more specific functions in lines 145 and 146, so could be both removed.

[jverzani]

Agreed

[jverzani]

Thanks @KalusC for your points. There seems to be one change we can take in the other direction.

[KlausC]

As I mentioned before: there should be test cases. At least one, which fails before the fix and succeeds after the fix.

[jverzani]

Thanks for responding @KlausC , I added tests, but not to test the issue with display. I just added that.

[jverzani]

Apologies @iojea I thought I was pushing commits to your PR, but somehow opened another one #616 You can cherry pick and put in here (615), or we can merge 616. Just let me know. I think this is ready to go.

[jverzani]

I'll merge yours, then mine. A bit easier that way.

[iojea]

Thanks! That was my very first PR :).

[jverzani]

Thanks for taking the time to do it. It was very much appreciated.