py3: no cmp() in real lazy

[fchapoton]

as another step tp py3

CC: @tscrim @a-andre @jdemeyer @saraedum

Component: python3

Keywords: days85

Author: Frédéric Chapoton

Branch/Commit: cec557e

Reviewer: Julian Rüth, Dima Pasechnik, Travis Scrimshaw

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

[fchapoton]

New commits:

6a919fa

py3 : removal of cmp() in real_lazy

[fchapoton]

Branch: u/chapoton/22257

[fchapoton]

Commit: 6a919fa

[fchapoton]

comment:2

waiting for the bots

[fchapoton]

comment:4

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

[fchapoton]

comment:5

here is another try ; let us wait for the bots

---

New commits:

ec2ca43

trac 22257 get rid of cmp(), second tentative

[fchapoton]

Changed branch from u/chapoton/22257 to u/chapoton/22257v2

[fchapoton]

Changed commit from 6a919fa to ec2ca43

[fchapoton]

comment:6

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

[fchapoton]

comment:8

This triggers that:

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

I have not been able to understand why.

[fchapoton]

Changed commit from ec2ca43 to efa77ed

[fchapoton]

Changed branch from u/chapoton/22257v2 to u/chapoton/22257v3

[fchapoton]

comment:9

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

[fchapoton]

comment:10

ok, this almost works, apparently.

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

[jdemeyer]

comment:11

Would this work instead:

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

[JohnCremona]

comment:12

Replying to @fchapoton:

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.

[fchapoton]

comment:13

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.

[fchapoton]

comment:43

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

[fchapoton]

comment:44

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]

comment:45

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]

[fchapoton]

comment:46

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]

comment:47

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]

comment:48

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

[fchapoton]

comment:49

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.

[fchapoton]

comment:50

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

[dimpase]

comment:51

Replying to @fchapoton:

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

just did; no problem in my setup.

[fchapoton]

comment:52

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]

comment:53

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.

[fchapoton]

comment:54

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]

comment:55

Replying to @fchapoton:

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.

[fchapoton]

comment:56

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

[fchapoton]

Changed commit from a93ded9 to cec557e

[fchapoton]

Changed branch from u/chapoton/22257v4 to u/chapoton/22257v6

[fchapoton]

comment:57

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

[dimpase]

comment:58

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

[fchapoton]

comment:59

ok, then let me ask again for a review

[fchapoton]

comment:60

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!

[fchapoton]

comment:61

ping ?

[tscrim]

Changed reviewer from Julian Rüth to Julian Rüth, Dima Pasechnik, Travis Scrimshaw

[tscrim]

comment:62

I'm good sending this back to the buildbots.

[vbraun]

Changed branch from u/chapoton/22257v6 to cec557e