{Real,Complex}Ball: Miscellaneous fixes and improvements #19152

mezzarobba · 2015-09-07T12:31:54Z

Mostly for compatibility with RIF/CIF.

Depends on #19063

CC: @cheuberg

Component: numerical

Author: Marc Mezzarobba

Branch/Commit: 0a04e62

Reviewer: Clemens Heuberger

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

The text was updated successfully, but these errors were encountered:

mezzarobba · 2015-09-07T12:32:48Z

Commit: d0e948b

mezzarobba · 2015-09-07T12:32:48Z

Branch: u/mmezzarobba/19152-arb-misc

mezzarobba · 2015-09-07T12:32:48Z

Last 10 new commits:

`f9e13d6`	`ComplexBall: conversions to ZZ, RR, RIF, CC, CIF`
`4a7b5c4`	`ComplexBallField: faster access to real field`
`b4e82aa`	`{Real,Complex}Ball: implement conversions to QQ`
`5e1cbda`	`ComplexBall: containment and overlapping predicates`
`d44c1b1`	`ComplexBall: abs(), arg()`
`62af563`	`RealBall: implement abs()`
`448bf24`	`{Real,Complex}Ball: implement {above,below}_abs()`
`0196f69`	`ComplexBallField: minor documentation improvements`
`f2d093e`	`RealBall: upper(), lower(), endpoints()`
`d0e948b`	{Real,Complex}Ball: change `__nonzero__` to check for syntactic !=0

mezzarobba · 2015-09-07T12:33:16Z

Dependencies: #19063

sagetrac-git · 2015-09-07T12:56:29Z

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

`cfdffdd`	`{Real,Complex}Ball: contains_zero()`

sagetrac-git · 2015-09-07T12:56:29Z

Changed commit from d0e948b to cfdffdd

sagetrac-git · 2015-09-07T15:37:21Z

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

`a53da39`	`{Real,Complex}Ball: <<, >> (of a ball by an integer)`

sagetrac-git · 2015-09-07T15:37:21Z

Changed commit from cfdffdd to a53da39

sagetrac-git · 2015-10-15T09:19:15Z

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

`63e4b76`	`{Real,Complex}Ball: add some tests`
`0db149f`	`{Real,Complex}Ball: rename abs() to __abs__()`
`1ee9919`	`RealBall: implement min(), max()`
`fa2c0b7`	`{Real,Complex}Ball: __hash__()`
`1741bce`	`{Real,Complex}Ball: bind arb's add_error`
`feb8384`	`Implement ComplexBall.rad()`
`a724343`	`real_arb: lone (and misplaced) sig_on()`
`9e3b251`	`real_arb: work around weakness of arb_set_interval_mpfr`
`f30820e`	`RealBall: union()`
`50f4459`	`Implement CBF.complex_field()`

sagetrac-git · 2015-10-15T09:19:15Z

Changed commit from a53da39 to 50f4459

mezzarobba · 2015-10-15T09:22:04Z

comment:7

Rebased and extended.

sagetrac-git · 2015-10-15T12:36:49Z

Changed commit from 50f4459 to 9097cd4

sagetrac-git · 2015-10-15T12:36:49Z

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

`560fe0b`	`{Real,Complex}Ball: add some tests`
`a1d9086`	`{Real,Complex}Ball: rename abs() to __abs__()`
`0a8efdd`	`RealBall: implement min(), max()`
`8049aad`	`{Real,Complex}Ball: __hash__()`
`e9700cb`	`{Real,Complex}Ball: bind arb's add_error`
`761d53f`	`Implement ComplexBall.rad()`
`ad628b8`	`real_arb: lone (and misplaced) sig_on()`
`030652a`	`real_arb: work around weakness of arb_set_interval_mpfr`
`3aa26c6`	`RealBall: union()`
`9097cd4`	`Implement CBF.complex_field()`

sagetrac-git · 2015-10-16T10:51:49Z

Changed commit from 9097cd4 to c802d70

sagetrac-git · 2015-10-16T10:51:49Z

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

`7b67b5e`	`{Real,Complex}Ball: add some tests`
`202f64c`	`{Real,Complex}Ball: rename abs() to __abs__()`
`8f1ddaf`	`RealBall: implement min(), max()`
`77f1174`	`{Real,Complex}Ball: __hash__()`
`0d312da`	`{Real,Complex}Ball: bind arb's add_error`
`11649d7`	`Implement ComplexBall.rad()`
`638607e`	`real_arb: lone (and misplaced) sig_on()`
`eaf70fa`	`real_arb: work around weakness of arb_set_interval_mpfr`
`23c34f8`	`RealBall: union()`
`c802d70`	`Implement CBF.complex_field()`

