Add support for ArbPoly #32

Joel-Dahne · 2020-08-12T09:49:02Z

So far I have just added one commit with

ArbPoly and arb_poly_struct types
Support for parsing arb_poly_t
Parsed methods from arb_poly.h

One of the main things to discuss is what interface to use for ArbPoly. There is Polynomials.jl but I think that might only work for polynomials with a parametrized type. We could maybe still take inspiration from it and possibly overload some of their methods for ArbPoly. Do you have any other ideas?

Another thing is that I want to add a dedicated ArbSeries type, basically an ArbPoly with a specified order. Something like

struct ArbSeries
    arb_poly::arb_poly_struct
    order::Int
    prec::Int

    ArbPoly(order::Int; prec::Integer = DEFAULT_PRECISION[]) = new(arb_poly_struct(), order, prec)
end

It would work identically to ArbPoly when it comes to interfacing directly with functions from Arb. The difference would be that we overload different methods for it. For example * would for ArbPoly use arb_poly_mul whereas for ArbSeries it would use arb_poly_mullow, for ArbPoly the length would depend on the highest non-zero term whereas for ArbSeries it would be fixed to the given degree. There are a number of differences like that. Here I'm also not sure what interface to use, we could maybe take inspiration from TaylorSeries.jl.

kalmarek · 2020-08-12T09:54:16Z

I'm pretty amazed by how fast we can bootstrap a new Arb type with almost all Arb functionality wrapped!

kalmarek · 2020-08-12T09:55:16Z

deps/build.jl

@@ -0,0 +1 @@
+


don't re-add build.jl ;)

saschatimme · 2020-08-12T10:23:17Z

src/arbcalls/arb_poly.jl

+arbcall"slong arb_poly_allocated_bytes(const arb_poly_t x)"
+
+### Basic manipulation
+arbcall"slong arb_poly_length(const arb_poly_t poly)"


This needs an import Base: length at the top

There will be a similar function in acb_poly.jl as well. I'm still thinking about how to handle this in a good way.

saschatimme · 2020-08-12T10:24:39Z

src/arbcalls/arb_poly.jl

+arbcall"void arb_poly_evaluate2(arb_t y, arb_t z, const arb_poly_t f, const arb_t x, slong prec)"
+arbcall"void _arb_poly_evaluate2_acb_horner(acb_t y, acb_t z, arb_srcptr f, slong len, const acb_t x, slong prec)"
+arbcall"void arb_poly_evaluate2_acb_horner(acb_t y, acb_t z, const arb_poly_t f, const acb_t x, slong prec)"
+arbcall"void _arb_poly_evaluate2_acb_rectangular(acb_t y, acb_t z, arb_srcptr f, slong len, const acb_t x, slong prec)"


Here we should probably change the code gen to produce a function which also accepts an ArbVector for the two arguments arb_srcptr f, slong len

My plan here was to later implement methods which are not in-place for which we add more conveniences, like automatically picking up the length from AcbVector and such. It would be great if we could change the code-gen to automatically make length a keyword argument which takes the length of AcbVector as default value, but I'm worried this would be hard to do. The length parameter is sometimes called len, sometimes flen or glen, so it's hard to automatically identify it.

Makes sense, but note that I added some code exactly doing this

It would be great if we could change the code-gen to automatically make length a keyword argument which takes the length of AcbVector as default value

in #27 for the vector types 🙃

Adds - ArbPoly and arb_poly_struct types - Support for parsing arb_poly_t - Parsed methods from arb_poly.h

This includes - Constructors - show method - predicates - basic indexing

Joel-Dahne · 2020-09-02T09:44:48Z

Now I have finally taken the time to finish this pull request. I have added support for ArbPoly, ArbSeries, AcbPoly and AcbSeries. So far the implemented methods are quite basic and there is a lot of convenience methods to add later on, but I think this is a good start. I have added tests for everything I have added and also cleaned up some of the other tests a little bit.

Later on I might do some rearranging of code, moving things from poly.jl to series.jl, but that can wait for now.

Joel-Dahne · 2020-09-06T09:04:13Z

Now I have merged this with the updated arbcall. The merge went very smooth, I really like the changes you did to arbcall. The code in the pull request is mostly self contained so it should not be to dangerous to merge it. I have added tests for all new functionality as well as some old functionality. Once this is merged I'll look into parsing more of the Arb header files.

