comparison of pi and infinity wrong

[dkrenn]

We have

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

which is obviously wrong. It seems that the problem only occurs with pi, because the following give correct results

sage: bool(pi<2*pi)
True
sage: bool(2*pi<Infinity)
True
sage: bool(e<Infinity)  
True
sage: bool(e<pi)
True

This was reported on  sage-support by Robert Samal.

Apply trac_12967-symbolic_ring-review.patch .

[burcin]

ATM, this is caused by #11506:

sage: pi.pyobject()
pi
sage: type(pi.pyobject())
<class 'sage.symbolic.constants.Pi'>
sage: pi.pyobject() < oo        
False
sage: pi.pyobject() > oo
True

With #12950, comparison of infinities in Pynac changed. Now I get:

sage: bool(pi>Infinity)
False
sage: bool(pi<Infinity) 
False

which is better. I hope with the ordering patches in the Pynac queue this will improve.

The results of comparison with e is not relevant in this case. e is not a constant in Pynac, it is the exp() function. Once you form the relational expression e < Infinity, the comparison is handled differently.

I suggest adding a patch with doctests reflecting the new behavior with #12950 and closing this ticket when #12950 is merged.

[tscrim]

Here's the quick patch with some doctests reflecting the new behavior.

[kcrisman]

----------------------------------------------------------------------
| Sage Version 5.4.1, Release Date: 2012-11-15                       |
| Type "notebook()" for the browser-based notebook interface.        |
| Type "help()" for help.                                            |
----------------------------------------------------------------------
sage: bool(pi < infinity)
False
sage: bool(oo < pi)
False

That is what should actually be tested, as Burcin points out. Also, I feel like this is unintuitive enough of behavior (that pi is more or less incomparable with infinity and not like this)

sage: bool(2< oo)
True

that we should say something to that effect here, maybe even elsewhere in comparison doc where we have other examples saying that > gives False if we can't prove it's True.

I'm also wondering whether this is really "fixed" and deserves that doctest status.

[tscrim]

How about this? Can you think of any other places to put the warning about symbolic ring comparisons? Thanks.

[kcrisman]

There is a typo I am fixing in a review patch.

Otherwise I'll give the patch qua patch positive review - I don't think there is more you can do here, this seems fine and passes tests etc. - but wait on a comment from Burcin or someone else as to whether this should count as a fix before pressing the button. I guess I'm just a little uncomfortable with that, though I understand the sentiment and could be easily persuaded by a third party.

[kcrisman]

Apply only this patch

[kcrisman]

Patchbot, apply trac_12967-symbolic_ring-review.patch

[tscrim]

I'll let someone else set this to positive review, and I do understand your discomfort. Thank you for the review.

[ppurka]

This is absurd. Why should bool(pi < oo) return False? If we have symbolic constants, then we should be able to coerce them to some ring, possibly RR, and do the comparison.