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

Polynomial performance improvements #142

Merged
merged 8 commits into from
Nov 25, 2021
Merged

Conversation

Joel-Dahne
Copy link
Collaborator

This adds a number of minor improvements to polynomials, mostly related to construction.

The main goal is to improve the performance when constructing ArbSeries with of the form ArbSeries([x, 1]), mostly by reducing the number of allocations. It makes the following changes related to this

  1. Allow passing tuples when constructing polynomials, saving one allocation in constructing the vector. So now ArbSeries((x, 1)) works. This change is breaking in the sense that previously passing a two-tuple (a, b) would set the first coefficient to Arb((a, b)), i.e. the interval from a to b. This could now be done with ArbSeries(((x, 1),)).
  2. Make setindex! work well with Int using arb_poly_set_coeff_si. Previously this would convert the input to Arb before setting the coefficient.
  3. Make a specialized constructor for two-tuples optimized for tuples with heterogeneous types. If you naively iterate over the elements in the tuple with a for loop the type inference doesn't work and you suffer in performance. By explicitly unfolding the loop type inference works well.
  4. I have also removed the fit_length! in one place in the construction for series, this is already done by the underlying constructor so doesn't have to be done again.

Before the above changes the fastest way to construct an ArbSeries with coefficients [x, 1] was with ArbSeries([x, one(x)]), now it is ArbSeries((x, 1)), comparing the performance we see that the later is much faster and (more importantly) allocates a lot less.

julia> x = Arb(1)
1.0000000000000000000000000000000000000000000000000000000000000000000000000000

julia> @btime ArbSeries([$x, one($x)])
  192.790 ns (4 allocations: 320 bytes)
1.0000000000000000000000000000000000000000000000000000000000000000000000000000 + 1.0000000000000000000000000000000000000000000000000000000000000000000000000000x + 𝒪(x^2)

julia> @btime ArbSeries(($x, 1))
  112.588 ns (2 allocations: 144 bytes)
1.0000000000000000000000000000000000000000000000000000000000000000000000000000 + 1.0000000000000000000000000000000000000000000000000000000000000000000000000000x + 𝒪(x^2)

In addition to the above I added @inbounds in one place where it was missing and better documentation for Arblib.ref for polynomials.

Since supporting constructing polynomials from tuples is a breaking change I have pushed the version to 0.6.0.

One of the benefits of this is that it allows construction of fixed
size polynomials without having to allocate a vector.

It does however break backwards compatability because previously
ArbPoly((1, 2)) would give the same polynomial as ArbPoly(Arb((1, 2)))
but now it instead gives ArbPoly([1, 2]).
This is a common case, in particular when computing Taylor series
since you then often want to construct series on the form
ArbSeries((x, 1)). This specialised constructor removes the drawback
of having a tuple with elements of different types. In the case
ArbSeries((x::Arb, 1)) it reduces the number of allocations from 5 to
2 compared to the generic implementation.
This is already taken care of by the underlying constructor so doesn't
have to be done again.
Copy link
Owner

@kalmarek kalmarek left a comment

Choose a reason for hiding this comment

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

This looks good.

Probably we should also support a vararg constructor as well, but this may wait for another time.

@Joel-Dahne
Copy link
Collaborator Author

I took the liberty of adding the bump ofSpecialFunctions.jl to this pull request from #143, it should not affect anything.

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