fix Parent.contains #17984

rwst · 2015-03-18T09:59:31Z

In the catch-all structure/parent.pyx:Parent.__contains__ we depend on the construction bool(item==self(item)) to get the right result for, e.g. sqrt(3) in RR/CC but bool(RR/CC(sqrt(3))==sqrt(3)) being False a trick was applied by the author that makes the function always return True. This trick uses the fact that Maxima doesn't know about the rings and will raise an exception, which is then caught.

This way of programming strikes me as very wrong and previously I supported the notion that dedicated __contains__ methods for both RR/CC were needed. As you can see from the discussion this was not wanted.

Of course, as soon as we want to get rid of Maxima this odd behaviour must be simulated, because there is no way to test containment in these inexact rings in a generic way.

In order to make it more reliable the way we treat inclusion in inexact rings needs to be reconsidered.

Previously this ticket proposed that:
...dedicated __contains__ methods for both rings are needed.

Depends on #19040

CC: @sagetrac-tmonteil

Component: basic arithmetic

Author: Ralf Stephan

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

The text was updated successfully, but these errors were encountered:

rwst · 2015-03-18T10:01:03Z

Branch: u/rws/17984

rwst · 2015-03-18T10:01:42Z

Commit: 39ab5b2

rwst · 2015-03-18T10:01:42Z

New commits:

`39ab5b2`	`17984: Dedicated RR.__contains__() and CC.__contains__()`

rwst · 2015-03-18T10:01:42Z

Author: Ralf Stephan

jdemeyer · 2015-03-18T13:11:41Z

Replying to @rwst:

In the catch-all structure/parent.pyx:Parent.__contains__ we depend on a bug to get the right result for, e.g. sqrt(3) in RR/CC

Can you clarify this please?

jdemeyer · 2015-03-18T13:13:55Z

comment:4

sage: RealField(10)(1/3) in RR
True
sage: RealField(10)(1/3) in CC
False

jdemeyer · 2015-03-18T13:14:16Z

comment:5

Why this???

sage: [1,2,3] in CC
True

jdemeyer · 2015-03-18T13:14:41Z

comment:6

I also think this is wrong:

sage: NaN in RR
True

jdemeyer · 2015-03-18T13:19:49Z

comment:7

Also please explain why you use ComplexField() in the code for RR.

rwst · 2015-03-18T14:40:41Z

comment:8

Replying to @jdemeyer:

Replying to @rwst:

In the catch-all structure/parent.pyx:Parent.__contains__ we depend on a bug to get the right result for, e.g. sqrt(3) in RR/CC

Can you clarify this please?

See comment 28 of #12967.

rwst · 2015-03-18T15:05:54Z

comment:9

Replying to @jdemeyer:

sage: RealField(10)(1/3) in RR
True
sage: RealField(10)(1/3) in CC
False

Clever. But before I fix this: What about the doctests? Do you think that any of them gives a wrong result? Do you agree that precision issue should be ignored when it comes to the behaviour of the contains function?

rwst · 2015-03-18T15:25:05Z

comment:10

Replying to @jdemeyer:

I also think this is wrong:
sage: NaN in RR
True

If so, then I think this is wrong, too:

sage: RR(NaN)
NaN
sage: type(_)
<type 'sage.rings.real_mpfr.RealNumber'>

sagetrac-git · 2015-03-18T16:27:58Z

Changed commit from 39ab5b2 to a22a6e4

sagetrac-git · 2015-03-18T16:27:58Z

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

`a22a6e4`	`17984: description in docstring; fixes and doctests`

sagetrac-git · 2015-03-18T16:34:08Z

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

`544450e`	`17984: fix typo in doctest`

sagetrac-git · 2015-03-18T16:34:08Z

Changed commit from a22a6e4 to 544450e

jdemeyer · 2015-03-18T16:43:44Z

comment:14

Replying to @rwst:

Do you agree that precision issue should be ignored when it comes to the behaviour of the contains function?

Yes, I think precision should be ignored for contains, therefore don't use numerical_approx() but a conversion like RR(foo) or CC(foo).

jdemeyer · 2015-03-18T16:45:20Z

