py3: no cmp() in real lazy

[chapoton]

as another step tp py3

[chapoton]

New commits:

​6a919fa

py3 : removal of cmp() in real_lazy

[chapoton]

waiting for the bots

[chapoton]

ooch, this one is going to be more complicated than expected..

[chapoton]

here is another try ; let us wait for the bots

---

New commits:

​ec2ca43

trac 22257 get rid of cmp(), second tentative

[chapoton]

Oooch, again. This is going to be *really* more complicated than expected.

[chapoton]

This triggers that:

sage: z4 = CyclotomicField(4).gen()
sage: CDF(z4)
0.0

I have not been able to understand why.

[chapoton]

another try, let us wait for the patchbots

---

New commits:

​6a919fa	py3 : removal of cmp() in real_lazy
​9eb46b5	Merge branch 'u/chapoton/22257' in 7.6.b6
​efa77ed	trac 22257 fixing doctests

[chapoton]

ok, this almost works, apparently.

Could some number-theorist please tell me if the failing doctest is really a failure ?

[jdemeyer]

Would this work instead:

return richcmp(left.endpoints(), right.endpoints(), op)

[cremona]

Replying to chapoton:

ok, this almost works, apparently.

Could some number-theorist please tell me if the failing doctest is really a failure ?

Could you post here what the test is and what fails? To save time.

[chapoton]

Yes, sure. Here is the existing doctest

sage: f = x^8 - 3*x^7 + 61/3*x^6 - 9*x^5 + 298*x^4 + 458*x^3 + 1875*x^2 + 4293*x + 3099
sage: F1 = NumberField(f, 'z', embedding=-1.18126721294295 + 3.02858651117832j)
sage: F2 = NumberField(f, 'z', embedding=-1.18126721294295 - 3.02858651117832j)
sage: (F, map1, map2, k) = F1.composite_fields(F2, both_maps=True)[0]
sage: F.degree()
32
sage: F.gen() == map2(F2.gen()) + k*map1(F1.gen())
True

It fails by answering instead that F has degree 8 (just as F1, F2), with 2 identity maps as morphisms.

[chapoton]

But wait, it seems that this number theoretical doctest failure disappears with the latest change suggested by Jeroen.

[chapoton]

let me launch my patchbot on this new version

---

New commits:

​e1f07ca

trac 22257 fixing back doctests

[cremona]

Degree 4 is certainly wrong since F1 contains only one root of f. Over F1, f factors into degrees 1, 3, 4. If the conjugate root is a root of the degree 4 factor then the degree of the composite would indeed be 8*4=32. I did not check whether the conjugate is a root of the degree 3 factor; if it was, the composite would have degree 24.

[chapoton]

Thank you, John and Jeroen.

Bot is now green, so please review. Turns out to be not so complicated, in the end.

[cremona]

Just to confirm, I checked that after adjoining the first root, the second is a root of the degree 4 factor, so the compositum does have degree 8*4=32.

[jdemeyer]

Why do you need to add those bool() calls to various doctests?

[jdemeyer]

You don't need this:

from cpython.object cimport Py_EQ

[jdemeyer]

Why this convoluted code

+        sage: x, y = xyz.xy()
+        sage: max(abs(x - 6), abs(y + 15)) < 1e-14
+        True

instead of abs_tol?

[git]

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

​a93ded9

trac 22257 details

[chapoton]

done.

For the added bool, they are necessary because otherwise one gets a symbolic equality. This is probably caused by a difference of behavior between cmp and richcmp in the symbolic ring (that we will have to handle at some point, for sure).

[chapoton]

green bot, please review

[chapoton]

ping ?

[saraedum]

This looks good. I don't really understand the bool() though. If we keep it like this, then code that compares elements of RLF is going to break, right?

[chapoton]

no, because the bool will be called by the "if" or "while" or whatever uses it as a test. Only bare equalities will stay unevaluated.

[jdemeyer]

Wouldn't it be better to add the bool in _richcmp_?

[chapoton]

