Opened 9 years ago
Closed 22 months ago
#15114 closed defect (fixed)
disallow dangerous coercions to RIF
Reported by:  Marco Streng  Owned by:  

Priority:  major  Milestone:  sage9.3 
Component:  coercion  Keywords:  RIF RR interval coercion 
Cc:  Vincent Delecroix, Marc Mezzarobba, Daniel Krenn, Clemens Heuberger, Ben Hutz, John Cremona, Peter Bruin  Merged in:  
Authors:  Marc Mezzarobba  Reviewers:  Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  3c7644f (Commits, GitHub, GitLab)  Commit:  3c7644f483a64f33159a1632e3b2426436a9cf50 
Dependencies:  Stopgaps: 
Description (last modified by )
Remove the following coercions because they make interval arithmetic much less trustworthy.
RealField
>RealIntervalField
float
>ComplexIntervalField
SR
>ComplexIntervalField
For example, one could easily do the following by accident.
sage: iv = 1 + 2^(55) + 0. + RIF(1) sage: iv.lower(), iv.upper() (2.00000000000000, 2.00000000000000)
See also this topic on sagedevel
Already removed in other tickets:
ComplexField
>ComplexIntervalField
Change History (54)
comment:1 Changed 9 years ago by
Cc:  Vincent Delecroix added 

Description:  modified (diff) 
comment:2 Changed 9 years ago by
Cc:  Marc Mezzarobba added 

comment:3 Changed 9 years ago by
Milestone:  sage6.1 → sage6.2 

comment:4 followup: 5 Changed 9 years ago by
comment:5 Changed 9 years ago by
Replying to tcoffee:
I think this change will cause many more problems than it solves.
Thanks for the comments. I mentioned them in the sagedevel thread where the original discussion took place. Let's move the discussion there.
comment:6 Changed 9 years ago by
Milestone:  sage6.2 → sage6.3 

comment:7 Changed 8 years ago by
Milestone:  sage6.3 → sage6.4 

comment:8 Changed 6 years ago by
Cc:  Daniel Krenn added 

comment:9 Changed 6 years ago by
Branch:  → u/dkrenn/removecoerceRRRIF 

comment:10 followup: 11 Changed 6 years ago by
Commit:  → b30ebc322cd9f6ef887a0e2b123b65b98e9fd1c0 

Does your branch pass the tests? I tried doing something similar a while ago (see u/mmezzarobba/wip/coerce_RR_RIF
), but I had to make additional changes, and, more importantly, I concluded that this should wait for #22029 if we don't want comparisons between floatingpoint numbers and intervals to give very misleading results.
New commits:
b30ebc3  disallow coercions from RR to RIF