comment:15

Replying to @rwst:

If so, then I think this is wrong, too:
sage: RR(NaN)
NaN
sage: type(_)
<type 'sage.rings.real_mpfr.RealNumber'>

I don't think the above is wrong, it is possible to represent NaN by a RealNumber, but that doesn't mean that NaN can be considered as element of RR.

rwst · 2015-03-18T17:31:01Z

comment:16

Replying to @rwst:

Replying to @jdemeyer:

Replying to @rwst:

In the catch-all structure/parent.pyx:Parent.__contains__ we depend on a bug to get the right result for, e.g. sqrt(3) in RR/CC

Can you clarify this please?

See comment 28 of #12967.

Ah, seems I was chasing ghosts there. Nevertheless the ticket makes sense because of the unreliability of the construction bool(item==self(item)) for determining elementship.

rwst · 2015-03-21T07:25:42Z

comment:24

Replying to @pjbruin:

Hence I am tending towards the opinion that if x is some exact element, then bool(RR(x) == x) should return True if and only if x is exactly representable in RR.

Can you please help me with this definition? In my understanding an exact representation has infinite precision (or another bit of information that makes it different from an inexact element). But everything in RR has finite precision(), even oo and NaN which by definition are special.

So, either no integers and rationals are in RR, or RR elements must carry an exact flag that is set to False when an operation with an inexact element is performed, or, most probably, I'm missing something about how to spot elements with exact representation in RR/CC. I cannot imagine you would mean 1 is exactly representable in RR because 1 == RR(1), since that would obviously be circular with your definition.

pjbruin · 2015-03-21T07:57:30Z

comment:25

Replying to @rwst:

Replying to @pjbruin:

Hence I am tending towards the opinion that if x is some exact element, then bool(RR(x) == x) should return True if and only if x is exactly representable in RR.

Can you please help me with this definition? In my understanding an exact representation has infinite precision (or another bit of information that makes it different from an inexact element). But everything in RR has finite precision(), even oo and NaN which by definition are special.

Elements of RR (except infinities and NaN) are certain fractions whose numerator and denominator are bounded and whose denominator is a power of 2. Hence 1/16 is representable in RR, but 1/3 is not.

Different people have different viewpoints about this, but above I was thinking of elements of RR (again except infinities and NaN) as exactly representing a certain subset of (mathematical) real numbers. The interpretation "an element of RR is a real number with some error" is just interpretation. One could say that what is inexact about RR are not the elements themselves, but the operations; rounding has to take place because this subset is not closed under the usual operations.

A related sage-devel discussion: https://groups.google.com/forum/#!topic/sage-devel/1gPkeL_X5dw

rwst · 2015-03-21T08:52:18Z

comment:26

Replying to @pjbruin:

Elements of RR (except infinities and NaN) are certain fractions whose numerator and denominator are bounded and whose denominator is a power of 2. Hence 1/16 is representable in RR, but 1/3 is not.

So this would be similar (equal?) to what was recently said about RIF in https://groups.google.com/d/topic/sage-devel/9MHkb4cUUHM/discussion

Different people have different viewpoints about this, but above I was thinking of elements of RR (again except infinities and NaN) as exactly representing a certain subset of (mathematical) real numbers.

So, pragmatically, every inexact ring needs a method is_exactly_representable(item) in order to determine usefully if bool(item == RR(item)).

rwst · 2015-03-21T09:01:03Z

comment:27

So, pragmatically, every inexact ring needs a method is_exactly_representable(item) in order to determine usefully if bool(item == RR(item)).

Note that at the moment 1/5 == RR(1/5).

rwst · 2015-03-21T09:53:56Z

comment:28

Replying to @rwst:

So, pragmatically, every inexact ring needs a method is_exactly_representable(item) in order to determine usefully if bool(item == RR(item)).

Ah okay, RIF(1/5).is_exact is implemented so we need the same method elsewhere.

videlec · 2015-03-21T10:01:16Z

comment:29

Replying to @rwst:

Replying to @rwst:

So, pragmatically, every inexact ring needs a method is_exactly_representable(item) in order to determine usefully if bool(item == RR(item)).