cheuberg · 2015-10-30T13:27:22Z

comment:10

As discussed in #17220, the experimental decorator should be removed.

cheuberg · 2015-10-31T19:18:35Z

Changed branch from u/mmezzarobba/19152-arb-misc to u/cheuberg/19152-arb-misc

cheuberg · 2015-10-31T19:20:29Z

Reviewer: Clemens Heuberger

cheuberg · 2015-10-31T19:20:29Z

Changed commit from c802d70 to 16a04a0

cheuberg · 2015-10-31T19:20:29Z

comment:12

I read the modifications and have the following comments.

arf.pxd: You define arf_rnd_t as an enum and rely on the fact that the numeric values of ARF_RND_DOWN etc. will never change in the C header files. However, there, the values are macros, so I do not know whether we can safely assume the values never to change.
polynomial_element.pyx:
- I am very surprised to see this change here in an arb ticket.
- The documentation of is_gen states: "Important - this function doesn’t return True if self equals the generator; it returns True if self is the generator." I.e., the outcome differs from the old behaviour.
power_series_ring_element.pyx: I am surprised to see this change here in an arb ticket, but I guess that it will not do any harm.
RealBall.__hash__: I do not like the fact that the fractional part of the midpoint is ignored completely:

sage: hash(RBF(3/4))
1048577
sage: hash(RBF(5/8))
1048577
sage: hash(RBF(7/8))
1048577

RealBall.upper, RealBall.lower, RealBall.endpoints: missing INPUT: and OUTPUT: sections, document and test parameter rnd
RealBall.add_error: missing INPUT: and OUTPUT: sections
RealBall.is_nonzero: The note section is hard to understand because __nonzero__ and is_zero should do very different things.
RealBall.__lshift__, RealBall.__rshift__: why is the first parameter called val instead of self and how can it happen that it is not a RealBall if this is a method of RealBall?
ComplexBall._is_atomic: self.is_real() or self.imag().is_zero() seems to be redundant. Do you want to say self.real().is_zero() instead?
ComplexBall.add_error: missing INPUT: and OUTPUT: sections
ComplexBall.is_nonzero: The note section is hard to understand because __nonzero__ and is_zero should do very different things.
ComplexBall.__lshift__, ComplexBall.__rshift__: why is the first parameter called val instead of self and how can it happen that it is not a ComplexBall if this is a method of ComplexBall?

New commits:

`a725299`	`Trac #19152: fix error in module description`
`ada2759`	`Trac #19152: Fix hyperlinks in documentation`
`32cc12a`	`Trac #19152: add SEEALSO blocks`
`617b9a7`	`Trac #19152: fix typo and spacing`
`59ab3cb`	`Trac #19152: fix _repr_option`
`16a04a0`	`Trac #19152: fix docstring`

cheuberg · 2015-11-13T18:21:09Z

comment:59

Replying to @mezzarobba:

Replying to @cheuberg:

I would not mind having RBF, CBF, RealBallField and ComplexBallField in the global name space; this is how most of sage seems to work. But this could also happen in another ticket ...

Done — I still don't like that very much, but since everyone seems to agree...

Is there a particular reason for not using lazy_import?

Apart from that question, all my concerns have been dealt with and as far as I can see, Jeroen's comments are also all taken into account.

I think that #19568 is not too related to the changes in this ticket (RealBallField was never marked as experimental, as that decorator did not yet exist when it was introduced).

Therefore, I intend to set this ticket to positive as soon as the question about lazy imports is settled.

jdemeyer · 2015-11-13T19:43:04Z

comment:61

Replying to @cheuberg:

I think that #19568 is not too related to the changes in this ticket

It's not related but it would be nice to fix #19568 before too many people start using RBF.

mezzarobba · 2015-11-16T09:48:30Z

comment:62

Replying to @cheuberg:

Is there a particular reason for not using lazy_import?

Yes: I'm not sure that the modules are expensive enough to be worth importing with lazy_import, and it causes doctest failures that I didn't have the time or motivation to investigate...

jdemeyer · 2015-11-16T14:40:57Z

comment:63

Replying to @mezzarobba:

it causes doctest failures that I didn't have the time or motivation to investigate...

Can you at least push your attempt such that it can be investigated?

