bugs in comparisons between constants, wrapped pyobjects, infinity

[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.

See the discussion at https://groups.google.com/forum/?hl=en#!topic/sage-devel/Oip2hzvjFZQ

pynac/pynac#69

---

Previously proposed patch: attachment: trac_12967-symbolic_ring-review.patch.

CC: @kcrisman

Component: symbolics

Keywords: compare pi infinity bool

Author: Travis Scrimshaw, Ralf Stephan

Reviewer: Karl-Dieter Crisman, Daniel Krenn

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

[burcin]

comment:1

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]

comment:3

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

[tscrim]

Author: Travis Scrimshaw

[kcrisman]

comment:4

----------------------------------------------------------------------
| 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]

comment:5

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

[kcrisman]

comment:6

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]

Reviewer: Karl-Dieter Crisman

[kcrisman]

comment:7

Attachment: trac_12967-symbolic_ring-review.patch.gz

Patchbot, apply trac_12967-symbolic_ring-review.patch

[tscrim]

comment:8

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

[ppurka]

comment:9

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.

[mezzarobba]

comment:55

Replying to @pjbruin:

Not at all; I often find that a good first step towards fundamentally fixing a bug is trying to remove hacks that someone else put in to make a specific example work.

I strongly agree with that. Making specific examples work with no regard for global consistency usually means introducing bugs, not fixing them.

You can only make it easier (but by a huge amount) with good design and a well thought out model. Since no good model presents itself and delegating the task to the item is good design this would be an ansatz.

I disagree that no good model presents itself; defining x in P as bool(P(x) == x) seems like a good programming model to me, since it is simple and predictable.

I tend to agree. Note however that the current definition is more along the lines of parent(x) == P or P(x) == x, which may not be the same (think of intervals or objects that may not compare as equal to copies of themselves).

The more I think about it, the more the fundamental problem does not seem to be membership testing, but comparison between real numbers defined in different approximations of the real numbers. Do we want RR(sqrt(2)) to be equal to SR(sqrt(2)) or not? And what about RR(1/3) and QQ(1/3)?

As I repeat on every possible occasion, comparisons in sage are broken in more ways than I can count. One of the fundamental reasons IMHO is that there should be at least two kinds of comparisons (in addition to is), call them strict comparisons and semantic comparisons. Under strict equality, for instance, elements of different parents would always compare as unequal, and elements of a parent with no normal form would typically compare as equal only if they have the same syntactic representation. Semantic equality however could attempt coercing the elements in a common parent much like == currently does. (I believe that == should refer to the strict equality since, among other things, that would avoid many of the problems with hashing. But I know that many people disagree.)

Clearly we are talking about semantic equality here, but I believe making the previous distinction (at least conceptually) helps separating the issues. Now about your examples: with the current model with a single equality predicate, my answer is **NOOO!!! ** in both cases. With a separate strict equality, it is less clear-cut, but intuitively I would still expect both results to be False.

[rwst]

comment:56

Given that it would be more consistent (but less user-friendly) to not mix R with RR, and item.is_real() is buggy, the fix of that is critical (#16436, #17117 are aspects). The Parent.__contains__ method would have to be adapted and a warning added. Maybe this even leads to a more consistent Parent.__contains__. The fix of the behaviour of bool(item==P(item)) is then less urgent (because it's rarely used otherwise). As to this ticket where failing comparisons with oo are described, the code does fix these but leaves at least the pi in RIF doctest failing (explanation in comment:33), and possibly more if I remove the RR/CC.__contains__. This could benefit from a more consistent Parent.__contains__ so let's go on.

[vbraun]

comment:57

The symbolic constants, like pi.pyobject(), should be elements in some parent set. The symbolic constants can then coerce into the infinity ring, solving the pi.pyobject() < oo == False issue.

[rwst]

comment:58

So we need another ticket for

sage: bool(sqrt(2)<oo)
False

[rwst]

comment:59

As I have now a better overview, at least some cases could and should be fixed in Pynac. This is pynac/pynac#69

[rwst]

Upstream: Reported upstream. Developers acknowledge bug.

[rwst]

comment:60

Replying to @vbraun:

The symbolic constants, like pi.pyobject(), should be elements in some parent set. The symbolic constants can then coerce into the infinity ring, solving the pi.pyobject() < oo == False issue.

How would NaN be coerced? Or a future Aleph2?

[rwst]

Changed branch from u/rws/12967-2 to none

[rwst]

Changed dependencies from #17984 to pynac-0.3.9.2

[rwst]

comment:61

Pynac git master has a fix that does this:

sage: bool(SR(oo) > 5)
True
sage: bool(5 < SR(oo))
True
sage: bool(SR(2)<Infinity)
True
sage: bool(pi<Infinity)
True
sage: bool(pi>Infinity)
False
sage: bool(2*pi<Infinity)
True
sage: bool(SR(pi) < SR(Infinity))
True
sage: bool(sqrt(2)<oo)
True
sage: bool(log(2)<oo)
True
sage: bool(e<oo)
True
sage: bool(e+pi<oo)
True
sage: bool(e^pi<oo)
True
sage: bool(SR(2)<-oo)
False
sage: bool(SR(2)>-oo)
True
sage: bool(exp(2)>-oo)
True
sage: bool(SR(oo) > sqrt(2))
True
sage: bool(sqrt(2) < SR(oo))
True
sage: bool(SR(-oo) < sqrt(2))
True
sage: bool(sqrt(2) > SR(-oo))
True

It uses info flags and evalf where applicable. Some function info flags were introduced earlier in Pynac.

[rwst]

Changed commit from c5845f6 to none

[rwst]

Changed dependencies from pynac-0.3.9.2 to none

[rwst]

comment:63

The above is doctested in #17321 so what remains here that is not #16397?

[rwst]

Changed upstream from Reported upstream. Developers acknowledge bug. to none

[dkrenn]

Changed reviewer from Karl-Dieter Crisman to Karl-Dieter Crisman, Daniel Krenn