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

Improve evaluation of LaurentPolynomial #292

Merged
merged 1 commit into from
Nov 3, 2020

Conversation

martinholters
Copy link
Contributor

Use ntuple instead of NTuple for constructing the coefficient tuples passed to evalpoly. This compiles much faster on Julia 1.0 (especially for high-degree polynomials) and is also faster to evaluate even on current Julia.

Julia 1.0.5:

julia> @time LaurentPolynomial(ones(5))(1)
  0.253496 seconds (990.00 k allocations: 47.603 MiB, 4.10% gc time) # master
  0.861258 seconds (2.45 M allocations: 119.648 MiB, 4.13% gc time) # PR
5.0

julia> @btime LaurentPolynomial(ones(5))(1)
  608.573 ns (12 allocations: 656 bytes) # master
  114.681 ns (8 allocations: 416 bytes) # PR
5.0

julia> @time LaurentPolynomial(ones(150))(1)
 22.524473 seconds (77.11 M allocations: 3.157 GiB, 8.39% gc time) # master
  0.087917 seconds (187.70 k allocations: 9.934 MiB, 3.63% gc time) # PR
150.0

julia> @btime LaurentPolynomial(ones(150))(1)
  375.336 μs (11011 allocations: 265.31 KiB) # master
  6.042 μs (161 allocations: 7.72 KiB) # PR
150.0

Julia 1.6.0-DEV.1408:

julia> @time LaurentPolynomial(ones(5))(1)
  0.076117 seconds (137.39 k allocations: 7.787 MiB, 99.90% compilation time) # master
  0.023298 seconds (26.14 k allocations: 1.687 MiB, 99.86% compilation time) # PR
5.0

julia> @btime LaurentPolynomial(ones(5))(1)
  595.810 ns (10 allocations: 608 bytes) # master
  118.526 ns (6 allocations: 352 bytes) # PR
5.0

julia> @time LaurentPolynomial(ones(150))(1)
  0.089302 seconds (211.92 k allocations: 12.811 MiB, 99.88% compilation time) # master
  0.056672 seconds (170.88 k allocations: 10.229 MiB, 99.90% compilation time) # PR
150.0

julia> @btime LaurentPolynomial(ones(150))(1)
  8.637 μs (161 allocations: 9.00 KiB) # master
  5.936 μs (157 allocations: 7.59 KiB) # PR
150.0

The compile time on 1.0.5 grows super-linear, making it impossible to evaluate even higher-order polynomials without this PR.

Use `ntuple` instead of `NTuple` for constructing the coefficient tuples
passed to `evalpoly`. This compiles much faster on Julia 1.0 (especially
for high-degree polynomials) and is also faster to evaluate even on
current Julia.
@codecov
Copy link

codecov bot commented Nov 3, 2020

Codecov Report

Merging #292 into master will not change coverage.
The diff coverage is 75.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #292   +/-   ##
=======================================
  Coverage   88.66%   88.66%           
=======================================
  Files          16       16           
  Lines        1827     1827           
=======================================
  Hits         1620     1620           
  Misses        207      207           
Impacted Files Coverage Δ
src/polynomials/LaurentPolynomial.jl 83.94% <75.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f2c4a0e...11f975b. Read the comment docs.

@jverzani
Copy link
Member

jverzani commented Nov 3, 2020

Thanks! I hadn't realized this would be an issue.

@jverzani jverzani merged commit d72dd38 into JuliaMath:master Nov 3, 2020
@martinholters martinholters deleted the mh/eval-laurent branch November 3, 2020 16:42
@martinholters
Copy link
Contributor Author

Could you tag the new version?

@jverzani
Copy link
Member

jverzani commented Nov 3, 2020

Yes, just did. I didn't include the qr fix. Thanks for your help with this!

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