mezzarobba · 2015-11-16T15:36:41Z

comment:64

Replying to @jdemeyer:

Replying to @mezzarobba:

it causes doctest failures that I didn't have the time or motivation to investigate...

Can you at least push your attempt such that it can be investigated?

The following change:

diff --git a/src/sage/rings/all.py b/src/sage/rings/all.py
index b5d4db7..33f18a1 100644
--- a/src/sage/rings/all.py
+++ b/src/sage/rings/all.py
@@ -83,7 +83,8 @@ from real_double import RealDoubleField, RDF, RealDoubleElement
 
 from real_lazy import RealLazyField, RLF, ComplexLazyField, CLF
 
-from sage.rings.real_arb import RealBallField, RBF
+lazy_import("sage.rings.real_arb", "RealBallField")
+lazy_import("sage.rings.real_arb", "RBF")
 
 # Polynomial Rings and Polynomial Quotient Rings
 from polynomial.all import *

leads to:

$ ./sage -t src/sage/rings/real_arb.pyx src/sage/rings/complex_arb.pyx
too many failed tests, not using stored timings
Running doctests with ID 2015-11-16-16-34-41-63268156.
Git branch: arb-misc
Using --optional=mpir,python2,sage,scons
Doctesting 2 files.
sage -t src/sage/rings/real_arb.pyx
**********************************************************************
File "src/sage/rings/real_arb.pyx", line 314, in sage.rings.real_arb.RealBallField
Failed example:
    (1/2*RBF(1)) + AA(sqrt(2)) - 1 + polygen(QQ, x)
Exception raised:
    Traceback (most recent call last):
      File "/home/marc/co/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 496, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/marc/co/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 858, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.rings.real_arb.RealBallField[2]>", line 1, in <module>
        (Integer(1)/Integer(2)*RBF(Integer(1))) + AA(sqrt(Integer(2))) - Integer(1) + polygen(QQ, x)
      File "sage/structure/element.pyx", line 1702, in sage.structure.element.RingElement.__add__ (build/cythonized/sage/structure/element.c:16575)
        return coercion_model.bin_op(left, right, add)
      File "sage/structure/coerce.pyx", line 1070, in sage.structure.coerce.CoercionModel_cache_maps.bin_op (build/cythonized/sage/structure/coerce.c:9739)
        raise TypeError(arith_error_message(x,y,op))
    TypeError: unsupported operand parent(s) for '+': 'Real ball field with 53 bits precision' and 'Univariate Polynomial Ring in x over Rational Field'
**********************************************************************
1 item had failures:
   1 of  14 in sage.rings.real_arb.RealBallField
    [454 tests, 1 failure, 0.23 s]
sage -t src/sage/rings/complex_arb.pyx
    [253 tests, 0.25 s]
----------------------------------------------------------------------
sage -t src/sage/rings/real_arb.pyx  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 0.7 seconds
    cpu time: 0.5 seconds
    cumulative wall time: 0.5 seconds
(exit 1)

sagetrac-git · 2015-11-23T15:01:02Z

Changed commit from 0a04e62 to b21aa6e

sagetrac-git · 2015-11-23T15:01:02Z

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

`b21aa6e`	`Trac #19152: lazy_import`

cheuberg · 2015-11-23T15:04:36Z

comment:67

Replying to @mezzarobba:

Replying to @jdemeyer:

Replying to @mezzarobba:
Can you at least push your attempt such that it can be investigated?

The following change:
...
leads to:
...
    TypeError: unsupported operand parent(s) for '+': 'Real ball field with 53 bits precision' and 'Univariate Polynomial Ring in x over Rational Field'

I cannot reproduce this. I used your diff and had no problems with make ptestlong, neither on 6.10.beta0 where this branch is based on nor after merging with 6.10.beta5. Tested only on 64-bit linux though.

Let's see what the patchbots say.

sagetrac-git · 2015-11-25T15:36:37Z

Changed commit from b21aa6e to 0a04e62

sagetrac-git · 2015-11-25T15:36:37Z

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

cheuberg · 2015-11-26T08:48:41Z

comment:69

So the patchbot could reproduce the problem (and in the meantime, I could also reproduce it). Thus I reset the branch to the earlier commit.

The commit with the lazy import is still available as branch u/cheuberg/19152-arb-misc-lazy-import if somebody wants to debug this.

My intention now is to set this ticket to positive review as it is in the next few days unless somebody objects.