Well, this is only needed for lazy_numbers initialized from the Symbolic Ring.

I would prefer to keep the bool where it is. This is really just a very tiny shadow of the fact that the symbolic ring has two different uses for cmp and richcmp. We will need to get rid of one of these at some point, but this is not the place to do so.

[jdemeyer]

Replying to chapoton:

Well, this is only needed for lazy_numbers initialized from the Symbolic Ring.

This is really just a very tiny shadow of the fact that the symbolic ring has two different uses for cmp and richcmp. We will need to get rid of one of these at some point, but this is not the place to do so.

I agree with all of the above. Still, I don't like the fact that you are breaking RLF(1) < RLF(sqrt(2)).

[saraedum]

Ok, I see now. I think this is fine then. jdemeyer, please set this back to needs_work if you think that something needs to be done about the comparisons.

[jdemeyer]

Replying to saraedum:

Ok, I see now. I think this is fine then. jdemeyer, please set this back to needs_work if you think that something needs to be done about the comparisons.

Well, I do think that there is something to be fixed here. But I don't feel strongly enough about it to set the ticket back to needs_work.

[vbraun]

On OSX:

sage -t --long --warn-long 126.9 src/sage/symbolic/expression.pyx
**********************************************************************
File "src/sage/symbolic/expression.pyx", line 3537, in sage.symbolic.expression.Expression._cmp_
Failed example:
    RealSet((0, pi),[pi, pi],(pi,4))
Exception raised:
    Traceback (most recent call last):
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 498, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 861, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.symbolic.expression.Expression._cmp_[40]>", line 1, in <module>
        RealSet((Integer(0), pi),[pi, pi],(pi,Integer(4)))
      File "sage/misc/lazy_import.pyx", line 389, in sage.misc.lazy_import.LazyImport.__call__ (/Users/buildslave-sage/slave/sage_git/build/src/build/cythonized/sage/misc/lazy_import.c:4007)
        return self._get_object()(*args, **kwds)
      File "sage/misc/classcall_metaclass.pyx", line 330, in sage.misc.classcall_metaclass.ClasscallMetaclass.__call__ (/Users/buildslave-sage/slave/sage_git/build/src/build/cythonized/sage/misc/classcall_metaclass.c:1410)
        return cls.classcall(cls, *args, **kwds)
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/sets/real_set.py", line 644, in __classcall__
        intervals.append(InternalRealInterval(lower, True, upper, True))
      File "sage/misc/classcall_metaclass.pyx", line 330, in sage.misc.classcall_metaclass.ClasscallMetaclass.__call__ (/Users/buildslave-sage/slave/sage_git/build/src/build/cythonized/sage/misc/classcall_metaclass.c:1410)
        return cls.classcall(cls, *args, **kwds)
      File "sage/misc/cachefunc.pyx", line 1059, in sage.misc.cachefunc.CachedFunction.__call__ (/Users/buildslave-sage/slave/sage_git/build/src/build/cythonized/sage/misc/cachefunc.c:6077)
        w = self.f(*args, **kwds)
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/structure/unique_representation.py", line 1022, in __classcall__
        instance = typecall(cls, *args, **options)
      File "sage/misc/classcall_metaclass.pyx", line 497, in sage.misc.classcall_metaclass.typecall (/Users/buildslave-sage/slave/sage_git/build/src/build/cythonized/sage/misc/classcall_metaclass.c:1859)
        return (<PyTypeObject*>type).tp_call(cls, args, kwds)
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/sets/real_set.py", line 123, in __init__
        raise ValueError('lower/upper bounds are not sorted')
    ValueError: lower/upper bounds are not sorted
**********************************************************************

[chapoton]

hell ! I have no apple on which to debug that..

[chapoton]

may be related to #16035 ?

[dimpase]

Replying to chapoton:

hell ! I have no apple on which to debug that..

this is perfectly reproducible on FreeBSD 11.0 with clang 3.8, as described in #22679 FreeBSD 11 is some kind of approximation of OSX.

