Update to pynac-0.5.2

[Ralf Stephan]

The next big Pynac update has:

• libgmp bigints and bigrats instead of Integer and Rational Python objects
• assumptions/domains can influence Pynac computation
• sync elementary assumptions on symbols/functions with Pynac (#19035)
• C++11 results in better code readability
• better decisions through more info flags and logic
• return not implemented and undecidable for some decisions (for math. logic changes see the #19040 description)
• add inexact flag for possible expansions / FP simplifications
• abs() expansion
• function::info()
• from GiNaC: Fix pow(+(...),2).expand()
• remove some unused files and other cleanup; add missing autoconf macro;
• some older memleaks closed

​https://github.com/pynac/pynac/releases/download/pynac-0.5.2/pynac-0.5.2.tar.bz2

[Ralf Stephan]

Mind you, it's not here yet. Git master has half a dozen small but difficult to trace bugs.

[Ralf Stephan]

This works and can be reviewed independently on machines with gcc>=4.6, but please wait with setting positive for the build system changes (i.e., #19298).

There are two failing doctests: one only happens on doctesting and needs the associated environment; the other is the "pi in RIF" doctest which presented a philosophical problem to Sage developers (see #17984) and thus cannot be resolved in my lifetime.

---

New commits:

​281541d	package version and checksum
​e56e779	C++11 compile switches
​474fff6	19312: pynac/Sage interface adaptations
​5b51215	19312: link assume/forget to pynac
​626fb47	19312: core of math logic rewrite of Expression.__nonzero__(), dedicated is_zero
​9b8a09e	19312: code changes in consequence of ex.__nonzero__() math logic changes
​361e3d8	19312: doc/doctest changes in consequence of math. logic changes
​8421ab9	19312: other doc/doctest changes because of pynac changes

[Jeroen Demeyer]

sage -t src/sage/plot/arc.py
**********************************************************************
File "src/sage/plot/arc.py", line 66, in sage.plot.arc.Arc.__init__
Failed example:
    bool(A[0].angle == pi/4)
Expected:
    True
Got:
    False
**********************************************************************
File "src/sage/plot/arc.py", line 70, in sage.plot.arc.Arc.__init__
Failed example:
    bool(A[0].s2 == pi)
Expected:
    True
Got:
    False
**********************************************************************
1 item had failures:
   2 of  12 in sage.plot.arc.Arc.__init__
    [47 tests, 2 failures, 2.28 s]
sage -t src/sage/structure/parent.pyx
**********************************************************************
File "src/sage/structure/parent.pyx", line 1227, in sage.structure.parent.Parent.__contains__
Failed example:
    pi in RIF # there is no element of RIF equal to pi
Expected:
    False
Got:
    True
**********************************************************************

[git]

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

​4784254

19312: fix omission, doctest

[Ralf Stephan]

I agree to try first a merge of #19040 to reduce the size of this patch.

[Jeroen Demeyer]

Replying to rws:

I agree to try first a merge of #19040 to reduce the size of this patch.

The other way around seems more logical to me: first merge this, then #19040.

[Ralf Stephan]

I excised the math logic changes. Please review.

---

New commits:

​3a48446	19312: package version and checksum
​cc6a6c9	19312: C++11 compile switches
​caa0aeb	19312: pynac/Sage interface adaptations
​a973071	19312: link assume/forget to pynac
​6cf8edb	19312: changes in Expression.__nonzero__ due to Pynac interface change
​d340d9d	19312: doctest adaptations
​fd68fd5	19312: fix code for failing doctest
​eeedddc	19312: kludge to allow unchanged __nonzero__ with pynac-0.5

[Travis Scrimshaw]

FYI - There is an Unknown located in sage.misc.unknown.

[Ralf Stephan]

Replying to tscrim:

FYI - There is an Unknown located in sage.misc.unknown.

That would be useful for #19040, thanks!

[Travis Scrimshaw]

Needs a rebase.

[Ralf Stephan]

If you commit to the review I'll do the rebase.

[Travis Scrimshaw]

I should be able to do the review.

[git]

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

​da6fe01

Merge branch 'develop' into t/19312/19312-2

[git]

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

​dfb2ba3

19321:fix typo

[Ralf Stephan]

Great. So please review.

[Jeroen Demeyer]

This should be rebased on top of #19606.

[Jeroen Demeyer]

Replace

if not (solution_dict == True or solution_dict == False)

by

if solution_dict is not True and solution_dict is not False

[Jeroen Demeyer]

Why do you need the __cmp__ method on Constant? Since we are trying to move to Python 3, we shouldn't add new __cmp__ or cmp().

[Jeroen Demeyer]

In src/sage/calculus/desolvers.py, I think you need to justify the changes

@@ -676,7 +676,7 @@ def desolve_laplace(de, dvar, ics=None, ivar=None):
         dvar, ivar = dvar
     elif ivar is None:
         ivars = de.variables()
-        ivars = [t for t in ivars if t != dvar]
+        ivars = [t for t in ivars if t is not dvar]
         if len(ivars) != 1:
             raise ValueError("Unable to determine independent variable, please specify.")
         ivar = ivars[0]
@@ -1182,7 +1181,7 @@ def desolve_rk4(de, dvar, ics=None, ivar=None, end_points=None, step=0.1, output
 
     if ivar is None:
         ivars = de.variables()
-        ivars = [t for t in ivars if t != dvar]
+        ivars = [t for t in ivars if repr(t) != repr(dvar)]
         if len(ivars) != 1:
             raise ValueError("Unable to determine independent variable, please specify.")
         ivar = ivars[0]

[Jeroen Demeyer]

Replace

cdef object py_integer_from_long(long x) except +:
    cdef Integer z = PY_NEW(Integer)
    mpz_set_si(z.value, x)
    return z

by

from sage.rings.integer cimport smallInteger

cdef py_integer_from_long(long x):
    return smallInteger(x)

[Jeroen Demeyer]

I also don't understand the following doctest changes:

@@ -1148,7 +1148,8 @@ def desolve_rk4(de, dvar, ics=None, ivar=None, end_points=None, step=0.1, output
 
     Variant 2 for input - more common in numerics::
 
-        sage: x,y=var('x y')
+        sage: _ = var('x,y')
+        sage: f = function('f')(x)
         sage: desolve_rk4(x*y*(2-y),y,ics=[0,1],end_points=1,step=0.5)
         [[0, 1], [0.5, 1.12419127424558], [1.0, 1.461590162288825]]
 
@@ -1156,15 +1157,13 @@ def desolve_rk4(de, dvar, ics=None, ivar=None, end_points=None, step=0.1, output
     desolve function In this example we integrate bakwards, since
     ``end_points < ics[0]``::
 
-        sage: y=function('y')(x)
-        sage: desolve_rk4(diff(y,x)+y*(y-1) == x-2,y,ics=[1,1],step=0.5, end_points=0)
+        sage: desolve_rk4(diff(f,x)+f*(f-1) == x-2,f,ics=[1,1],step=0.5, end_points=0)
         [[0.0, 8.904257108962112], [0.5, 1.909327945361535], [1, 1]]
 
     Here we show how to plot simple pictures. For more advanced
     aplications use list_plot instead. To see the resulting picture
     use ``show(P)`` in Sage notebook. ::
 
-        sage: x,y=var('x y')
         sage: P=desolve_rk4(y*(2-y),y,ics=[0,.1],ivar=x,output='slope_field',end_points=[-4,6],thickness=3)
 
     ALGORITHM:

[Jeroen Demeyer]

There are various doctest failures:

sage -t --long src/sage/combinat/finite_state_machine.py
**********************************************************************
File "src/sage/combinat/finite_state_machine.py", line 10001, in sage.combinat.finite_state_machine.FiniteStateMachine.?
Failed example:
    R.<phi> = NumberField(x^2-x-1, embedding=golden_ratio)
Exception raised:
    Traceback (most recent call last):
      File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 496, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 858, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.combinat.finite_state_machine.FiniteStateMachine.?[11]>", line 1, in <module>
        R = NumberField(x**Integer(2)-x-Integer(1), embedding=golden_ratio, names=('phi',)); (phi,) = R._first_ngens(1)
      File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/rings/number_field/number_field.py", line 490, in NumberField
        return NumberField_version2(polynomial=polynomial, name=name, check=check, embedding=embedding, latex_name=latex_name, assume_disc_small=assume_disc_small, maximize_at_primes=maximize_at_primes, structure=structure)
      File "sage/structure/factory.pyx", line 366, in sage.structure.factory.UniqueFactory.__call__ (build/cythonized/sage/structure/factory.c:1328)
        return self.get_object(version, key, kwds)
      File "sage/structure/factory.pyx", line 410, in sage.structure.factory.UniqueFactory.get_object (build/cythonized/sage/structure/factory.c:1734)
        obj = self.create_object(version, key, **extra_args)
      File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/rings/number_field/number_field.py", line 612, in create_object
        return NumberField_quadratic(polynomial, name, latex_name, check, embedding, assume_disc_small=assume_disc_small, maximize_at_primes=maximize_at_primes, structure=structure)
      File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/rings/number_field/number_field.py", line 10070, in __init__
        assume_disc_small=assume_disc_small, maximize_at_primes=maximize_at_primes, structure=structure)
      File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/rings/number_field/number_field.py", line 6421, in __init__
        assume_disc_small=assume_disc_small, maximize_at_primes=maximize_at_primes, structure=structure)
      File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/rings/number_field/number_field.py", line 1202, in __init__
        embedding = number_field_morphisms.create_embedding_from_approx(self, embedding)
      File "sage/rings/number_field/number_field_morphisms.pyx", line 503, in sage.rings.number_field.number_field_morphisms.create_embedding_from_approx (build/cythonized/sage/rings/number_field/number_field_morphisms.c:7328)
        raise ValueError, "%s is not a root of the defining polynomial of %s" % (gen_image, K)
    ValueError: golden_ratio is not a root of the defining polynomial of Number Field in phi with defining polynomial x^2 - x - 1
**********************************************************************
File "src/sage/combinat/finite_state_machine.py", line 10002, in sage.combinat.finite_state_machine.FiniteStateMachine.?
Failed example:
    N = NAFp.number_of_words(base_ring=R); N
Expected:
    1/5*(3*golden_ratio + 1)*golden_ratio^n
    - 1/5*(3*golden_ratio - 4)*(-golden_ratio + 1)^n
Got:
    1/10*(1/2*sqrt(5) + 1/2)^n*(3*sqrt(5) + 5) - 1/10*(-1/2*sqrt(5) + 1/2)^n*(3*sqrt(5) - 5)
**********************************************************************
File "src/sage/functions/other.py", line 1693, in sage.functions.other.Function_beta.__init__
Failed example:
    beta(1/3, 1/2)
Expected:
    beta(1/2, 1/3)
Got:
    beta(1/3, 1/2)
**********************************************************************

[Jeroen Demeyer]

Why do you use except + for non-extern functions? For example

cdef bint py_is_exact(object x) except +:
    return isinstance(x, int) or isinstance(x, long) or isinstance(x, Integer) or \
           (isinstance(x, Element) and
            ((<Element>x)._parent.is_exact() or (<Element>x)._parent == ring.SR))

[Ralf Stephan]

Replying to jdemeyer:

Why do you use except + for non-extern functions? For example

cdef bint py_is_exact(object x) except +:
    return isinstance(x, int) or isinstance(x, long) or isinstance(x, Integer) or \
           (isinstance(x, Element) and
            ((<Element>x)._parent.is_exact() or (<Element>x)._parent == ring.SR))

I have just copied from py_is_even which is not extern either and written by yourself.

[Jeroen Demeyer]

Replying to rws:

I have just copied from py_is_even which is not extern either

Well, then py_is_even is wrong too. But ok, you can leave the except + for now, it's not a major problem.

and written by yourself.

No way. I may have been the last person to edit that line, but I certainly didn't write that function or the except +.

[git]

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

​76733e4

19312: address reviewer's comments

[Travis Scrimshaw]

Here are my current comments:

• I also don't understand the changes on comment:30.
• decl_assume and decl_forget aren't doctested.
• raise TypeError('Cannot compare: ' + repr(self)): I'm thinking that should be a ValueError since it depends on the value of the value of the expression. Also, to better conform with python, error messages are not sentences and should not start with a capital letter.
• Could we change:

  -        if solution_dict is not True and solution_dict is not False:
  +        if not isinstance(solution_dict, bool):
  

• I don't understand this change:

  src/sage/geometry/hyperbolic_space/hyperbolic_isometry.py

  @@ -1054 +1054 @@
           TypeError: A must be an invertible 2x2 matrix over the complex numbers or a symbolic ring
   
       The matrix can be symbolic or can be a matrix over the real
  -    or complex numbers, but must be invertible::
  -
  -        sage: (a,b,c,d) = var('a,b,c,d');
  -        sage: mobius_transform(matrix(2,[a,b,c,d]),I)
  -        (I*a + b)/(I*c + d)
  +    or complex numbers, but must be provably invertible::
   
  +        sage: (b,c) = var('b,c');
  +        sage: mobius_transform(matrix(2,[1,b,c,b*c+1]),I)
  +        (b + I)/(b*c + I*c + 1)
           sage: mobius_transform(matrix(2,[0,0,0,0]),I)
           Traceback (most recent call last):
           ...

  I feel it obfuscates what the Möbius transform does.

[Jeroen Demeyer]

In src/sage/rings/number_field/number_field_morphisms.pyx, can you import test_relation_maxima only when you need it?

This doctest loses its meaning now:

src/sage/functions/other.py

@@ -1691 +1691 @@
         Ginac might reorder the arguments::
 
             sage: beta(1/3, 1/2)
-            beta(1/2, 1/3)
+            beta(1/3, 1/2)
 
         The result is symbolic if exact input is given::
 

[Jeroen Demeyer]

This is actually a bug:

src/sage/symbolic/constants_c.pyx

@@ -217 +217 @@
             sage: t = e^a; t
             e^a
             sage: t^b
-            (e^a)^b
+            e^a^b
             sage: SR(1).exp()
             e
 

Usually, e^a^b is interpreted as e^(a^b):

sage: var('a,b')
(a, b)
sage: (e^a)^b
e^a^b
sage: e^a^b
e^(a^b)

[Ralf Stephan]

Replying to jdemeyer:

Usually, e^a^b is interpreted as e^(a^b):

Oh, that is serious.

[Jeroen Demeyer]

For __hash__, could you also test a number which has a hash more than 2^32, in order to test for #19310:

sage: hash(1e30)
-442393380            # 32-bit
5076964209140210896   # 64-bit
sage: hash(SR(1e30))  # known bug #19310
-442393380            # 32-bit
5076964209140210896   # 64-bit

[Ralf Stephan]

Replying to tscrim:

• I also don't understand the changes on comment:30.

They are reverted now. In the first branch of this ticket they were necessary.

• I don't understand this change:

@@ -1054,12 +1054,11 @@ def mobius_transform(A, z):

I feel it obfuscates what the Möbius transform does.

Well you can't give a transform of a noninvertible matrix, so I changed the example to one that is always invertible. I can revert to the original doctest but it will lead to problems in a later ticket.

[Ralf Stephan]

Replying to jdemeyer:

For __hash__, could you also test a number which has a hash more than 2^32, in order to test for #19310:

That doesn't work in 0.5 but some work is already done. See ​https://github.com/pynac/pynac/issues/95

[Travis Scrimshaw]

Replying to rws:

Replying to tscrim:

• I don't understand this change:

@@ -1054,12 +1054,11 @@ def mobius_transform(A, z):

I feel it obfuscates what the Möbius transform does.

Well you can't give a transform of a noninvertible matrix, so I changed the example to one that is always invertible. I can revert to the original doctest but it will lead to problems in a later ticket.

Unless a*d - b*c == 0, then this is an invertible matrix, and this is a "rare" event for generic a,b,c,d. So I would say this is a safe test, or perhaps we can add an assumption that a*d - b*c != 0? Also could you tell me what ticket you are referring to or give some more details?

[Ralf Stephan]

Replying to tscrim:

Replying to rws:

Well you can't give a transform of a noninvertible matrix, so I changed the example to one that is always invertible. I can revert to the original doctest but it will lead to problems in a later ticket.

Unless a*d - b*c == 0, then this is an invertible matrix, and this is a "rare" event for generic a,b,c,d. So I would say this is a safe test, or perhaps we can add an assumption that a*d - b*c != 0? Also could you tell me what ticket you are referring to or give some more details?

If I remember correctly there was a problem due to the Expression.__nonzero__ changes in the first branch. So this might resurface with #19040. I'm sorry I can't give more details, let's just see about it whenever #19040 gets tackled.

[git]

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

​976c80e

19312: address reviewer's comments

[Ralf Stephan]

This should cover everything except the exponentiation bug and...

Replying to jdemeyer:

This should be rebased on top of #19606.

As I need to make a new release, and #19606 will be in the next beta I'll just do a rebase later.

[git]

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

​c9e4c7e	19312: build dir changes for version 0.5.1
​c5f20d6	19312: doctest and some code fixes with pynac-0.5.1

[Ralf Stephan]

pynac-0.5.1 fixed the exp bug (and contains a previous fix). This should be it.

[Travis Scrimshaw]

Replying to rws:

If I remember correctly there was a problem due to the Expression.__nonzero__ changes in the first branch. So this might resurface with #19040. I'm sorry I can't give more details, let's just see about it whenever #19040 gets tackled.

I will be happy to work on it if there is a problem with #19040.

I'm guessing changes of this type:

-        sage: x,y = var('x,y')
+        sage: _ = var('x,y')

are because you're thinking towards #17958. However I don't think that takes away from the problem, but instead just makes the behavior harder to do find/replace for and makes it harder for the reader to understand what is going on in the doctest. Could you provide some other justification for these changes?

[git]

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

​065791e

19312: address reviewer's comments

[Ralf Stephan]

Replying to tscrim:

I'm guessing changes of this type:

-        sage: x,y = var('x,y')
+        sage: _ = var('x,y')

are because you're thinking towards #17958. However I don't think that takes away from the problem, but instead just makes the behavior harder to do find/replace for and makes it harder for the reader to understand what is going on in the doctest. Could you provide some other justification for these changes?

If you object to that you will have to stem the tide now by posting to sage-devel, because a recursive grep just showed about 30 instances of such usage in Sage (and certainly not all from me). Or open a separate ticket, please. Here they are now reverted.

[Jeroen Demeyer]

This

isinstance(rel, bool) and rel is True

can be simplified to

rel is True

[Jeroen Demeyer]

Why this?

src/sage/symbolic/constants.py

@@ -303 +303 @@
             True
             sage: p == s
             False
-            sage: p != s
-            True
         """
         return (self.__class__ == other.__class__ and
                 self._name == other._name)

[Jeroen Demeyer]

This is not valid LaTeX:

e^{\sqrt{x}}^{x}

[git]

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

​d7e7981	build dir changes for 0.5.2
​af71087	address reviewer's comments; add doctests

[Ralf Stephan]

This is not valid LaTeX:

Is changed. The doctest removal also was to satisfy the first branch. I have reincluded it. Another possible matter with #19040.

[Vincent Delecroix]

What is the definition of an "undecidable relation"? (see also the comments: ​comment:24 and ​comment:41 on #19040).

[Ralf Stephan]

Please see my reply there.

[git]

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

​6c27be1

Merge branch 'develop' into t/19312/19312-2

[Travis Scrimshaw]

There still is

e^{\sqrt{x}}^{x}

in functions/log.py.

Also, for the test in hyperbolic_isometry.py, if we add

sage: assume(a*d != b*c)

then would that avoid issues with #19040?

Other than that, I'm okay with the current state up to Jeroen's comments that haven't been addressed.

[git]

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

​7722756

19312: fix doctest

[Ralf Stephan]

Replying to tscrim:

Also, for the test in hyperbolic_isometry.py, if we add

sage: assume(a*d != b*c)

then would that avoid issues with #19040?

I'm unable to answer this at the moment but I appreciate the idea. Thanks for your review.

[Jeroen Demeyer]

Regarding #19606: Is pynac now linked with gmp? If so, you need to revert #19606 and you need to change the dependencies of the package.

I have no other comments (which doesn't mean that I understand every change you made, but at least I don't see any obvious problems).

[git]

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

​f71e3d8

19312: add new pynac dependencies

[Ralf Stephan]

Indeed, thanks.

[Travis Scrimshaw]

I think then we are good to go.

[Volker Braun]

32-bit fails with

sage -t --long src/sage/symbolic/expression.pyx
**********************************************************************
File "src/sage/symbolic/expression.pyx", line 1455, in sage.symbolic.expression.Expression.__hash__
Failed example:
    hash(SR(19.23))
Expected:
    2836855582
Got:
    -1458111714
**********************************************************************
File "src/sage/symbolic/expression.pyx", line 1457, in sage.symbolic.expression.Expression.__hash__
Failed example:
    hash(19.23)
Expected:
    2836855582
Got:
    -1458111714
**********************************************************************
1 item had failures:
   2 of  26 in sage.symbolic.expression.Expression.__hash__

[git]

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

​bdda35e

19312: differentiate doctest result wrt 32/64 bit