vbraun · 2015-11-26T19:27:41Z

comment:70

Fine with me; There seems to be something fishy with lazy import and coercion. Can you open a new ticket and push your lazy-import branch there?

cheuberg · 2015-11-27T10:29:57Z

comment:71

Replying to @vbraun:

Can you open a new ticket and push your lazy-import branch there?

This is now #19628.

Thus it seems time to set this to positive_review.

mezzarobba · 2015-11-27T11:34:05Z

comment:72

I just noticed a stupid bug in the new __hash__ (07e365c509fbbd6e54da9e172f1845f6d24971c9): I forgot to fmpz_init some variables. Would you prefer me to fix it here or to open a new ticket?

cheuberg · 2015-11-27T11:55:06Z

comment:73

It's safer to do it in a new ticket (The release manager might already have pulled this ticket for inclusion).

mezzarobba · 2015-11-27T12:03:08Z

comment:74

Replying to @cheuberg:

It's safer to do it in a new ticket (The release manager might already have pulled this ticket for inclusion).

Done in #19629.

vbraun · 2015-11-28T13:46:09Z

Changed branch from public/19152-arb-misc to 0a04e62

mezzarobba added this to the sage-6.9 milestone Sep 7, 2015

mezzarobba added c: numerical labels Sep 7, 2015

mezzarobba added the s: needs review label Sep 7, 2015

This comment has been minimized.

Sign in to view

cheuberg modified the milestones: sage-6.9, sage-6.10 Oct 30, 2015

cheuberg added s: needs work and removed s: needs review labels Oct 31, 2015

cheuberg added s: needs info and removed s: needs review labels Nov 13, 2015

mezzarobba added s: needs review and removed s: needs info labels Nov 17, 2015

cheuberg added s: positive review and removed s: needs review labels Nov 27, 2015

vbraun removed the s: positive review label Nov 28, 2015

vbraun closed this as completed in f318c4a Nov 28, 2015

This was referenced Nov 27, 2015

Matrix_complex_ball_dense: Level 1 implementation (using acb_mat from Arb) #17220

Closed

Coercions and basic arithmetic for complex balls #19063

Closed

Elementary and special functions for complex balls #19082

Closed

jdemeyer mentioned this issue Nov 29, 2015

Upgrade arb and fix FLINT includes #19563

Closed

mezzarobba mentioned this issue Nov 28, 2015

Fix RealBall.__hash__ #19629

Closed

{Real,Complex}Ball: Miscellaneous fixes and improvements #19152

{Real,Complex}Ball: Miscellaneous fixes and improvements #19152

Comments

mezzarobba commented Sep 7, 2015

mezzarobba commented Sep 7, 2015

mezzarobba commented Sep 7, 2015

mezzarobba commented Sep 7, 2015

mezzarobba commented Sep 7, 2015

sagetrac-git mannequin commented Sep 7, 2015

sagetrac-git mannequin commented Sep 7, 2015

sagetrac-git mannequin commented Sep 7, 2015

sagetrac-git mannequin commented Sep 7, 2015

This comment has been minimized.

sagetrac-git mannequin commented Oct 15, 2015

sagetrac-git mannequin commented Oct 15, 2015

mezzarobba commented Oct 15, 2015

sagetrac-git mannequin commented Oct 15, 2015

sagetrac-git mannequin commented Oct 15, 2015

sagetrac-git mannequin commented Oct 16, 2015

sagetrac-git mannequin commented Oct 16, 2015

cheuberg commented Oct 30, 2015

cheuberg commented Oct 31, 2015

cheuberg commented Oct 31, 2015

cheuberg commented Oct 31, 2015

cheuberg commented Oct 31, 2015

cheuberg commented Nov 13, 2015

jdemeyer commented Nov 13, 2015

mezzarobba commented Nov 16, 2015

jdemeyer commented Nov 16, 2015

mezzarobba commented Nov 16, 2015

sagetrac-git mannequin commented Nov 23, 2015

sagetrac-git mannequin commented Nov 23, 2015

cheuberg commented Nov 23, 2015

sagetrac-git mannequin commented Nov 25, 2015

sagetrac-git mannequin commented Nov 25, 2015

cheuberg commented Nov 26, 2015

vbraun commented Nov 26, 2015

cheuberg commented Nov 27, 2015

mezzarobba commented Nov 27, 2015

cheuberg commented Nov 27, 2015

mezzarobba commented Nov 27, 2015

vbraun commented Nov 28, 2015