I took the current branch there and merged your u/chapoton/22257v4.

[chapoton]

I made a branch u/chapoton/22257v5 where the richcmp are wrapped with bool

[chapoton]

This really needs somebody with either osx or freebsd to do the debugging. Please help !

[chapoton]

Could please at least someone with either a Mac or FreeBSD give me the result of the failing doctest above with the branch

u/chapoton/22257_testing

applied ?

[dimpase]

here is FreeBSD with u/chapoton/22257_testing

sage -t --long --warn-long 41.0 src/sage/symbolic/expression.pyx
**********************************************************************
File "src/sage/symbolic/expression.pyx", line 3537, in sage.symbolic.expression.Expression._cmp_
Failed example:
    RealSet((0, pi),[pi, pi],(pi,4))
Exception raised:
    Traceback (most recent call last):
      File "/usr/home/dima/Sage/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 498, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/home/dima/Sage/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 861, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.symbolic.expression.Expression._cmp_[40]>", line 1, in <module>
        RealSet((Integer(0), pi),[pi, pi],(pi,Integer(4)))
      File "sage/misc/lazy_import.pyx", line 389, in sage.misc.lazy_import.LazyImport.__call__ (/home/dima/Sage/sage/src/build/cythonized/sage/misc/lazy_import.c:4016)
        return self._get_object()(*args, **kwds)
      File "sage/misc/classcall_metaclass.pyx", line 330, in sage.misc.classcall_metaclass.ClasscallMetaclass.__call__ (/home/dima/Sage/sage/src/build/cythonized/sage/misc/classcall_metaclass.c:1415)
        return cls.classcall(cls, *args, **kwds)
      File "/usr/home/dima/Sage/sage/local/lib/python2.7/site-packages/sage/sets/real_set.py", line 645, in __classcall__
        intervals.append(InternalRealInterval(lower, True, upper, True))
      File "sage/misc/classcall_metaclass.pyx", line 330, in sage.misc.classcall_metaclass.ClasscallMetaclass.__call__ (/home/dima/Sage/sage/src/build/cythonized/sage/misc/classcall_metaclass.c:1415)
        return cls.classcall(cls, *args, **kwds)
      File "sage/misc/cachefunc.pyx", line 1059, in sage.misc.cachefunc.CachedFunction.__call__ (/home/dima/Sage/sage/src/build/cythonized/sage/misc/cachefunc.c:6086)
        w = self.f(*args, **kwds)
      File "/usr/home/dima/Sage/sage/local/lib/python2.7/site-packages/sage/structure/unique_representation.py", line 1022, in __classcall__
        instance = typecall(cls, *args, **options)
      File "sage/misc/classcall_metaclass.pyx", line 497, in sage.misc.classcall_metaclass.typecall (/home/dima/Sage/sage/src/build/cythonized/sage/misc/classcall_metaclass.c:1864)
        return (<PyTypeObject*>type).tp_call(cls, args, kwds)
      File "/usr/home/dima/Sage/sage/local/lib/python2.7/site-packages/sage/sets/real_set.py", line 124, in __init__
        raise ValueError(msg.format(lower, upper))
    ValueError: lower/upper bounds (3.141592653589794?/3.141592653589794?) are not sorted
**********************************************************************
1 item had failures:
   1 of  44 in sage.symbolic.expression.Expression._cmp_
    [2673 tests, 1 failure, 25.49 s]

[chapoton]

Thanks a lot, Dima ! So the problem is not coming from where Volker thought it could come..

Could you just try RealSet([pi, pi]) and pi == pi please ?

[dimpase]

And, just in case, that's what I have at the prompt with this branch applied:

sage: bool(pi-pi == 0)
True
sage: bool(pi-pi < 0)
False
sage: bool(pi-pi > 0)
False
sage: pi<pi
pi < pi
sage: bool(pi<pi)
True
sage: bool(pi==pi)
True
sage: bool(pi>pi)
True
sage: bool(pi<4)
True
sage: bool(pi>4)
False