One thing I noted is that now that the length of the vectors become key-word arguments we have some clashes. Specifically void _arb_vec_add(arb_ptr C, arb_srcptr A, arb_srcptr B, slong n, slong prec) and void _arb_poly_add(arb_ptr C, arb_srcptr A, slong lenA, arb_srcptr B, slong lenB, slong prec) parses to the same method. My solution now was to comment out the latter. If you do want to add them as polynomials you can always do that by using the length of the shorter one, so I don't see it as a problem for now.

saschatimme · 2020-09-07T09:28:40Z

src/poly.jl

+
+Base.@propagate_inbounds function Base.setindex!(
+    poly::Union{ArbPoly,ArbSeries},
+    x::Arb,


Suggested change

x::Arb,

x::ArbLike,

saschatimme · 2020-09-07T09:28:50Z

src/poly.jl

+
+Base.@propagate_inbounds function Base.setindex!(
+    poly::Union{AcbPoly,AcbSeries},
+    x::Acb,


Suggested change

x::Acb,

x::AcbLike,

saschatimme

Looks good! Some comments are nitpicky, but I think we should definitely widen the setindex! methods and add a ref version for getindex.

saschatimme · 2020-09-07T09:29:31Z

src/poly.jl

+end
+
+# Constructors
+for (TPoly, T) in [(:ArbPoly, :Arb), (:AcbPoly, :Acb)]


Suggested change

for (TPoly, T) in [(:ArbPoly, :Arb), (:AcbPoly, :Acb)]

for (TPoly, T) in [(:ArbPoly, :ArbLike), (:AcbPoly, :AcbLike)]

saschatimme · 2020-09-07T09:29:50Z

src/poly.jl

+    end
+
+    @eval function $TPoly(
+        coeffs::AbstractVector{$T};


Suggested change

coeffs::AbstractVector{$T};

coeffs::AbstractVector{<:$T};

saschatimme · 2020-09-07T09:30:26Z

src/poly.jl

+    end
+end
+
+for (TSeries, T) in [(:ArbSeries, :Arb), (:AcbSeries, :Acb)]


Prob. the same changes as for the poly case above

saschatimme · 2020-09-07T09:32:00Z

src/poly.jl

+    res = eltype(T)(prec = precision(poly))
+    get_coeff!(res, poly, i)
+    return res
+end


I think should also add a ref(poly:T, i) method which returns an ArbRef/AcbRef

This was my initial plan, but it's slightly complicated. Arb allows getting the value of any index i >= 0, for i higher than any set index it will just give the value 0. This means that while poly[i] will be defined for any i >= 0 it might not correspond to an actual arb_struct in memory.

In practice my experience is that you don't change polynomials element-wise very often, or at least it's definitely not where most of the computational cost lies, so it's not such a big problem.

Ok that's a fair point :)

saschatimme · 2020-09-07T09:33:56Z

src/poly.jl

+function Base.one(::Type{T}) where {T<:Union{ArbPoly,AcbPoly}}
+    res = T()
+    one!(res)
+    return res
+end


Suggested change

function Base.one(::Type{T}) where {T<:Union{ArbPoly,AcbPoly}}

res = T()

one!(res)

return res

end

Base.one(::Type{T}) where {T<:Union{ArbPoly,AcbPoly}} = one!(T())

We can do these way shorter by now :)

Joel-Dahne · 2020-09-07T19:22:38Z

I'll wait for #62 to be merged with master and then start updating things!

Joel-Dahne · 2020-09-08T19:59:25Z

I merged it with the updates to master and took care of your comments! The only thing I have not taken care of is adding a ref method, but as I explained this is not really applicable.

kalmarek reviewed Aug 12, 2020

View reviewed changes

deps/build.jl Outdated

@@ -0,0 +1 @@

Copy link

Owner

kalmarek Aug 12, 2020

Choose a reason for hiding this comment

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

don't re-add build.jl ;)

Joel-Dahne reacted with thumbs up emoji

saschatimme reviewed Aug 12, 2020

View reviewed changes

Joel-Dahne force-pushed the poly branch from da50e0b to ba7373a Compare August 20, 2020 10:20

Joel-Dahne force-pushed the poly branch from ba7373a to 3b29fd0 Compare August 28, 2020 14:34

Joel-Dahne added 21 commits September 2, 2020 11:21

Add basic support for ArbPoly

f931a63

Adds - ArbPoly and arb_poly_struct types - Support for parsing arb_poly_t - Parsed methods from arb_poly.h

Formatting

b23367d

Add ArbSeries type

b81f228

Add basic functionality for ArbPoly and ArbSeries

ff6e301

This includes - Constructors - show method - predicates - basic indexing

Remove shallow constructors for ArbPoly and ArbSeries

9ff86c0

Fix precision methods

8aafca6

Fix formatting

ce4b153

Move ArbPoly and ArbSeries constructors to poly.jl

0152be2

Add more predicates for ArbPoly and ArbSeries

34a85d7

Fix arbcall not working for ArbSeries

f2f8aaa

ArbPoly, ArbSeries: Fix indexing being off by one

c106319

ArbPoly,ArbSeries: Fix zero and one

53370e1

ArbPoly,ArbSeries: Add != method

40939e4

Add tests for ArbPoly

aa0336e

Rewrite ArbPoly tests

4cac1aa

Add tests for ArbSeries and fix some issues

08712c6

Add types and arbcalls for AcbPoly and AcbSeries

e691e84

Add interface, constructors and predicates for AcbPoly and AcbSeries

8e26c3a

Add setprecision methods for AcbPoly and AcbSeries

8598de5

Fix precision for Poly and Series types and add tests

16bb329

Add show tests for Polynomials and Series and fix some issues

d337eb6

Joel-Dahne force-pushed the poly branch from eb2bfde to d337eb6 Compare September 2, 2020 09:22

Joel-Dahne added 2 commits September 2, 2020 11:37

Fix printing of zero polynomial and series

83916dc

Add another test for series

2f82bac

Joel-Dahne marked this pull request as ready for review September 2, 2020 09:41

saschatimme mentioned this pull request Sep 4, 2020

Make a first release #60

Closed

6 tasks

Joel-Dahne mentioned this pull request Sep 4, 2020

Arbcall refactor #57

Merged

Merge branch 'master' into poly

5d9e1b3

Joel-Dahne requested review from kalmarek and saschatimme September 6, 2020 08:58

saschatimme reviewed Sep 7, 2020

View reviewed changes

saschatimme requested changes Sep 7, 2020

View reviewed changes

Joel-Dahne mentioned this pull request Sep 7, 2020

use ispredicate(::Arbfunction) to return Bool from arbcall #59

Merged

Joel-Dahne added 2 commits September 8, 2020 19:55

Merge branch 'master' into poly

c538353

Poly: Allow more general type for setindex! and add some more tests

e931219

saschatimme approved these changes Sep 8, 2020

View reviewed changes

saschatimme merged commit 7a57f32 into master Sep 8, 2020

kalmarek deleted the poly branch July 21, 2022 15:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for ArbPoly #32

Add support for ArbPoly #32

Joel-Dahne commented Aug 12, 2020

kalmarek commented Aug 12, 2020

kalmarek Aug 12, 2020

saschatimme Aug 12, 2020

Joel-Dahne Aug 12, 2020

saschatimme Aug 12, 2020

Joel-Dahne Aug 12, 2020

saschatimme Aug 13, 2020 •

edited

Loading

Joel-Dahne commented Sep 2, 2020

Joel-Dahne commented Sep 6, 2020

saschatimme Sep 7, 2020

saschatimme Sep 7, 2020

saschatimme left a comment

saschatimme Sep 7, 2020

saschatimme Sep 7, 2020

saschatimme Sep 7, 2020

saschatimme Sep 7, 2020

Joel-Dahne Sep 7, 2020

saschatimme Sep 8, 2020

saschatimme Sep 7, 2020

Joel-Dahne commented Sep 7, 2020

Joel-Dahne commented Sep 8, 2020

	for (TPoly, T) in [(:ArbPoly, :Arb), (:AcbPoly, :Acb)]
	for (TPoly, T) in [(:ArbPoly, :ArbLike), (:AcbPoly, :AcbLike)]

Add support for ArbPoly #32

Add support for ArbPoly #32

Conversation

Joel-Dahne commented Aug 12, 2020

kalmarek commented Aug 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saschatimme Aug 13, 2020 • edited Loading

Choose a reason for hiding this comment

Joel-Dahne commented Sep 2, 2020

Joel-Dahne commented Sep 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saschatimme left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Joel-Dahne commented Sep 7, 2020

Joel-Dahne commented Sep 8, 2020

saschatimme Aug 13, 2020 •

edited

Loading