Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

work on polynomial_composition. Close #520 #524

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jverzani
Copy link
Member

Start on some special cases for polynomial composition that improve inferrability.

@codecov
Copy link

codecov bot commented Aug 17, 2023

Codecov Report

Patch coverage has no change and project coverage change: -1.42% ⚠️

Comparison is base (6f5e5bb) 76.59% compared to head (cd1cc01) 75.18%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #524      +/-   ##
==========================================
- Coverage   76.59%   75.18%   -1.42%     
==========================================
  Files          35       34       -1     
  Lines        3739     3534     -205     
==========================================
- Hits         2864     2657     -207     
- Misses        875      877       +2     
Files Changed Coverage Δ
...polynomials/standard-basis/immutable-polynomial.jl 84.78% <0.00%> (-12.90%) ⬇️

... and 26 files with indirect coverage changes

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

@@ -106,8 +106,18 @@ function polynomial_composition(p::ImmutableDensePolynomial{B,T,X,N}, q::Immutab
cs = evalpoly(q, p.coeffs)
convert(P, cs)
end
function polynomial_composition(p::AbstractUnivariatePolynomial{B,T,X}, q::ImmutableDensePolynomial{B,S,Y,0}) where {B<:StandardBasis,T,S,X,Y}
P = ImmutableDensePolynomial{B,promote_type(T,S), Y,0}
zero(P)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A polynomial composed with zero is the polynomial's constant term, not always zero. For example, if we have f(x) = 1 and g(x) = 0, f∘g is 1, not 0.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from that, I don't think the promotion is necessary, that is, I think you could just ignore S, because the value of q is always zero, independently of S.

@nsajko
Copy link
Contributor

nsajko commented Aug 20, 2023

Could you also add fallback methods for polynomial_composition that would work by simply converting the ImmutablePolynomial to Polynomial?

Something like:

polynomial_composition(p::Polynomial, r::ImmutablePolynomial) = p(Polynomial(r))

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants