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

Branch cuts of functions on ComplexBalls #24717

Closed
mezzarobba opened this issue Feb 12, 2018 · 38 comments
Closed

Branch cuts of functions on ComplexBalls #24717

mezzarobba opened this issue Feb 12, 2018 · 38 comments

Comments

@mezzarobba
Copy link
Member

Add support for the analytic flag required by the rigorous integration code in some methods of complex balls.

Depends on #24627
Depends on #24686

CC: @fredrik-johansson @videlec @cheuberg

Component: numerical

Author: Marc Mezzarobba

Branch/Commit: c827491

Reviewer: Vincent Delecroix

Issue created by migration from https://trac.sagemath.org/ticket/24717

@mezzarobba mezzarobba added this to the sage-8.2 milestone Feb 12, 2018
@mezzarobba
Copy link
Member Author

Commit: 39c78d1

@mezzarobba
Copy link
Member Author

Author: Marc Mezzarobba

@mezzarobba

This comment has been minimized.

@mezzarobba
Copy link
Member Author

comment:1

Let's hope I didn't get too many branch cuts wrong...


Last 10 new commits:

374a942#24686 robustify, clean up, document rigorous integration using arb
376811ddoc markup fixes
0902f14another doc fix
1d6ca9cadd a few more doctest examples
120bcb4#24686 minor fixes
16962e5real_arb: use arb 2.6+ comparison functions
ac4ba37complex_arb: use arb 2.6+ comparison functions
b55e3c2stop considering inexact balls equal to themselves
09f788cMerge branch 'arb_cmp' into cuts
39c78d1#24717 support the "analytic" flag in some methods of complex balls

@mezzarobba
Copy link
Member Author

Branch: u/mmezzarobba/acb_cuts

@mezzarobba mezzarobba changed the title Branch cuts of functions on ComplexBallFields Branch cuts of functions on ComplexBalls Feb 12, 2018
@fredrik-johansson
Copy link

comment:2

This looks great. I would just suggest adding a pow method with this flag as well, and perhaps documenting some examples for .integral() where these are used (examples with .integral() could also be used as doctests for the individual methods).

@mezzarobba
Copy link
Member Author

comment:3

Replying to @fredrik-johansson:

This looks great. I would just suggest adding a pow method with this flag as well,

