Sage: Ticket #14485: Get rid of the bogus coercion from SR to QQbar
https://trac.sagemath.org/ticket/14485
<p>
Original issue: there is no problem buildind a polynomial over AA:
</p>
<pre class="wiki">sage: R = AA[x]
sage: P = R(x^6+x^5+x^4-x^3+x^2-x+2/5)
</pre><p>
But the same operation does not work on QQbar:
</p>
<pre class="wiki">sage: S = QQbar[x]
sage: Q = S(x^6+x^5+x^4-x^3+x^2-x+2/5)
Traceback (most recent call last)
...
NotImplementedError: symbol
</pre><p>
Hovewer, this workaround works:
</p>
<pre class="wiki">sage: Q = P.change_ring(QQbar)
</pre><p>
Moreover the following works :
</p>
<pre class="wiki">sage: S.<x> = PolynomialRing(QQbar,'x')
sage: Q = S(x^6+x^5+x^4-x^3+x^2-x+2/5)
sage: S == QQbar[x]
True
</pre><p>
Weird isn't it ?
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/14485
Trac 1.1.6tmonteilWed, 24 Apr 2013 22:10:52 GMTdescription changed
https://trac.sagemath.org/ticket/14485#comment:1
https://trac.sagemath.org/ticket/14485#comment:1
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/14485?action=diff&version=1">diff</a>)
</li>
</ul>
TicketnbruinWed, 24 Apr 2013 22:42:36 GMT
https://trac.sagemath.org/ticket/14485#comment:2
https://trac.sagemath.org/ticket/14485#comment:2
<p>
The weirdness is really just limited to the difference in behaviour between these:
</p>
<pre class="wiki">sage: AA['x'](var('x'))
x
sage: QQbar['x'](var('x'))
NotImplementedError: symbol
</pre><p>
The thing that's failing in the latter case is
</p>
<pre class="wiki">QQbar._element_constructor_(x)
</pre><p>
and
</p>
<pre class="wiki">AA._element_constructor_(x)
</pre><p>
fails in exactly the same way. That suggest that in the case of <code>AA['x']</code> something is tried before resorting to the thing that fails for <code>QQbar['x']</code>.
</p>
<p>
For instance,
</p>
<pre class="wiki">AA['y'](var('x'))
QQbar['y'](var('x'))
</pre><p>
both (rightly) fail,but with significantly different tracebacks. This might give some indication of what is tried for <code>AA</code> but not for <code>QQbar</code>.
</p>
TicketnbruinWed, 24 Apr 2013 23:19:34 GMT
https://trac.sagemath.org/ticket/14485#comment:3
https://trac.sagemath.org/ticket/14485#comment:3
<p>
The thing that goes wrong is the following:
</p>
<pre class="wiki">sage: S=QQbar['x']
sage: S.base_ring().has_coerce_map_from(SR)
True
</pre><p>
Once this is found, it's clear how an element of <code>SR</code> should be converted into <code>QQbar['x']</code>: <code>SR</code> already coerces into <code>QQbar</code> itself, so we should just convert the element into a "constant polynomial".
</p>
<p>
Except, of course, that <code>SR</code> doesn't really coerce into <code>QQbar</code>: Only "constant" expressions might (and even then, of course <code>pi</code> doesn't), and <code>x</code> of course isn't one of those. So the proper solution is probably to NOT install a coercion from SR to QQbar, because it isn't a coercion anyway. It's only a partial map. Doing so is probably going to be painful for something else.
</p>
<p>
This problem doesn't arise for <code>AA</code> because it's pretty clear that not even algebraic elements fit in <code>AA</code>, so no coercion is installed.
</p>
TicketnbruinWed, 24 Apr 2013 23:31:42 GMTattachment set
https://trac.sagemath.org/ticket/14485
https://trac.sagemath.org/ticket/14485
<ul>
<li><strong>attachment</strong>
set to <em>trac_14485.patch</em>
</li>
</ul>
<p>
Don't pretend there is a coercion from the symbolic ring into QQbar
</p>
TicketnbruinWed, 24 Apr 2013 23:37:29 GMTpriority changed
https://trac.sagemath.org/ticket/14485#comment:4
https://trac.sagemath.org/ticket/14485#comment:4
<ul>
<li><strong>priority</strong>
changed from <em>major</em> to <em>minor</em>
</li>
</ul>
<p>
OK, solving a ticket by deleting two lines is just too appealing to pass by. Of course, there is are all kinds of things that fail this way, e.g.
</p>
<pre class="wiki">QQbar(1)+sqrt(2)
</pre><p>
doesn't work anymore, but <code>QQbar(1)+QQbar(sqrt(2))</code> of course still does.
</p>
TicketjdemeyerTue, 13 Aug 2013 15:35:53 GMTmilestone changed
https://trac.sagemath.org/ticket/14485#comment:5
https://trac.sagemath.org/ticket/14485#comment:5
<ul>
<li><strong>milestone</strong>
changed from <em>sage-5.11</em> to <em>sage-5.12</em>
</li>
</ul>
TicketmmezzarobbaSat, 28 Dec 2013 14:41:58 GMTcc set
https://trac.sagemath.org/ticket/14485#comment:6
https://trac.sagemath.org/ticket/14485#comment:6
<ul>
<li><strong>cc</strong>
<em>burcin</em> added
</li>
</ul>
<p>
I was going to report another problem caused by the bogus coercion from <code>SR</code>:
</p>
<pre class="wiki">sage: CC.has_coerce_map_from(QQbar)
True
sage: QQbar.has_coerce_map_from(SR)
True
sage: CC.has_coerce_map_from(SR)
False
</pre><p>
So I'm all in favor of removing it, even if it breaks stuff that should never have worked in the first place.
</p>
<p>
We might want to put (or actually put back?) a coercion in the other direction, since symbolic expressions can embed elements of <code>QQbar</code>. But <code>SR</code> has coerce maps from <code>RR</code>, <code>CC</code>, <code>RLF</code>, etc., which yield other coercions from <code>QQbar</code> by composition. Can we consider that all these maps are the same because they only differ in the "precision" of the result? Or should all coercions from inexact rings to <code>QQbar</code> be removed? (Note that the coercion from <code>QQ</code> to <code>SR</code> has exactly the same problem.)
</p>
Ticketvbraun_spamThu, 30 Jan 2014 21:20:52 GMTmilestone changed
https://trac.sagemath.org/ticket/14485#comment:7
https://trac.sagemath.org/ticket/14485#comment:7
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.1</em> to <em>sage-6.2</em>
</li>
</ul>
Ticketvbraun_spamTue, 06 May 2014 15:20:58 GMTmilestone changed
https://trac.sagemath.org/ticket/14485#comment:8
https://trac.sagemath.org/ticket/14485#comment:8
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.2</em> to <em>sage-6.3</em>
</li>
</ul>
Ticketvbraun_spamSun, 10 Aug 2014 16:51:03 GMTmilestone changed
https://trac.sagemath.org/ticket/14485#comment:9
https://trac.sagemath.org/ticket/14485#comment:9
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.3</em> to <em>sage-6.4</em>
</li>
</ul>
TicketmmezzarobbaFri, 25 Mar 2016 12:13:36 GMTstatus changed; commit, branch, author set
https://trac.sagemath.org/ticket/14485#comment:10
https://trac.sagemath.org/ticket/14485#comment:10
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>commit</strong>
set to <em>8b0a847cbf2aa656e6a2fd8c4e0a383df2ae68bc</em>
</li>
<li><strong>branch</strong>
set to <em>u/mmezzarobba/14485-QQbartoSR</em>
</li>
<li><strong>author</strong>
set to <em>Marc Mezzarobba</em>
</li>
</ul>
<p>
As it turns out, it is possible to remove the coercion from <code>SR</code> to <code>QQbar</code> without breaking too many things. Moreover, most of the workarounds I had to add are there to deal with the case of <code>I</code> and will no longer be necessary once <a class="new ticket" href="https://trac.sagemath.org/ticket/5355" title="enhancement: QQbar should have a coercion from number fields with embedding (new)">#5355</a> (or <a class="new ticket" href="https://trac.sagemath.org/ticket/12715" title="defect: Number field embeddings should go via AA and QQbar (new)">#12715</a>) and/or <a class="closed ticket" href="https://trac.sagemath.org/ticket/18036" title="defect: I.parent() should not be the symbolic ring (closed: fixed)">#18036</a> are fixed (but I suggest we <em>dont't</em> wait fix this to happen).
</p>
<hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=8f85af166614455f24e255332995c7db003501db"><span class="icon"></span>8f85af1</a></td><td><code>Remove the bogus coercion from SR to QQbar</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=8b0a847cbf2aa656e6a2fd8c4e0a383df2ae68bc"><span class="icon"></span>8b0a847</a></td><td><code>#14485 regression test</code>
</td></tr></table>
TicketgitFri, 25 Mar 2016 12:18:41 GMTcommit changed
https://trac.sagemath.org/ticket/14485#comment:11
https://trac.sagemath.org/ticket/14485#comment:11
<ul>
<li><strong>commit</strong>
changed from <em>8b0a847cbf2aa656e6a2fd8c4e0a383df2ae68bc</em> to <em>9f5b44b3b5e431a92c77af7c52f4984c5c199a49</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=9f5b44b3b5e431a92c77af7c52f4984c5c199a49"><span class="icon"></span>9f5b44b</a></td><td><code>#14485 a few additional tests</code>
</td></tr></table>
TicketjdemeyerFri, 25 Mar 2016 12:37:47 GMTsummary changed
https://trac.sagemath.org/ticket/14485#comment:12
https://trac.sagemath.org/ticket/14485#comment:12
<ul>
<li><strong>summary</strong>
changed from <em>Creation of a polynomial over QQbar</em> to <em>Converting a symbolic polynomial to a polynomial over QQbar</em>
</li>
</ul>
TicketmmezzarobbaFri, 25 Mar 2016 16:01:45 GMTdescription, summary changed
https://trac.sagemath.org/ticket/14485#comment:13
https://trac.sagemath.org/ticket/14485#comment:13
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/14485?action=diff&version=13">diff</a>)
</li>
<li><strong>summary</strong>
changed from <em>Converting a symbolic polynomial to a polynomial over QQbar</em> to <em>Get rid of the bogus coercion from SR to QQbar</em>
</li>
</ul>
TicketrwsSat, 26 Mar 2016 09:00:39 GMT
https://trac.sagemath.org/ticket/14485#comment:14
https://trac.sagemath.org/ticket/14485#comment:14
<p>
Patchbot: <code>sage -t --long src/sage/graphs/matchpoly.pyx # 1 doctest failed</code>
</p>
TicketmmezzarobbaSat, 26 Mar 2016 11:52:59 GMT
https://trac.sagemath.org/ticket/14485#comment:15
https://trac.sagemath.org/ticket/14485#comment:15
<p>
Thanks for the notice. It works here, as well as with the other patchbots. I have no idea at all what is going on.
</p>
<p>
Jeroen, that <code>sage4</code> is one of your patchbots, isn't it? Would you mind having a quick look at the failing test?
</p>
TicketvdelecroixTue, 05 Apr 2016 17:24:03 GMTstatus changed
https://trac.sagemath.org/ticket/14485#comment:16
https://trac.sagemath.org/ticket/14485#comment:16
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
You should get rid of the function <code>_late_import</code> since it is not used anymore. And it would be better to simplify the code of <code>_coerce_map_from_</code> as
</p>
<pre class="wiki">return from_par == ZZ or from_par == QQ or from_par == int or \
from_par == long or from_par == AA or from_par == QQbar
</pre><p>
Moreover, I am not sure if coercion from <code>int/long</code> is necessary here since a conversion would exists through <code>ZZ</code>.
</p>
TicketmmezzarobbaWed, 06 Apr 2016 06:58:00 GMT
https://trac.sagemath.org/ticket/14485#comment:17
https://trac.sagemath.org/ticket/14485#comment:17
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14485#comment:16" title="Comment 16">vdelecroix</a>:
</p>
<blockquote class="citation">
<p>
You should get rid of the function <code>_late_import</code> since it is not used anymore.
</p>
</blockquote>
<p>
Indeed, I didn't notice, thanks!
</p>
<blockquote class="citation">
<p>
And it would be better to simplify the code of <code>_coerce_map_from_</code> as
</p>
<pre class="wiki">return from_par == ZZ or from_par == QQ or from_par == int or \
from_par == long or from_par == AA or from_par == QQbar
</pre></blockquote>
<p>
The original style made sense to me so I refrained from making unnecessary changes, but if you prefer this version, fine. I also replaced the <code>==</code> with <code>is</code>.
</p>
<blockquote class="citation">
<p>
Moreover, I am not sure if coercion from <code>int/long</code> is necessary here since a conversion would exists through <code>ZZ</code>.
</p>
</blockquote>
<p>
<code>AlgebraicNumber_base.__init__</code> also has a code path for <code>int</code> and <code>long</code> initializers, but they end up being converted to <code>ZZ</code>, so I guess it won't hurt to remove these cases. Done.
</p>
TicketgitWed, 06 Apr 2016 07:18:27 GMTcommit changed
https://trac.sagemath.org/ticket/14485#comment:18
https://trac.sagemath.org/ticket/14485#comment:18
<ul>
<li><strong>commit</strong>
changed from <em>9f5b44b3b5e431a92c77af7c52f4984c5c199a49</em> to <em>4ba720258752fc035549147219161e6568e80cde</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=4ba720258752fc035549147219161e6568e80cde"><span class="icon"></span>4ba7202</a></td><td><code>#14485 simplify code according to reviewer's comments</code>
</td></tr></table>
TicketmmezzarobbaWed, 06 Apr 2016 08:40:04 GMTstatus changed
https://trac.sagemath.org/ticket/14485#comment:19
https://trac.sagemath.org/ticket/14485#comment:19
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketvdelecroixWed, 06 Apr 2016 13:58:26 GMT
https://trac.sagemath.org/ticket/14485#comment:20
https://trac.sagemath.org/ticket/14485#comment:20
<p>
While working on something else I saw the following comment in <code>symbolic/functions.pyx</code> line 473
</p>
<pre class="wiki"># There is no natural coercion from QQbar to the symbolic ring
# in order to support
# sage: QQbar(sqrt(2)) + sqrt(3)
# 3.146264369941973?
# to work around this limitation, we manually convert
# elements of QQbar to symbolic expressions here
</pre>
TicketmmezzarobbaWed, 06 Apr 2016 14:52:49 GMT
https://trac.sagemath.org/ticket/14485#comment:21
https://trac.sagemath.org/ticket/14485#comment:21
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14485#comment:20" title="Comment 20">vdelecroix</a>:
</p>
<blockquote class="citation">
<p>
While working on something else I saw the following comment in <code>symbolic/functions.pyx</code> line 473
</p>
<pre class="wiki"># There is no natural coercion from QQbar to the symbolic ring
# in order to support
# sage: QQbar(sqrt(2)) + sqrt(3)
# 3.146264369941973?
# to work around this limitation, we manually convert
# elements of QQbar to symbolic expressions here
</pre></blockquote>
<p>
The patches on this ticket remove this comment and the corresponding code. I think people now agree that even if it was more or less intentional, trying to support this was a bad idea.
</p>
TicketvdelecroixWed, 06 Apr 2016 17:08:15 GMTstatus, milestone changed; reviewer set
https://trac.sagemath.org/ticket/14485#comment:22
https://trac.sagemath.org/ticket/14485#comment:22
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Vincent Delecroix</em>
</li>
<li><strong>milestone</strong>
changed from <em>sage-6.4</em> to <em>sage-7.2</em>
</li>
</ul>
<p>
let it be!
</p>
TicketvbraunFri, 08 Apr 2016 22:40:26 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/14485#comment:23
https://trac.sagemath.org/ticket/14485#comment:23
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>branch</strong>
changed from <em>u/mmezzarobba/14485-QQbartoSR</em> to <em>4ba720258752fc035549147219161e6568e80cde</em>
</li>
</ul>
TicketrwsSat, 09 Apr 2016 06:05:35 GMTcommit deleted
https://trac.sagemath.org/ticket/14485#comment:24
https://trac.sagemath.org/ticket/14485#comment:24
<ul>
<li><strong>commit</strong>
<em>4ba720258752fc035549147219161e6568e80cde</em> deleted
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14485#comment:21" title="Comment 21">mmezzarobba</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14485#comment:20" title="Comment 20">vdelecroix</a>:
</p>
<blockquote class="citation">
<p>
While working on something else I saw the following comment in <code>symbolic/functions.pyx</code> line 473
</p>
<pre class="wiki"># There is no natural coercion from QQbar to the symbolic ring
# in order to support
# sage: QQbar(sqrt(2)) + sqrt(3)
# 3.146264369941973?
# to work around this limitation, we manually convert
# elements of QQbar to symbolic expressions here
</pre></blockquote>
<p>
The patches on this ticket remove this comment and the corresponding code. I think people now agree that even if it was more or less intentional, trying to support this was a bad idea.
</p>
</blockquote>
<p>
See also <a class="closed ticket" href="https://trac.sagemath.org/ticket/17790" title="defect: convert not coerce padics symbolic function arguments (closed: wontfix)">#17790</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/18832" title="defect: non-numeric non-symbolic BuiltinFunction arguments (closed: fixed)">#18832</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/20312" title="defect: parent of argument lost with Functions (closed: fixed)">#20312</a>
</p>
Ticket