comment:11 Changed 6 years ago by
Replying to mmezzarobba:
Does your branch pass the tests? I tried doing something similar a while ago (see
u/mmezzarobba/wip/coerce_RR_RIF
), but I had to make additional changes, and, more importantly, I concluded that this should wait for #22029 if we don't want comparisons between floatingpoint numbers and intervals to give very misleading results.
I am cleaning up my SageMathversions and found this partial work. As no branch was attached to this ticket, I've uploaded it. So it will, for sure, need some work.
comment:12 followup: 13 Changed 5 years ago by
Replying to mstreng:
For example, one could easily do the following by accident.
sage: iv = 1 + 2^(55) + 0. + RIF(1) sage: iv.lower(), iv.upper() (2.00000000000000, 2.00000000000000)
What's wrong with this? I honestly don't really see a problem with coercion RR
> RIF
.
comment:13 followups: 14 19 Changed 5 years ago by
Replying to jdemeyer:
Replying to mstreng:
For example, one could easily do the following by accident.
sage: iv = 1 + 2^(55) + 0. + RIF(1) sage: iv.lower(), iv.upper() (2.00000000000000, 2.00000000000000)What's wrong with this? I honestly don't really see a problem with coercion
RR
>RIF
.
With RIF
you always have guaranteed results, with RR
you do not. RIF
should be as safe as possible to ensure that you actually have proven results. Just compare
sage: r1 = RIF(1/3) + (2.0).cos() sage: r2 = RIF(1/3) + RIF(2).cos() sage: r1.endpoints() (0.0828135032138091, 0.0828135032138090) sage: r2.endpoints() (0.0828135032138091, 0.0828135032138089)
For me it is similar to the way coercions are implemented between RealField
. It goes from more precision to less precision so that you have no artificial big precision in your result.
In conclusion: if any coercion, it should be the other way around (by taking the center).
comment:14 followups: 15 21 Changed 5 years ago by
I think a lot depends on how you model mathematically elements of interval fields. There are two ways to interpret them: either as real numbers with some uncertainty or actually as intervals of real numbers.
In the "interval" model, the coercion RR > RIF makes sense: you identify a real number with a singleton, which is a special case of an interval. On the other hand, the "choose the middle point map" RIF > RR is neither natural (the middle point is an arbitrary choice) nor a morphism, which are two requirements for a coercion.
In the "real number with uncertainty" model, you could see RIF > RR as the forgetful map which keeps the real number but forgets about the uncertainty. In that case, this map is more natural than the opposite.
For me, it is pretty clear that MPFI is really meant to model intervals while arb models real numbers with uncertainty.
comment:15 followup: 16 Changed 5 years ago by
Replying to jdemeyer:
For me, it is pretty clear that MPFI is really meant to model intervals
I sort of agree, but still, the key property is that operations on MPFI intervals return overapproximations of the set of possible results (in particular, they round the endpoints of the results in the correct direction), and having a coercion from plain floatingpoint numbers would effectively break that.
What would make sense to me if you want ”intervals of real numbers” with a coercion from the corresponding real numbers themselves would be a separate parent Intervals(R)
explicitly parametrized by the parent R
= RR
, QQ
, SR
... where the endpoints live.
comment:16 followup: 17 Changed 5 years ago by
Replying to mmezzarobba:
What would make sense to me if you want ”intervals of real numbers”
RealIntervalField
is exactly that for RR
with a coercion from the corresponding real numbers themselves
So, is this an argument in favor of the coercion RR
> RIF
?
comment:17 followup: 18 Changed 5 years ago by
Replying to jdemeyer:
Replying to mmezzarobba:
What would make sense to me if you want ”intervals of real numbers”
RealIntervalField
is exactly that forRR
No: operations in RIF
round the upper bound of their result up and the lower bound down. They don't just defer to operations in RR
.
with a coercion from the corresponding real numbers themselves
So, is this an argument in favor of the coercion
RR
>RIF
?
Certainly not.
comment:18 Changed 5 years ago by
Replying to mmezzarobba:
Replying to jdemeyer:
Replying to mmezzarobba:
What would make sense to me if you want ”intervals of real numbers”
RealIntervalField
is exactly that forRR
No: operations in
RIF
round the upper bound of their result up and the lower bound down.
Yes, obviously because that's the right thing to do.
They don't just defer to operations in
RR
.
Technically not, but that is just an implementation detail. RIF
is the set of intervals over RR
. Whether it's implemented by operations in RR
or by a separate library (MPFI in this case) doesn't really matter.
comment:19 Changed 5 years ago by
Replying to vdelecroix:
What's wrong with this? I honestly don't really see a problem with coercion
RR
>RIF
.With
RIF
you always have guaranteed results, withRR
you do not.RIF
should be as safe as possible to ensure that you actually have proven results. [...] For me it is similar to the way coercions are implemented betweenRealField
. It goes from more precision to less precision so that you have no artificial big precision in your result.In conclusion: if any coercion, it should be the other way around (by taking the center).
Huge +1. (Having reliable numerical computations is a (the) major application of interval arithmetic.)
comment:20 Changed 5 years ago by
Cc:  Clemens Heuberger added 

comment:21 followup: 22 Changed 5 years ago by
Cc:  Clemens Heuberger removed 