from the point of view of numerical analysis, this all makes sense. One cannot reliably compare floating point numbers for equality, and pi is just a floating point number in disguise. So you compare two "equal" floating point numbers for equality, this won't fly...

But the following seems to make sense:

sage: bool(pi-pi == 0)
True
sage: bool(pi-pi < 0)
False
sage: bool(pi-pi > 0)
False

(what happens is I suppose that pi-pi gets properly simplified to 0...)

[dimpase]

sage: RealSet([pi, pi])
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-15-46f200bbe718> in <module>()
----> 1 RealSet([pi, pi])

/usr/home/dima/Sage/sage/src/sage/misc/lazy_import.pyx in sage.misc.lazy_import.LazyImport.__call__ (/home/dima/Sage/sage/src/build/cythonized/sage/misc/lazy_import.c:4016)()
    387             True
    388         """
--> 389         return self._get_object()(*args, **kwds)
    390 
    391     def __repr__(self):

/usr/home/dima/Sage/sage/src/sage/misc/classcall_metaclass.pyx in sage.misc.classcall_metaclass.ClasscallMetaclass.__call__ (/home/dima/Sage/sage/src/build/cythonized/sage/misc/classcall_metaclass.c:1415)()
    328         """
    329         if cls.classcall is not None:
--> 330             return cls.classcall(cls, *args, **kwds)
    331         else:
    332             # Fast version of type.__call__(cls, *args, **kwds)

/usr/home/dima/Sage/sage/local/lib/python2.7/site-packages/sage/sets/real_set.pyc in __classcall__(cls, *args)
    643             elif isinstance(arg, list):
    644                 lower, upper = RealSet._prep(*arg)
--> 645                 intervals.append(InternalRealInterval(lower, True, upper, True))
    646             elif isinstance(arg, InternalRealInterval):
    647                 intervals.append(arg)

/usr/home/dima/Sage/sage/src/sage/misc/classcall_metaclass.pyx in sage.misc.classcall_metaclass.ClasscallMetaclass.__call__ (/home/dima/Sage/sage/src/build/cythonized/sage/misc/classcall_metaclass.c:1415)()
    328         """
    329         if cls.classcall is not None:
--> 330             return cls.classcall(cls, *args, **kwds)
    331         else:
    332             # Fast version of type.__call__(cls, *args, **kwds)

/usr/home/dima/Sage/sage/src/sage/misc/cachefunc.pyx in sage.misc.cachefunc.CachedFunction.__call__ (/home/dima/Sage/sage/src/build/cythonized/sage/misc/cachefunc.c:6086)()
   1057                 return self.cache[k]
   1058         except KeyError:
-> 1059             w = self.f(*args, **kwds)
   1060             self.cache[k] = w
   1061             return w

/usr/home/dima/Sage/sage/local/lib/python2.7/site-packages/sage/structure/unique_representation.pyc in __classcall__(cls, *args, **options)
   1020             True
   1021         """
-> 1022         instance = typecall(cls, *args, **options)
   1023         assert isinstance( instance, cls )
   1024         if instance.__class__.__reduce__ == CachedRepresentation.__reduce__:

/usr/home/dima/Sage/sage/src/sage/misc/classcall_metaclass.pyx in sage.misc.classcall_metaclass.typecall (/home/dima/Sage/sage/src/build/cythonized/sage/misc/classcall_metaclass.c:1864)()
    495             TypeError: Argument 'cls' has incorrect type (expected type, got classobj)
    496     """
--> 497     return (<PyTypeObject*>type).tp_call(cls, args, kwds)
    498 
    499 # Class for timing::

/usr/home/dima/Sage/sage/local/lib/python2.7/site-packages/sage/sets/real_set.pyc in __init__(self, lower, lower_closed, upper, upper_closed, check)
    122             if not(lower is minus_infinity or upper is infinity) and lower > upper:
    123                 msg = 'lower/upper bounds ({}/{}) are not sorted'
--> 124                 raise ValueError(msg.format(lower, upper))
    125             if (lower_closed and lower == minus_infinity):
    126                 raise ValueError('interval cannot be closed at -oo')

ValueError: lower/upper bounds (3.141592653589794?/3.141592653589794?) are not sorted

[chapoton]

ok, so the problem is here:

sage: bool(pi>pi)
True

which is not the normal behaviour. I get False with the same branch.

One possible solution (not solving the real issue) would be to replace the test ̀lower > upper by lower - upper > 0.

[chapoton]

By the way, Dima, could you please check the status of #16035 and report there ?

[dimpase]

Replying to chapoton:

By the way, Dima, could you please check the status of #16035 and report there ?

just did; no problem in my setup.

[chapoton]

Thanks. Could you please tell me what happens with

bool(e < e)

and

bool(sqrt(2) < sqrt(2))

Just to be sure we really have located the problem..

[dimpase]

sage: e<e
e < e
sage: bool(e<e)
False
sage: bool(e>e)
False
sage: bool(e==e)
True
sage: type(e)
<type 'sage.symbolic.constants_c.E'>
sage: type(pi)
<type 'sage.symbolic.expression.Expression'>
sage: type(sqrt(2))
<type 'sage.symbolic.expression.Expression'>
sage: sqrt(2)==sqrt(2)
sqrt(2) == sqrt(2)
sage: sqrt(2)<sqrt(2)
sqrt(2) < sqrt(2)
sage: bool(sqrt(2)<sqrt(2))
False
sage: bool(sqrt(2)==sqrt(2))
True
sage: bool(sqrt(2)>sqrt(2))
False

I wonder why pi is not of type sage.symbolic.constants_c.E, while e is. The latter explains why these comparison tests work.

With pi vs sqrt(2), apparently simplification works better with algebraic numbers than with transcendentals.

[chapoton]

good. So the problem is specific to pi and other similar constants. Could you please try

sage: from sage.symbolic.constants import Pi
sage: mypi = Pi()
sage: mypi.__lt__(pi)
pi > 3.141592653589793
sage: bool(_)
False

and

sage: bool(euler_gamma < euler_gamma)
False

[dimpase]

Replying to chapoton:

good. So the problem is specific to pi and other similar constants. Could you please try

sage: from sage.symbolic.constants import Pi
sage: mypi = Pi()
sage: mypi.__lt__(pi)
pi > 3.141592653589793
sage: bool(_)
False

this is OK.

and

sage: bool(euler_gamma < euler_gamma)
False

no, this is not

sage: bool(euler_gamma < euler_gamma)
True
sage: bool(euler_gamma > euler_gamma)
True
sage: bool(euler_gamma == euler_gamma)
True
sage: type(euler_gamma)
<type 'sage.symbolic.expression.Expression'>

so euler_gamma is as bad as pi here.

[chapoton]

Here is a new tentative. Dima, when you can, could you try with this branch (u/chapoton/22257v6), please ?

---

New commits:

​00fc3f1	Merge branch 'u/chapoton/22257v4' in 8.0.b0
​cec557e	trac 22257 trying to fix comparison pi < pi issue on osx

[chapoton]

ok, at least the patchbot is still green on linux.

[dimpase]

OK, u/chapoton/22257v6 passes the long tests in src/sage/symbolic/expression.pyx. And the following also looks sane:

sage: pi<pi
pi < pi
sage: bool(pi<pi)
False
sage: bool(pi>pi)
False
sage: bool(pi==pi)
True

[chapoton]

ok, then let me ask again for a review

[chapoton]

Green bot on linux and should be ok also on macosx.

The only change compared to the previously "positive-reviewed" branch is the removal of the method __lt__ in the class Constant inside the file constants.py. Some doctests have also been added in expressions.pyx.

This means that comparison with pi and similar constants will be handled (in a better way) by the general comparison of expressions.

Once again, this is currently the point where python3 compilation fails. And this is needed to go forward, with next step being 22297.

Please review!

[chapoton]

ping ?

[tscrim]

I'm good sending this back to the buildbots.