Yes, and there are several other methods that could implement analytic=True--but I thought it would be better to have some easy ones now rather than all later. (Though, thinking of it, pow wouldn't be that hard to support.)

and perhaps documenting some examples for .integral() where these are used (examples with .integral() could also be used as doctests for the individual methods).

Either I don't understand what you mean, or there are already one or two such examples.

@fredrik-johansson
Copy link

comment:4

Replying to @mezzarobba:

Replying to @fredrik-johansson:

This looks great. I would just suggest adding a pow method with this flag as well,

Yes, and there are several other methods that could implement analytic=True--but I thought it would be better to have some easy ones now rather than all later. (Though, thinking of it, pow wouldn't be that hard to support.)

Indeed, but pow is perhaps the most important of them all...

and perhaps documenting some examples for .integral() where these are used (examples with .integral() could also be used as doctests for the individual methods).

Either I don't understand what you mean, or there are already one or two such examples.

Thanks, I missed that.

@mezzarobba
Copy link
Member Author

comment:5

Replying to @fredrik-johansson:

Indeed, but pow is perhaps the most important of them all...

Yes, but it is also a bit harder to implement due to the need to share code with the __pow__, which participates in the coercion system... but ok, I'll try to see what I can do.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 12, 2018

Changed commit from 39c78d1 to c97bfcf

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 12, 2018

Branch pushed to git repo; I updated commit sha1. New commits:

c566ca7complex_arb: sage.rings.integer.Integer -> Integer
c97bfcf#24717 implement ComplexBall.pow(expo, analytic=...)

@tscrim
Copy link
Collaborator

tscrim commented Feb 13, 2018

comment:7

Replying to @mezzarobba:

Replying to @fredrik-johansson:

Indeed, but pow is perhaps the most important of them all...

Yes, but it is also a bit harder to implement due to the need to share code with the __pow__, which participates in the coercion system... but ok, I'll try to see what I can do.

If you implement __pow__, you bypass the coercion system altogether (similar to if you implement __mul__). ADDENDUM - Unless of course you call the coercion framework yourself.

@mezzarobba
Copy link
Member Author

comment:8

Replying to @tscrim:

ADDENDUM - Unless of course you call the coercion framework yourself.

Yes, that's what both the previous code and my patch are doing.

@videlec
Copy link
Contributor

videlec commented Mar 4, 2018

comment:10

I agree with comment:2 that this would better be tested in conjunction with numerical integration.

Moreover, does it work correctly with function compositions? Could I integrate cos(exp(x)) using

lambda z,f: z.exp(f).cos(f)

If so, we shoud add examples in CBF.integral.

@fchapoton
Copy link
Contributor

comment:11

does not apply

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 11, 2018

Changed commit from c97bfcf to 08cd02d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 11, 2018

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

3b26420#24717 support the "analytic" flag in some methods of complex balls
aad5a45complex_arb: sage.rings.integer.Integer -> Integer
08cd02d#24717 implement ComplexBall.pow(expo, analytic=...)

@mezzarobba
Copy link
Member Author

comment:13

thanks, rebased!

@videlec
Copy link
Contributor

videlec commented Apr 7, 2018

comment:14

You should be using PY_NEW(Integer) and not Integer.__new__(Integer) (unless something changed around Cython about tp_new).

It would be nice to have usage example in the integral docstring.

@mezzarobba
Copy link
Member Author

comment:15

Replying to @videlec:

You should be using PY_NEW(Integer) and not Integer.__new__(Integer) (unless something changed around Cython about tp_new).

Can you explain the difference? I thought Integer.__new__(Integer) had been working already for a long time, and

$ git grep 'Integer.__new__' | wc -l
95

It would be nice to have usage example in the integral docstring.

See above :-)

@videlec
Copy link
Contributor

videlec commented Apr 8, 2018

Reviewer: Vincent Delecroix

@vbraun
Copy link
Member

vbraun commented May 6, 2018

comment:20

Merge conflict

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 7, 2018

Changed commit from d4cb5b9 to 93679c5

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 7, 2018

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

7023769#24717 support the "analytic" flag in some methods of complex balls
8164105complex_arb: sage.rings.integer.Integer → Integer
aa833eecomplex_arb: Integer.__new__ → PY_NEW
93679c5#24717 implement ComplexBall.pow(expo, analytic=...)

@mezzarobba
Copy link
Member Author

comment:22

Rebased (but without conflict for me: are you seeing a merge conflict with develop, or was that with additional changes?).

@vbraun
Copy link
Member

vbraun commented May 8, 2018

comment:23

Merge conflict (wait for the next beta)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 10, 2018

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

c921c89#24717 support the "analytic" flag in some methods of complex balls
cb84329complex_arb: sage.rings.integer.Integer → Integer
4b8df54complex_arb: Integer.__new__ → PY_NEW
e174e59#24717 implement ComplexBall.pow(expo, analytic=...)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 10, 2018

Changed commit from 93679c5 to e174e59

@mezzarobba
Copy link
Member Author

comment:25

Ok, it was a trivial conflict.

@vbraun
Copy link
Member

vbraun commented May 12, 2018

comment:26
[sagelib-8.3.beta0] Error compiling Cython file:
[sagelib-8.3.beta0] ------------------------------------------------------------
[sagelib-8.3.beta0] ...
[sagelib-8.3.beta0]             sage: ZZ(CBF(i))
[sagelib-8.3.beta0]             Traceback (most recent call last):
[sagelib-8.3.beta0]             ...
[sagelib-8.3.beta0]             ValueError: 1.000000000000000*I does not contain a unique integer
[sagelib-8.3.beta0]         """
[sagelib-8.3.beta0]         cdef Integer res
[sagelib-8.3.beta0]             ^
[sagelib-8.3.beta0] ------------------------------------------------------------
[sagelib-8.3.beta0] 
[sagelib-8.3.beta0] sage/rings/complex_arb.pyx:1512:13: 'Integer' is not a type identifier

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 12, 2018

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

9395581complex_arb: sage.rings.integer.Integer → Integer
19005f5complex_arb: Integer.__new__ → PY_NEW
c827491#24717 implement ComplexBall.pow(expo, analytic=...)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 12, 2018

Changed commit from e174e59 to c827491

@mezzarobba
Copy link
Member Author

comment:28

Sorry.

@vbraun
Copy link
Member

vbraun commented May 15, 2018

Changed branch from u/mmezzarobba/acb_cuts to c827491

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

No branches or pull requests

6 participants