Replying to jdemeyer:
For me, it is pretty clear that MPFI is really meant to model intervals
It is not to me. I found various texts (by MPFI developers) that motivate why MPFI exists and they all start by saying things about "guaranteeing results" and "certified enclosures" for floating point real arithmetic. And if an automatic silent coercion mixes a guaranteed result with a nonguaranteed result, this is bad.
People prove theorems using RealIntervalField
as "real numbers that are guaranteed to be in a certain interval", and I would personally trust such theorems much less if they use software that allows automatic accidental coercions from RR to RIF. The reason: a mistake or bug could introduce a (nonguaranteedcorrect) element of RR and the coercion will silently add it to your RIF element, which now could have an incorrect error bound.
I did not know that people also use the outwardrounding RIF for "arithmetic with intervals" without caring about guaranteed inclusion. I understand that for them it is convenient that the following works:
sage: I = RIF(pi, 2*pi) sage: p = RR(pi) sage: I + p 1.?e1
to get the interval [5, 8]. However, it is hardly any extra work to do the direct thing that the coercion model currently does for you, which is:
sage: I + RIF(p) 1.?e1
And we can also implement additional functions, e.g.
sage: I.translate(p) sage: I.add(p)
So I am very much in favour of removing automatic coercions between RR and RIF. Perhaps with a DeprecationWarning
if possible.
comment:22 followup: 25 Changed 5 years ago by
Replying to mstreng:
So I am very much in favour of removing automatic coercions between RR and RIF. Perhaps with a
DeprecationWarning
if possible.
If it is considered a bug, then no deprecation is needed...
comment:23 Changed 5 years ago by
Cc:  Clemens Heuberger added 

