Skip to content
New issue

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

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

Already on GitHub? Sign in 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))

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

Successfully merging this pull request may close these issues.

2 participants