Ah okay, RIF(1/5).is_exact is implemented so we need the same method elsewhere.

This method makes sense in RIF but not in RR: a number is exact in RIF if the associated interval is a singleton. This is very different from the proposition of having a method RR.is_exactly_representable(...).

I do not like the fact that we treat RR as a subset of the real numbers and at the same time a field

sage: RR.is_field()
True
sage: RR in Fields()
True

Vincent

rwst · 2015-03-21T14:04:58Z

comment:30

Replying to @videlec:

I do not like the fact that we treat RR as a subset of the real numbers and at the same time a field

What would be consequences of removing it from the set (apart from documentation issues)?

rwst · 2015-05-24T16:03:26Z

comment:31

Ping?

rwst · 2015-09-20T13:47:56Z

comment:32

This issue has come up again with working on #19040. This is intolerable. I will have to have a new attempt at this, maybe I understand the arguments better now.

rwst · 2015-09-20T15:46:38Z

Changed branch from u/rws/17984 to u/rws/17984-1

rwst · 2015-09-20T15:48:25Z

Dependencies: #19040

rwst · 2015-09-20T15:48:25Z

comment:34

The solution is quite simple with #19040 (which is no completely here yet).

New commits:

`1edc690`	`17984: simplify contains, dependent on 19040`

rwst · 2015-09-20T15:48:25Z

Changed commit from 544450e to 1edc690

rwst · 2015-09-21T13:48:06Z

Changed commit from 1edc690 to none

rwst · 2015-09-21T13:48:06Z

Changed branch from u/rws/17984-1 to none

rwst · 2018-01-15T08:36:56Z

comment:38

Unexpectedly it looks like we get a much more correct fix whenever we'll have #24456.

rwst added this to the sage-6.6 milestone Mar 18, 2015

rwst added c: basic arithmetic labels Mar 18, 2015

rwst added the s: needs review label Mar 18, 2015

jdemeyer added s: needs work and removed s: needs review labels Mar 18, 2015

This comment has been minimized.

Sign in to view

rwst added s: needs review and removed s: needs work labels Mar 18, 2015

This comment has been minimized.

Sign in to view

rwst added s: needs info and removed s: needs review labels Mar 21, 2015

rwst changed the title ~~Dedicated RR.__contains__() and CC.__contains__()~~ fix Parent.__contains__ Mar 21, 2015

fix Parent.__contains__ #17984

fix Parent.__contains__ #17984

Comments

rwst commented Mar 18, 2015

rwst commented Mar 18, 2015

rwst commented Mar 18, 2015

rwst commented Mar 18, 2015

rwst commented Mar 18, 2015

jdemeyer commented Mar 18, 2015

jdemeyer commented Mar 18, 2015

jdemeyer commented Mar 18, 2015

jdemeyer commented Mar 18, 2015

jdemeyer commented Mar 18, 2015

rwst commented Mar 18, 2015

This comment has been minimized.

rwst commented Mar 18, 2015

rwst commented Mar 18, 2015

sagetrac-git mannequin commented Mar 18, 2015

sagetrac-git mannequin commented Mar 18, 2015

sagetrac-git mannequin commented Mar 18, 2015

sagetrac-git mannequin commented Mar 18, 2015

jdemeyer commented Mar 18, 2015

jdemeyer commented Mar 18, 2015

This comment has been minimized.

rwst commented Mar 18, 2015

rwst commented Mar 21, 2015

pjbruin commented Mar 21, 2015

rwst commented Mar 21, 2015

rwst commented Mar 21, 2015

rwst commented Mar 21, 2015

videlec commented Mar 21, 2015

rwst commented Mar 21, 2015

This comment has been minimized.

rwst commented May 24, 2015

rwst commented Sep 20, 2015

rwst commented Sep 20, 2015

rwst commented Sep 20, 2015

rwst commented Sep 20, 2015

rwst commented Sep 20, 2015

rwst commented Sep 21, 2015

rwst commented Sep 21, 2015

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rwst commented Jan 15, 2018

fix Parent.contains #17984

fix Parent.contains #17984