comment:24 Changed 5 years ago by
The problem with this ticket is that various places in Sage use this coercion. In particular QQbar
:
********************************************************************** File "src/sage/rings/number_field/number_field.py", line 1513, in sage.rings.number_field.number_field.NumberField_generic.construction Failed example: K.<a> = NumberField(x^35,embedding=0) Exception raised: Traceback (most recent call last): File "/usr/local/src/sageconfig/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 517, in _run self.compile_and_execute(example, compiler, test.globs) File "/usr/local/src/sageconfig/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 920, in compile_and_execute exec(compiled, globs) File "<doctest sage.rings.number_field.number_field.NumberField_generic.construction[14]>", line 1, in <module> K = NumberField(x**Integer(3)Integer(5),embedding=Integer(0), names=('a',)); (a,) = K._first_ngens(1) File "/usr/local/src/sageconfig/local/lib/python2.7/sitepackages/sage/rings/number_field/number_field.py", line 529, in NumberField return NumberField_version2(polynomial=polynomial, name=name, check=check, embedding=embedding, latex_name=latex_name, assume_disc_small=assume_disc_small, maximize_at_primes=maximize_at_primes, structure=structure) File "sage/structure/factory.pyx", line 364, in sage.structure.factory.UniqueFactory.__call__ (build/cythonized/sage/structure/factory.c:1964) return self.get_object(version, key, kwds) File "sage/structure/factory.pyx", line 403, in sage.structure.factory.UniqueFactory.get_object (build/cythonized/sage/structure/factory.c:2197) cache_key = _cache_key(cache_key) File "sage/misc/cachefunc.pyx", line 642, in sage.misc.cachefunc.cache_key (build/cythonized/sage/misc/cachefunc.c:3189) o = cache_key_unhashable(o) File "sage/misc/cachefunc.pyx", line 651, in sage.misc.cachefunc.cache_key_unhashable (build/cythonized/sage/misc/cachefunc.c:3503) return tuple(cache_key(item) for item in o) File "sage/misc/cachefunc.pyx", line 651, in genexpr (build/cythonized/sage/misc/cachefunc.c:3398) return tuple(cache_key(item) for item in o) File "sage/misc/cachefunc.pyx", line 642, in sage.misc.cachefunc.cache_key (build/cythonized/sage/misc/cachefunc.c:3189) o = cache_key_unhashable(o) File "sage/misc/cachefunc.pyx", line 651, in sage.misc.cachefunc.cache_key_unhashable (build/cythonized/sage/misc/cachefunc.c:3503) return tuple(cache_key(item) for item in o) File "sage/misc/cachefunc.pyx", line 651, in genexpr (build/cythonized/sage/misc/cachefunc.c:3398) return tuple(cache_key(item) for item in o) File "sage/misc/cachefunc.pyx", line 642, in sage.misc.cachefunc.cache_key (build/cythonized/sage/misc/cachefunc.c:3189) o = cache_key_unhashable(o) File "sage/misc/cachefunc.pyx", line 653, in sage.misc.cachefunc.cache_key_unhashable (build/cythonized/sage/misc/cachefunc.c:3558) k = o._cache_key() File "sage/structure/element.pyx", line 1059, in sage.structure.element.Element._cache_key (build/cythonized/sage/structure/element.c:9948) return(self.parent(),str(self)) File "sage/structure/sage_object.pyx", line 237, in sage.structure.sage_object.SageObject.__repr__ (build/cythonized/sage/structure/sage_object.c:2778) result = repr_func() File "sage/rings/real_lazy.pyx", line 748, in sage.rings.real_lazy.LazyFieldElement._repr_ (build/cythonized/sage/rings/real_lazy.c:9768) return str(self.approx()) File "sage/rings/real_lazy.pyx", line 772, in sage.rings.real_lazy.LazyFieldElement.approx (build/cythonized/sage/rings/real_lazy.c:9863) return self.eval(self._parent.interval_field()) File "sage/rings/real_lazy.pyx", line 1640, in sage.rings.real_lazy.LazyAlgebraic.eval (build/cythonized/sage/rings/real_lazy.c:18796) roots = self._poly.roots(ring = AA if isinstance(self._parent, RealLazyField_class) else QQbar) File "sage/rings/polynomial/polynomial_element.pyx", line 7540, in sage.rings.polynomial.polynomial_element.Polynomial.roots (build/cythonized/sage/rings/polynomial/polynomial_element.c:68834) rts = real_roots(self, retval='algebraic_real') File "sage/rings/polynomial/real_roots.pyx", line 4155, in sage.rings.polynomial.real_roots.real_roots (build/cythonized/sage/rings/polynomial/real_roots.c:49563) return [(AA.polynomial_root(r[1], r[0]), r[2]) for r in intv_roots] File "/usr/local/src/sageconfig/local/lib/python2.7/sitepackages/sage/rings/qqbar.py", line 997, in polynomial_root return AlgebraicReal(ANRoot(poly, interval, multiplicity)) File "/usr/local/src/sageconfig/local/lib/python2.7/sitepackages/sage/rings/qqbar.py", line 5830, in __init__ self._interval = self.refine_interval(interval, 64) File "/usr/local/src/sageconfig/local/lib/python2.7/sitepackages/sage/rings/qqbar.py", line 5992, in refine_interval return self._real_refine_interval(interval, prec) File "/usr/local/src/sageconfig/local/lib/python2.7/sitepackages/sage/rings/qqbar.py", line 6096, in _real_refine_interval new_range = uinfo['endpoint']  uinfo['value'] / slope File "sage/structure/element.pyx", line 1369, in sage.structure.element.Element.__sub__ (build/cythonized/sage/structure/element.c:11538) return coercion_model.bin_op(left, right, sub) File "sage/structure/coerce.pyx", line 1133, in sage.structure.coerce.CoercionModel_cache_maps.bin_op (build/cythonized/sage/structure/coerce.c:10655) raise bin_op_exception(op, x, y) TypeError: unsupported operand parent(s) for : 'Real Field with 64 bits of precision and rounding RNDU' and 'Real Interval Field with 64 bits of precision'
comment:25 Changed 5 years ago by
Replying to dkrenn:
If it is considered a bug, then no deprecation is needed...
It's documented behaviour so it's maybe a misfeature but not a bug.
Anyway, it seems that most people think that the coercion RR
> RIF
is a misfeature, so I'm willing to remove it in #24371. However, I don't want to deal with the fallout so if somebody wants to fix 24, please go ahead. It's hard to tell if that's the only issue because that one issue in QQbar
causes so many doctest failures.
comment:26 followups: 27 28 Changed 5 years ago by
To everybody who is supporting this ticket: please be aware that fixing it will lead to issues like #24410 for RIF
too.
comment:27 Changed 5 years ago by
comment:28 Changed 5 years ago by
Replying to jdemeyer:
To everybody who is supporting this ticket: please be aware that fixing it will lead to issues like #24410 for
RIF
too.
That what comment #10 above is about.
comment:29 Changed 4 years ago by
Jeroen: This is the ticket I was talking about at breakfast. I agree that there is an argument for keeping the coercion in place, but I think the cons outweigh the pros, and most people seem to agree. Also, I seem to remember that #21991 and perhaps a couple of other tickets were going to depend on this one, but I don't remember the details.
comment:31 Changed 4 years ago by
The link is not immediate. What happens is that
 before #22029, “interval < float” comparisons gave reasonable (though possibly nonrigorous) results thanks to the coercion that this ticket wants to remove,
 with the coercion removed but without #22029, they would silently return nonsense (compare by id),
 they can now (correctly, I'd argue) throw an error if we remove the coercion.
comment:32 Changed 2 years ago by
Branch:  u/dkrenn/removecoerceRRRIF → u/mmezzarobba/15114_RR_to_RIF 

Commit:  b30ebc322cd9f6ef887a0e2b123b65b98e9fd1c0 → d2e93d76e816c22f85684bca4b161610f37db4f3 
Description:  modified (diff) 
Milestone:  sage6.4 → sage9.3 
Summary:  disallow coercions from RR to RIF → disallow dangerous coercions to RIF 
New commits:
e587b14  #15114 remove coercions from RealField(n), RDF to RIF

a91e43c  #15114 remove coercions from SR, float to CIF

bb9663b  #15114 force choice of coercion maps from UCF to CC, CIF, ...

cb7e794  #15114 binary_form_reduce(): avoid mixing intervals and FP numbers

df6373a  #15114 fix doctest tolerance parsing after removal of RR → RIF coercion

a5488ac  #15114 minor fixes to elliptic_curves code and tests mixing RR with RIF

d2e93d7  #15114 misc small fixes after removal of dangerous coercions

comment:33 Changed 2 years ago by
Commit:  d2e93d76e816c22f85684bca4b161610f37db4f3 → 0aab184224fae9bf51e3f9a5cd6597712d4e977f 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
6b8232c  #15114 remove coercions from RR, RDF, float to RIF

c11b6a2  #15114 remove coercions from SR, float to CIF

6d7107e  #15114 force choice of coercion maps from UCF to CC, CIF, ...

7f7e3b3  #15114 binary_form_reduce(): avoid mixing intervals and FP numbers

9d30857  #15114 fix doctest tolerance parsing after removal of RR → RIF coercion

6cdc8d6  #15114 minor fixes to elliptic_curves code and tests mixing RR with RIF

0aab184  #15114 misc small fixes after removal of dangerous coercions

comment:34 Changed 2 years ago by
Commit:  0aab184224fae9bf51e3f9a5cd6597712d4e977f → 88e07641ede4a45b6983790d3e36cad5fb0d4885 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
b225ec4  #15114 binary_form_reduce: avoid mixing intervals and FP numbers

bd8ac6c  #15114 fix doctest tolerance parsing after removal of RR → RIF coercion

ab8ebca  #15114 minor fixes to elliptic_curves code and tests mixing RR with RIF

88e0764  #15114 misc small fixes after removal of dangerous coercions

comment:35 Changed 2 years ago by
Authors:  → Marc Mezzarobba 

Cc:  Ben Hutz John Cremona Peter Bruin added 
Status:  new → needs_review 
I could not test everything locally due to version issues with pari and giac, so the patchbot may still uncover problems, but this is starting to take shape, and comments are already welcome.
@bhutz: You may want to check the commit touching binary_form_reduce
. I don't think I broke anything, but the way intervals are used in the parts I had to modify looks a bit strange to me.
@cremona, @pbruin: Same remark regarding the changes to elliptic_curves.height
.
comment:36 Changed 2 years ago by
I see no problems with the minor code changes to elliptic curve heights, though I don't think that any of the affected functions was written by me.
comment:37 Changed 2 years ago by
I don't think I contributed much to this precise code either, but the changes certainly look fine to me.
comment:38 followup: 39 Changed 2 years ago by
Thank you both for your comments. John: indeed, git blame
did not tell the whole story. If I understand correctly, you committed the code but it was originally written by Robert Bradshaw.
comment:39 Changed 2 years ago by
Replying to mmezzarobba:
Thank you both for your comments. John: indeed,
git blame
did not tell the whole story. If I understand correctly, you committed the code but it was originally written by Robert Bradshaw.
That agrees with my memory. heegner.py was from his thesis, I think, and he rewrote the unpleasant complex interval stuff from a script I had in (I think) gp.
comment:40 Changed 2 years ago by
Commit:  88e07641ede4a45b6983790d3e36cad5fb0d4885 → 5dcf52cdfc108339d57a326d1c17a7957e34b170 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
5c463c8  #15114 remove coercions from RR, RDF, float to RIF

ccf7e2e  #15114 remove coercions from SR, float to CIF

c307736  #15114 force choice of coercion maps from UCF to CC, CIF, ...

97a48a2  #15114 binary_form_reduce: avoid mixing intervals and FP numbers

02308be  #15114 fix doctest tolerance parsing after removal of RR → RIF coercion

7e0d6d9  #15114 minor fixes to elliptic_curves code and tests mixing RR with RIF

5dcf52c  #15114 misc small fixes after removal of dangerous coercions

comment:41 followup: 42 Changed 2 years ago by
The changes in binary_form_reduce.py look fine to me. The use of .lower() and .upper() I expect should have been there in the first place and we've just never come across an example where being that careful with the errors mattered.
comment:42 Changed 2 years ago by
Replying to bhutz:
The changes in binary_form_reduce.py look fine to me. The use of .lower() and .upper() I expect should have been there in the first place and we've just never come across an example where being that careful with the errors mattered.
Thank you.
All tests pass now, reviewers welcome!
comment:43 followup: 44 Changed 2 years ago by
This is great. I am positively surprised for seeing only few changes for making this happen!
The only trouble I see is that it might break user code without deprecation notice.
comment:44 followup: 45 Changed 2 years ago by
Replying to vdelecroix:
This is great. I am positively surprised for seeing only few changes for making this happen!
The switch to Python 3 (and, I think, also the change to make i
an element of ℚ[i] that you recently reviewed) went a long way toward solving the worst issues I found the first time I tried. But it is good news in any case!
The only trouble I see is that it might break user code without deprecation notice.
I agree. But I don't know what to to to ease the transition. Do you see a way to display a deprecation warning when a coercion map is used while keeping the corresponding conversion map working?
That being said, between the issues it indirectly causes and the inconsistency with other interval fields, I would almost call the existence of this coercion a bug.
comment:45 followup: 46 Changed 2 years ago by
Replying to mmezzarobba:
Replying to vdelecroix:
This is great. I am positively surprised for seeing only few changes for making this happen!
The switch to Python 3 (and, I think, also the change to make
i
an element of ℚ[i] that you recently reviewed) went a long way toward solving the worst issues I found the first time I tried. But it is good news in any case!The only trouble I see is that it might break user code without deprecation notice.
I agree. But I don't know what to to to ease the transition. Do you see a way to display a deprecation warning when a coercion map is used while keeping the corresponding conversion map working?
One possibility would be to support the operations (with a warning) without having the coercions. One cumbersome way to do this is to implement custom __add__
, __mul__
, etc
class Interval: def __add__(self, other): if self is an interval and other a floating point: warn("don't do that") return old_behavior(self, other) else: return Element.__add__(self, other)
But I don't see a simple way to do it in order to also handle functorial constructions such as vectors or polynomials. Though, given the changes you had to do in the doctests, I think it makes sense to only consider scalars.
That being said, between the issues it indirectly causes and the inconsistency with other interval fields, I would almost call the existence of this coercion a bug.
Agreed, but https://xkcd.com/1172/
comment:46 followup: 48 Changed 2 years ago by
Replying to vdelecroix:
One possibility would be to support the operations (with a warning) without having the coercions. One cumbersome way to do this is to implement custom
__add__
Unless we do it for floatingpoint types too (which would be a bit invasive), it will work for interval + float
but float + interval
will still fail.
comment:47 Changed 2 years ago by
Commit:  5dcf52cdfc108339d57a326d1c17a7957e34b170 → ddab8474d3a942a18188515eae80175d07ee1c5a 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
315ddc2  #15114 remove coercions from RR, RDF, float to RIF

3cc998a  #15114 remove coercions from SR, float to CIF

a51cfe0  #15114 force choice of coercion maps from UCF to CC, CIF, ...

ccd98a7  #15114 binary_form_reduce: avoid mixing intervals and FP numbers

ca84412  #15114 fix doctest tolerance parsing after removal of RR → RIF coercion

584d6e4  #15114 minor fixes to elliptic_curves code and tests mixing RR with RIF

e1aabd6  #15114 misc small fixes after removal of dangerous coercions

ddab847  #15114 restore basic functionality, with deprecation warnings

comment:48 Changed 2 years ago by
Replying to mmezzarobba:
Replying to vdelecroix:
One possibility would be to support the operations (with a warning) without having the coercions. One cumbersome way to do this is to implement custom
__add__
Unless we do it for floatingpoint types too (which would be a bit invasive), it will work for
interval + float
butfloat + interval
will still fail.
Ok, for lack of a better option, I have done that—for RealNumber
s only. It was... painful.
comment:49 Changed 23 months ago by
Commit:  ddab8474d3a942a18188515eae80175d07ee1c5a → 3c7644f483a64f33159a1632e3b2426436a9cf50 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
674b4f7  #15114 remove coercions from RR, RDF, float to RIF

9abccbb  #15114 remove coercions from SR, float to CIF

3be1c0f  #15114 force choice of coercion maps from UCF to CC, CIF, ...

5fce3b6  #15114 binary_form_reduce: avoid mixing intervals and FP numbers

ee5fd27  #15114 fix doctest tolerance parsing after removal of RR → RIF coercion

ba66ce3  #15114 minor fixes to elliptic_curves code and tests mixing RR with RIF

9f47f4c  #15114 misc small fixes after removal of dangerous coercions

3c7644f  #15114 restore basic functionality, with deprecation warnings

comment:51 Changed 23 months ago by
Vincent, do you think you will finish the review, or should I look for someone willing to take over?
comment:52 Changed 23 months ago by
Reviewers:  → Vincent Delecroix 

Status:  needs_review → positive_review 
comment:54 Changed 22 months ago by
Branch:  u/mmezzarobba/15114_RR_to_RIF → 3c7644f483a64f33159a1632e3b2426436a9cf50 

Resolution:  → fixed 
Status:  positive_review → closed 
I think this change will cause many more problems than it solves.
In practice: I expect this will break a lot of existing code. I often use the RR > RIF coercion intentionally, because it makes it more convenient to do exactly what I want. For instance:
In theory: I think the motivation is flawed. The change was proposed to avoid accidental coercion of exact ring values to RIF via potentially inexact RR. But the problem in this case is the first step, not the second, and reflects the need for a direct coercion from the exact ring to RIF. In such a case, the lack of such a coercion is the bug. Whereas each value in RR *does* have a valid coercion to RIF, and the lack of this coercion would also be a bug.
Realistically, I think anyone who is actually
is going to be sufficiently aware of the potential problem that they will handle/check the conversion explicitly. Better to expect this than to inconvenience many users who use this coercion in far less arcane circumstances.