Sage: Ticket #14982: When a parent is equipped with an embedding, consider coercions that don't go through the embedding
https://trac.sagemath.org/ticket/14982
<p>
<code>Parent.discover_coerce_map_from</code> used to always give priority to coercions that start with applying an embedding over those provided by <code>_coerce_map_from_</code>. This leads to coercion failures such as the following:
</p>
<pre class="wiki">sage: K.<a> = NumberField(x^2+1/2, embedding=CC(0,1))
sage: L = NumberField(x^2+2, 'b', embedding=1/a)
sage: R = PolynomialRing(L, 'x')
sage: R.coerce_map_from(R.base_ring())
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
<ipython-input-8-fb89bb074d92> in <module>()
----> 1 R.coerce_map_from(R.base_ring())
...
AttributeError: 'NoneType' object has no attribute 'domain'
</pre><p>
This patch series modifies the coercion discovery algorithm to fix the issue, and fixes or works around a few weaknesses in existing code exposed by the change. See the individual commit messages for details.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/14982
Trac 1.1.6mmezzarobbaThu, 08 Aug 2013 16:40:48 GMTcc set
https://trac.sagemath.org/ticket/14982#comment:1
https://trac.sagemath.org/ticket/14982#comment:1
<ul>
<li><strong>cc</strong>
<em>robertwb</em> <em>SimonKing</em> added
</li>
</ul>
<p>
Apparently, the problem is more general stems from the policy of <code>Parent.discover_coerce_map_from</code> of always giving priority to coercions that start with applying an embedding over those provided by <code>_coerce_map_from_</code>.
</p>
<p>
In the special case of this ticket, a workaround would be to revert part of a3c5958b8e6983a3ef8466d0bd0b6ce09fd32d41 so that the <code>BaseringInjection</code> gets registered as early as possible:
</p>
<pre class="wiki">--- a/src/sage/rings/polynomial/polynomial_ring.py
+++ b/src/sage/rings/polynomial/polynomial_ring.py
@@ -283,9 +283,11 @@ class PolynomialRing_general(sage.algebras.algebra.Algebra):
# since we want to use PolynomialBaseringInjection.
sage.algebras.algebra.Algebra.__init__(self, base_ring, names=name, normalize=True, category=category)
self.__generator = self._polynomial_class(self, [0,1], is_gen=True)
+ base_inject = PolynomialBaseringInjection(base_ring, self)
+ self._unset_coercions_used()
self._populate_coercion_lists_(
- #coerce_list = [base_inject],
- #convert_list = [list, base_inject],
+ coerce_list = [base_inject],
+ convert_list = [list, base_inject],
convert_method_name = '_polynomial_')
@@ -552,8 +554,8 @@ class PolynomialRing_general(sage.algebras.algebra.Algebra):
"""
# In the first place, handle the base ring
base_ring = self.base_ring()
- if P is base_ring:
- return PolynomialBaseringInjection(base_ring, self)
+ #if P is base_ring:
+ # return PolynomialBaseringInjection(base_ring, self)
# handle constants that canonically coerce into self.base_ring()
# first, if possible
try:
</pre><p>
(I didn't check whether this breaks anything else.)
</p>
<p>
But the same issue potentially affects anything constructed over a base ring with <code>coerce_embedding</code>. At best, a suboptimal coercion is found:
</p>
<pre class="wiki">sage: M = MatrixSpace(L, 2, 2)
sage: M.coerce_map_from(L)
Composite map:
From: Number Field in b with defining polynomial x^2 + 2
To: Full MatrixSpace of 2 by 2 dense matrices over Number Field in b with defining polynomial x^2 + 2
Defn: Generic morphism:
From: Number Field in b with defining polynomial x^2 + 2
To: Number Field in a with defining polynomial x^2 + 1/2
Defn: b -> -2*a
then
Call morphism:
From: Number Field in a with defining polynomial x^2 + 1/2
To: Full MatrixSpace of 2 by 2 dense matrices over Number Field in b with defining polynomial x^2 + 2
sage: PowerSeriesRing(L, 'x').coerce_map_from(L)
Composite map:
From: Number Field in b with defining polynomial x^2 + 2
To: Power Series Ring in x over Number Field in b with defining polynomial x^2 + 2
Defn: Generic morphism:
From: Number Field in b with defining polynomial x^2 + 2
To: Number Field in a with defining polynomial x^2 + 1/2
Defn: b -> -2*a
then
Conversion map:
From: Number Field in a with defining polynomial x^2 + 1/2
To: Power Series Ring in x over Number Field in b with defining polynomial x^2 + 2
</pre><p>
and at worst the construction itself fails (well, I'm assuming it's the same issue as above, but I didn't check in detail):
</p>
<pre class="wiki">sage: L.extension(x^3+x+1, 'c')
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
<ipython-input-28-b42564719919> in <module>()
----> 1 L.extension(x**Integer(3)+x+Integer(1), 'c')
/home/marc/co/sage/local/lib/python2.7/site-packages/sage/rings/number_field/number_field.pyc in extension(self, poly, name, names, check, embedding)
4173 raise TypeError, "the variable name must be specified."
4174 from sage.rings.number_field.number_field_rel import NumberField_relative
-> 4175 return NumberField_relative(self, poly, str(name), check=check, embedding=embedding)
4176
4177 def factor(self, n):
/home/marc/co/sage/local/lib/python2.7/site-packages/sage/rings/number_field/number_field_rel.pyc in __init__(self, base, polynomial, name, latex_name, names, check, embedding)
301
302 v[0] = self._gen_relative()
--> 303 v = [self(x) for x in v]
304 self.__gens = tuple(v)
305 self._zero_element = self(0)
/home/marc/co/sage/local/lib/python2.7/site-packages/sage/structure/parent.so in sage.structure.parent.Parent.__call__ (sage/structure/parent.c:8078)()
/home/marc/co/sage/local/lib/python2.7/site-packages/sage/structure/parent.so in sage.structure.parent.Parent.convert_map_from (sage/structure/parent.c:15826)()
/home/marc/co/sage/local/lib/python2.7/site-packages/sage/structure/parent.so in sage.structure.parent.Parent.discover_convert_map_from (sage/structure/parent.c:15991)()
/home/marc/co/sage/local/lib/python2.7/site-packages/sage/structure/parent.so in sage.structure.parent.Parent.coerce_map_from (sage/structure/parent.c:14563)()
/home/marc/co/sage/local/lib/python2.7/site-packages/sage/structure/parent.so in sage.structure.parent.Parent.discover_coerce_map_from (sage/structure/parent.c:14932)()
/home/marc/co/sage/local/lib/python2.7/site-packages/sage/structure/parent.so in sage.structure.parent.Parent.coerce_map_from (sage/structure/parent.c:14563)()
/home/marc/co/sage/local/lib/python2.7/site-packages/sage/structure/parent.so in sage.structure.parent.Parent.discover_coerce_map_from (sage/structure/parent.c:14985)()
/home/marc/co/sage/local/lib/python2.7/site-packages/sage/structure/parent_old.so in sage.structure.parent_old.Parent._coerce_map_from_ (sage/structure/parent_old.c:6658)()
/home/marc/co/sage/local/lib/python2.7/site-packages/sage/rings/number_field/number_field_rel.pyc in _coerce_map_from_(self, R)
1031 mor = self.base_field().coerce_map_from(R)
1032 if mor is not None:
-> 1033 return self.coerce_map_from(self.base_field()) * mor
1034
1035 def _rnfeltreltoabs(self, element, check=False):
/home/marc/co/sage/local/lib/python2.7/site-packages/sage/categories/map.so in sage.categories.map.Map.__mul__ (sage/categories/map.c:4796)()
AttributeError: 'NoneType' object has no attribute 'domain'
</pre>
TicketmmezzarobbaThu, 08 Aug 2013 16:43:52 GMTsummary changed
https://trac.sagemath.org/ticket/14982#comment:2
https://trac.sagemath.org/ticket/14982#comment:2
<ul>
<li><strong>summary</strong>
changed from <em>Coercion from base ring fails for some polynomial rings over number fields with embeddings</em> to <em>Coercion from rings with coerce_embedding into constructions over those rings is broken</em>
</li>
</ul>
TicketmmezzarobbaTue, 27 Aug 2013 09:16:58 GMTstatus changed; keywords, branch, author set
https://trac.sagemath.org/ticket/14982#comment:3
https://trac.sagemath.org/ticket/14982#comment:3
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>keywords</strong>
<em>embedding</em> added
</li>
<li><strong>branch</strong>
set to <em>u/mmezzarobba/coerce_embeddings</em>
</li>
<li><strong>author</strong>
set to <em>Marc Mezzarobba</em>
</li>
</ul>
<p>
Here's an attempt to fix the problem with a small change to the coercion discovery algorithm. It does break a few things, but I'd argue it only does so by exposing weaknesses in existing code. The other patches in the series fix (or, in one case, work around) these weaknesses.
</p>
<p>
See the commit messages for details.
</p>
<p>
What do the experts say?
</p>
TicketmmezzarobbaTue, 27 Aug 2013 09:17:34 GMTmilestone changed
https://trac.sagemath.org/ticket/14982#comment:4
https://trac.sagemath.org/ticket/14982#comment:4
<ul>
<li><strong>milestone</strong>
changed from <em>sage-5.12</em> to <em>sage-6.0</em>
</li>
</ul>
TicketSimonKingTue, 27 Aug 2013 11:13:50 GMT
https://trac.sagemath.org/ticket/14982#comment:5
https://trac.sagemath.org/ticket/14982#comment:5
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:3" title="Comment 3">mmezzarobba</a>:
</p>
<blockquote class="citation">
<p>
Here's an attempt to fix the problem with a small change to the coercion discovery algorithm. It does break a few things, but I'd argue it only does so by exposing weaknesses in existing code. The other patches in the series fix (or, in one case, work around) these weaknesses.
</p>
<p>
See the commit messages for details.
</p>
<p>
What do the experts say?
</p>
</blockquote>
<p>
I couldn't tell, unless you give me a pointer to the new workfow. What do I need to do, when not a patch but a brunch (aaahm, branch...) is given?
</p>
TicketSimonKingTue, 27 Aug 2013 11:17:54 GMT
https://trac.sagemath.org/ticket/14982#comment:6
https://trac.sagemath.org/ticket/14982#comment:6
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:5" title="Comment 5">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I couldn't tell, unless you give me a pointer to the new workfow. What do I need to do, when not a patch but a brunch (aaahm, branch...) is given?
</p>
</blockquote>
<p>
For example, when I look at the "commits" link, I see that there are three commits, and only one of them seems to refer to this ticket. So, what do I do with the other commits, review-wise? I see that there is a discussion on sage-git on the question what exactly is to be reviewed. Would I only review the last commit (since its commit message refers to this ticket)? Would I review all three commits (but then I would potentially also review dependencies that are reviewed elsewhere!)?
</p>
TicketmmezzarobbaTue, 27 Aug 2013 11:39:42 GMT
https://trac.sagemath.org/ticket/14982#comment:7
https://trac.sagemath.org/ticket/14982#comment:7
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:6" title="Comment 6">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:5" title="Comment 5">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I couldn't tell, unless you give me a pointer to the new workfow. What do I need to do, when not a patch but a brunch (aaahm, branch...) is given?
</p>
</blockquote>
</blockquote>
<p>
If you already have a git-based installation of sage (i.e. you once did something like <code>git clone https://github.com/sagemath/sage.git && git checkout -b origin/build_system && make build</code>) and a remote that refers to the trac server, you just need to fetch and checkout the branch <code>u/mmezzarobba/coerce_embeddings</code> from trac and review the three commits on top of <code>build_system</code> just like you would if they were patches you just applied.
</p>
<p>
Otherwise, hmm, I guess it is easier to set up a git-based installation than to try to do the review with Mercurial... See
<a class="ext-link" href="http://sagemath.github.io/git-developer-guide/"><span class="icon"></span>http://sagemath.github.io/git-developer-guide/</a>
(or ask!) in case you need more information on how to do that.
</p>
<p>
If the branch was based off an old version of sage, I guess you might also want to check that it merges cleanly, but here it is already a descendant of the last beta.
</p>
<blockquote class="citation">
<p>
For example, when I look at the "commits" link, I see that there are three commits, and only one of them seems to refer to this ticket. So, what do I do with the other commits, review-wise? I see that there is a discussion on sage-git on the question what exactly is to be reviewed. Would I only review the last commit (since its commit message refers to this ticket)? Would I review all three commits (but then I would potentially also review dependencies that are reviewed elsewhere!)?
</p>
</blockquote>
<p>
All three commits need review, and none of them is part of any other ticket. I am not sure if/where the ticket number should go in the commit logs (perhaps it would make more sense to put it in the message associated to the <em>merge</em> commit?). The last commit of the branch is also the one that fixes the issue the ticket is about, so I put it here, but I can rewrite history to add it to the two other commits if you want.
</p>
TicketSimonKingTue, 27 Aug 2013 11:54:07 GMT
https://trac.sagemath.org/ticket/14982#comment:8
https://trac.sagemath.org/ticket/14982#comment:8
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:7" title="Comment 7">mmezzarobba</a>:
</p>
<blockquote class="citation">
<p>
If you already have a git-based installation of sage (i.e. you once did something like <code>git clone https://github.com/sagemath/sage.git && git checkout -b origin/build_system && make build</code>)
</p>
</blockquote>
<p>
I have. But at least on one of my machines I get several doctest errors with the latest git version.
</p>
<blockquote class="citation">
<p>
and a remote that refers to the trac server,
</p>
</blockquote>
<p>
I did store my ssh keys on trac. But "having a remote" sounds like I need to inform git about the existence of the trac server.
</p>
<blockquote class="citation">
<p>
you just need to fetch and checkout the branch <code>u/mmezzarobba/coerce_embeddings</code> from trac
</p>
</blockquote>
<p>
I hope the meaning and how-to of "fetch" and "checkout" will be clear to me from the git-developer-guide.
</p>
<blockquote class="citation">
<p>
<a class="ext-link" href="http://sagemath.github.io/git-developer-guide/"><span class="icon"></span>http://sagemath.github.io/git-developer-guide/</a>
(or ask!) in case you need more information on how to do that.
</p>
</blockquote>
<p>
I saw this (or an old version) before, which made me register my ssh keys, for example.
</p>
<blockquote class="citation">
<p>
All three commits need review, and none of them is part of any other ticket. I am not sure if/where the ticket number should go in the commit logs (perhaps it would make more sense to put it in the message associated to the <em>merge</em> commit?). The last commit of the branch is also the one that fixes the issue the ticket is about, so I put it here, but I can rewrite history to add it to the two other commits if you want.
</p>
</blockquote>
<p>
No idea about this. From the discussion on sage-git, it seems to me that the git branches can be totally orthogonal to what one used to do with patches. A branch may span over different tickets, and it is totally unclear <em>from the commits themselves</em> which of them belong to a ticket and which of them belong to a dependency. That's bad for reviewing. In the old model, one would talk about a list of patches <em>attached to the ticket</em>, which makes it crystal clear what ticket a patch belongs to.
</p>
TicketSimonKingTue, 27 Aug 2013 12:02:18 GMT
https://trac.sagemath.org/ticket/14982#comment:9
https://trac.sagemath.org/ticket/14982#comment:9
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:7" title="Comment 7">mmezzarobba</a>:
</p>
<blockquote class="citation">
<p>
Otherwise, hmm, I guess it is easier to set up a git-based installation than to try to do the review with Mercurial... See
<a class="ext-link" href="http://sagemath.github.io/git-developer-guide/"><span class="icon"></span>http://sagemath.github.io/git-developer-guide/</a>
(or ask!) in case you need more information on how to do that.
</p>
</blockquote>
<p>
It fails totally, it seems.
</p>
<p>
Namely, in my git install of sage (which I kept up to date by <code>git pull</code> followed by make), I tried
</p>
<pre class="wiki">simon@linux-sqwp:~/SAGE/GIT/sage> ./sage -dev create-ticket
sage-run received unknown option: -dev
usage: sage [options]
Try 'sage -h' for more information.
</pre><p>
<code>sage -dev create-ticket</code> is about the first example of using the development scripts in the development guide!
</p>
TicketmmezzarobbaTue, 27 Aug 2013 16:42:40 GMT
https://trac.sagemath.org/ticket/14982#comment:10
https://trac.sagemath.org/ticket/14982#comment:10
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:9" title="Comment 9">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
It fails totally, it seems.
</p>
</blockquote>
<p>
Sorry, I always use git directly and didn't realize that de developer guide presented the custom devel scripts first.
</p>
TicketmmezzarobbaTue, 27 Aug 2013 17:22:02 GMT
https://trac.sagemath.org/ticket/14982#comment:11
https://trac.sagemath.org/ticket/14982#comment:11
<p>
(It looks like you got most of this sorted out, based on you posts on sage-git. I'm replying anyway just in case; please feel free to ask for more detail!)
</p>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:8" title="Comment 8">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:7" title="Comment 7">mmezzarobba</a>:
</p>
<blockquote class="citation">
<p>
If you already have a git-based installation of sage (i.e. you once did something like <code>git clone https://github.com/sagemath/sage.git && git checkout -b origin/build_system && make build</code>)
</p>
</blockquote>
<p>
I have. But at least on one of my machines I get several doctest errors with the latest git version.
</p>
</blockquote>
<p>
Yes. These are rather minor issues that are handled elsewhere.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
and a remote that refers to the trac server,
</p>
</blockquote>
<p>
I did store my ssh keys on trac. But "having a remote" sounds like I need to inform git about the existence of the trac server.
</p>
</blockquote>
<p>
Yes:
</p>
<pre class="wiki">git remote add trac ssh://git@trac.sagemath.org:2222/sage.git
</pre><p>
(With this setup, <code>git fetch trac</code> will automatically download copies of all branches available on the trac server. This will likely mean hundreds of branches when everyone switches to git. They don't live in the same namespace as your local branches and don't pollute the output of most commands. But if you don't like that behaviour, you can use <code>-t</code> to limit the subset of branches to track.)
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
you just need to fetch and checkout the branch <code>u/mmezzarobba/coerce_embeddings</code> from trac
</p>
</blockquote>
<p>
I hope the meaning and how-to of "fetch" and "checkout" will be clear to me from the git-developer-guide.
</p>
</blockquote>
<ul><li>fetch = synchronize your local repository's view of the branches on trac (!= your local branches) with their remote state:
<pre class="wiki">git fetch trac
</pre></li></ul><ul><li>checkout = get the contents of a given revision in the form of actual files in your working directory:
<pre class="wiki">git checkout -b my_local_review_branch -t trac/u/mmezzarobba/coerce_embeddings
</pre></li></ul><ul><li>The last command also sets up a local branch for you to work on (e.g., to prepare a reviewer patch). If you only want to have a look at some remote branch, you can omit the <code>-b</code> and <code>-t</code> options. git will issue a scary warning about detached heads. That "detached head state" is when you don't have anywhere to commit your changes if you make any.
</li></ul><p>
If you want to prepare a reviewer patch, you will need to:
</p>
<ul><li>Make your changes.
</li></ul><ul><li>Commit them to your local review branch (<code>git commit -a</code> to commit everything).
</li></ul><ul><li>(Repeat if there are several unrelated changes.)
</li></ul><ul><li>Once you are happy with your local changes, push them somewhere on trac:
<pre class="wiki">git push trac my_local_review_branch:trac/u/SimonKing/coerce_embeddings
</pre></li></ul><ul><li>Tell me about the new branch you created on trac, or just put its name in the Branch: field.
</li></ul><blockquote class="citation">
<p>
No idea about this. From the discussion on sage-git, it seems to me that the git branches can be totally orthogonal to what one used to do with patches. A branch may span over different tickets,
</p>
</blockquote>
<p>
A git branch is just a pointer to a commit (and thus implicitly to everything that precedes it history-wise). So in some sense it every branch spans the whole history of sage, but otherwise a branch cannot really span several tickets.
</p>
<p>
(There are several other kinds of "pointers to commits" in git. Branches are pointers to commit that move up automatically when new commits are added "on" them.)
</p>
<blockquote class="citation">
<p>
and it is totally unclear <em>from the commits themselves</em> which of them belong to a ticket and which of them belong to a dependency. That's bad for reviewing. In the old model, one would talk about a list of patches <em>attached to the ticket</em>, which makes it crystal clear what ticket a patch belongs to.
</p>
</blockquote>
<p>
I agree, and that's why I started this thread...
</p>
TicketjpfloriWed, 30 Oct 2013 10:52:01 GMTcc changed; commit set
https://trac.sagemath.org/ticket/14982#comment:12
https://trac.sagemath.org/ticket/14982#comment:12
<ul>
<li><strong>cc</strong>
<em>jpflori</em> added
</li>
<li><strong>commit</strong>
set to <em>0869e9aae81cc0174ca7151a724cc30489bb4bac</em>
</li>
</ul>
<p>
New commits:
</p>
<table class="wiki">
<tr><td>[changeset:0869e9a]</td><td>14982: consider coercions of parents with embeddings that do not use the embedding
</td></tr><tr><td>[changeset:cd982ae]</td><td>Make coercions that go through lazy fields more robust
</td></tr><tr><td>[changeset:110dd48]</td><td>Cleanup after querying the coercion system with a temporary element_class in NumberField_cyclotomic
</td></tr></table>
TicketSimonKingWed, 30 Oct 2013 11:45:52 GMT
https://trac.sagemath.org/ticket/14982#comment:13
https://trac.sagemath.org/ticket/14982#comment:13
<p>
Is this branch compatible with <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/15303" title="defect: Coercion discovery fails to be transitive (needs_work)">#15303</a>?
</p>
TicketmmezzarobbaWed, 30 Oct 2013 15:17:35 GMT
https://trac.sagemath.org/ticket/14982#comment:14
https://trac.sagemath.org/ticket/14982#comment:14
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:13" title="Comment 13">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Is this branch compatible with <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/15303" title="defect: Coercion discovery fails to be transitive (needs_work)">#15303</a>?
</p>
</blockquote>
<p>
The issues do look related, but otherwise I have no idea: I stopped working on it weeks before the discussion on <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/15303" title="defect: Coercion discovery fails to be transitive (needs_work)">#15303</a> started, and I haven't have time to read that thread yet. I hope to be able to work on this some time next week.
</p>
TicketSimonKingWed, 30 Oct 2013 15:39:03 GMT
https://trac.sagemath.org/ticket/14982#comment:15
https://trac.sagemath.org/ticket/14982#comment:15
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:14" title="Comment 14">mmezzarobba</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:13" title="Comment 13">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Is this branch compatible with <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/15303" title="defect: Coercion discovery fails to be transitive (needs_work)">#15303</a>?
</p>
</blockquote>
<p>
The issues do look related, but otherwise I have no idea: I stopped working on it weeks before the discussion on <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/15303" title="defect: Coercion discovery fails to be transitive (needs_work)">#15303</a> started, and I haven't have time to read that thread yet. I hope to be able to work on this some time next week.
</p>
</blockquote>
<p>
I have tested that <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/15303" title="defect: Coercion discovery fails to be transitive (needs_work)">#15303</a> does not fix the bug from your ticket description. What I meant was: Are there merge conflicts?
</p>
TicketgitWed, 27 Nov 2013 22:14:09 GMTcommit changed
https://trac.sagemath.org/ticket/14982#comment:16
https://trac.sagemath.org/ticket/14982#comment:16
<ul>
<li><strong>commit</strong>
changed from <em>0869e9aae81cc0174ca7151a724cc30489bb4bac</em> to <em>4cf3ca5d71b4a701e5ac19e413fc4b1ab50eb851</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=4cf3ca5"><span class="icon"></span>4cf3ca5</a></td><td>Fix gross bug in Map.domains(), add missing docstrings
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=17832a0"><span class="icon"></span>17832a0</a></td><td>Merge coercion transitivity fixes (<a class="needs_work ticket" href="https://trac.sagemath.org/ticket/15303" title="defect: Coercion discovery fails to be transitive (needs_work)">#15303</a>)
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=528a035"><span class="icon"></span>528a035</a></td><td>Merge branch 'ticket/13394' into ticket/15303, to make weak caches safer
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=851cc95"><span class="icon"></span>851cc95</a></td><td>Avoid some pointer casts in <a class="missing wiki">WeakValueDict?</a> callbacks
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=246518f"><span class="icon"></span>246518f</a></td><td>Use <dict>'s internals in <a class="missing wiki">WeakValueDictionary?</a> and do not reinvent the bucket.
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=fab0ed4"><span class="icon"></span>fab0ed4</a></td><td>Use <a class="missing wiki">WeakValueDict?</a>'s iteration guard more consequently
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=e4adaeb"><span class="icon"></span>e4adaeb</a></td><td>Implement copy and deepcopy for <a class="missing wiki">WeakValueDictionary?</a>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=70a7b8a"><span class="icon"></span>70a7b8a</a></td><td>Guard <a class="missing wiki">WeakValueDictionary?</a> against deletions during iteration
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=c3dba98"><span class="icon"></span>c3dba98</a></td><td>Replace weakref.<a class="missing wiki">WeakValueDictionary?</a> by sage.misc.weak_dict.<a class="missing wiki">WeakValueDictionary?</a>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=17b0236"><span class="icon"></span>17b0236</a></td><td>Documentation for <a class="missing wiki">WeakValueDictionary?</a>
</td></tr></table>
TicketmmezzarobbaWed, 27 Nov 2013 22:20:46 GMT
https://trac.sagemath.org/ticket/14982#comment:17
https://trac.sagemath.org/ticket/14982#comment:17
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:15" title="Comment 15">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I have tested that <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/15303" title="defect: Coercion discovery fails to be transitive (needs_work)">#15303</a> does not fix the bug from your ticket description. What I meant was: Are there merge conflicts?
</p>
</blockquote>
<p>
Sorry for the late reply. There were a few conflicts indeed. I did the merge, and the tests still pass, except that I'm getting a timeout with <code>sage/rings/number_field/number_field.py</code> and random segfaults with some other files.
</p>
<p>
For instance:
</p>
<pre class="wiki">$ sage -t -a
...
----------------------------------------------------------------------
sage -t src/sage/rings/number_field/number_field.py # Time out
sage -t src/sage/rings/number_field/number_field_element_quadratic.pyx # Killed due to segmentation fault
sage -t src/sage/groups/abelian_gps/values.py # Killed due to segmentation fault
----------------------------------------------------------------------
</pre><p>
but then:
</p>
<pre class="wiki">$ sage -t src/sage/rings/number_field/number_field_element_quadratic.pyx
Running doctests with ID 2013-11-27-23-09-47-2b207575.
Doctesting 1 file.
sage -t src/sage/rings/number_field/number_field_element_quadratic.pyx
[420 tests, 7.97 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
</pre><pre class="wiki">$ sage -t src/sage/groups/abelian_gps/values.py
Running doctests with ID 2013-11-27-23-10-03-7a10efd7.
Doctesting 1 file.
sage -t src/sage/groups/abelian_gps/values.py
[81 tests, 0.11 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
</pre>
TicketmmezzarobbaThu, 28 Nov 2013 08:16:59 GMT
https://trac.sagemath.org/ticket/14982#comment:18
https://trac.sagemath.org/ticket/14982#comment:18
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:17" title="Comment 17">mmezzarobba</a>:
</p>
<blockquote class="citation">
<p>
random segfaults with some other files.
</p>
</blockquote>
<p>
It looks like the crashes happen in <code>__pyx_pf_4sage_10categories_3map_3Map_6domain___get__</code>, called from sage/structure/parent.pyx:2536...
</p>
TicketmmezzarobbaThu, 28 Nov 2013 15:32:39 GMT
https://trac.sagemath.org/ticket/14982#comment:19
https://trac.sagemath.org/ticket/14982#comment:19
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:18" title="Comment 18">mmezzarobba</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:17" title="Comment 17">mmezzarobba</a>:
</p>
<blockquote class="citation">
<p>
random segfaults with some other files.
</p>
</blockquote>
<p>
It looks like the crashes happen in <code>__pyx_pf_4sage_10categories_3map_3Map_6domain___get__</code>, called from sage/structure/parent.pyx:2536...
</p>
</blockquote>
<p>
I tried again with a clean build, and I am unable to reproduce the crashes. I guess it was only a compilation problem...
</p>
Ticketvbraun_spamTue, 17 Dec 2013 18:39:51 GMTmilestone changed
https://trac.sagemath.org/ticket/14982#comment:20
https://trac.sagemath.org/ticket/14982#comment:20
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.0</em> to <em>sage-6.1</em>
</li>
</ul>
TicketgitWed, 18 Dec 2013 17:05:19 GMTcommit changed
https://trac.sagemath.org/ticket/14982#comment:21
https://trac.sagemath.org/ticket/14982#comment:21
<ul>
<li><strong>commit</strong>
changed from <em>4cf3ca5d71b4a701e5ac19e413fc4b1ab50eb851</em> to <em>7e8d4aa5ad04fa2b6536508efefcf69776d6d160</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=7e8d4aa"><span class="icon"></span>7e8d4aa</a></td><td><code>Merge coercion transitivity fixes (#15303) again</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=858cd39"><span class="icon"></span>858cd39</a></td><td><code>typo</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=12e8055"><span class="icon"></span>12e8055</a></td><td><code>Merge branch 'ticket/14711' into ticket/15303</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=ee30c20"><span class="icon"></span>ee30c20</a></td><td><code>Address the "check" keyword of scheme morphisms by name, not position</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=d68c5df"><span class="icon"></span>d68c5df</a></td><td><code>Minor fixes, that became needed since #14711 was not merged quickly enough</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=c42b539"><span class="icon"></span>c42b539</a></td><td><code>Merge branch 'master' into ticket/14711</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=2712c53"><span class="icon"></span>2712c53</a></td><td><code>Merge 'trac/master' into ticket/15303</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=23f18f2"><span class="icon"></span>23f18f2</a></td><td><code>Merge branch 'master' into ticket/14711</code>
</td></tr></table>
TicketmmezzarobbaMon, 27 Jan 2014 16:12:55 GMTcommit, description, branch changed
https://trac.sagemath.org/ticket/14982#comment:22
https://trac.sagemath.org/ticket/14982#comment:22
<ul>
<li><strong>commit</strong>
changed from <em>7e8d4aa5ad04fa2b6536508efefcf69776d6d160</em> to <em>2fcc47300d619cd8c0c96ab57686096993e64e5d</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/14982?action=diff&version=22">diff</a>)
</li>
<li><strong>branch</strong>
changed from <em>u/mmezzarobba/coerce_embeddings</em> to <em>u/mmezzarobba/14982-coerce_embeddings</em>
</li>
</ul>
<p>
(git-)Rebased on top of 6.1.rc0, removed dependency on <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/15303" title="defect: Coercion discovery fails to be transitive (needs_work)">#15303</a>.
</p>
<hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=0faffde6f7c72bb7a3f3f34b17bae97ef4039c59"><span class="icon"></span>0faffde</a></td><td><code>Cleanup after querying the coercion system with a temporary element_class in NumberField_cyclotomic</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=bd4f772d2296b9071895eece52172318b96de677"><span class="icon"></span>bd4f772</a></td><td><code>Make coercions that go through lazy fields more robust</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=2fcc47300d619cd8c0c96ab57686096993e64e5d"><span class="icon"></span>2fcc473</a></td><td><code>14982: consider coercions of parents with embeddings that do not use the embedding</code>
</td></tr></table>
Ticketvbraun_spamThu, 30 Jan 2014 21:20:52 GMTmilestone changed
https://trac.sagemath.org/ticket/14982#comment:23
https://trac.sagemath.org/ticket/14982#comment:23
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.1</em> to <em>sage-6.2</em>
</li>
</ul>
TicketmmezzarobbaTue, 04 Mar 2014 09:44:04 GMTsummary changed
https://trac.sagemath.org/ticket/14982#comment:24
https://trac.sagemath.org/ticket/14982#comment:24
<ul>
<li><strong>summary</strong>
changed from <em>Coercion from rings with coerce_embedding into constructions over those rings is broken</em> to <em>Consider coercions of embedded parents that do not use the embedding</em>
</li>
</ul>
TicketgitWed, 16 Apr 2014 15:25:59 GMTcommit changed
https://trac.sagemath.org/ticket/14982#comment:25
https://trac.sagemath.org/ticket/14982#comment:25
<ul>
<li><strong>commit</strong>
changed from <em>2fcc47300d619cd8c0c96ab57686096993e64e5d</em> to <em>b7816788a46fe54990c1f80d89836dc8674b1f89</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=693dfc14777ca08f96cf7062137d0425697480ed"><span class="icon"></span>693dfc1</a></td><td><code>Cleanup after querying the coercion system with a temporary element_class in NumberField_cyclotomic</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=372553ac788694dfa53f156f43136f5f8a707f4a"><span class="icon"></span>372553a</a></td><td><code>Make coercions that go through lazy fields more robust</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=b7816788a46fe54990c1f80d89836dc8674b1f89"><span class="icon"></span>b781678</a></td><td><code>14982: consider coercions of parents with embeddings that do not use the embedding</code>
</td></tr></table>
TicketmmezzarobbaWed, 16 Apr 2014 15:28:28 GMT
https://trac.sagemath.org/ticket/14982#comment:26
https://trac.sagemath.org/ticket/14982#comment:26
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:25" title="Comment 25">git</a>:
</p>
<blockquote class="citation">
<p>
Branch pushed to git repo; I updated commit sha1. This was a forced push.
</p>
</blockquote>
<p>
Rebased. Tests pass.
</p>
Ticketvbraun_spamTue, 06 May 2014 15:20:58 GMTmilestone changed
https://trac.sagemath.org/ticket/14982#comment:27
https://trac.sagemath.org/ticket/14982#comment:27
<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/14982#comment:28
https://trac.sagemath.org/ticket/14982#comment:28
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.3</em> to <em>sage-6.4</em>
</li>
</ul>
TicketgitThu, 05 Feb 2015 17:04:28 GMTcommit changed
https://trac.sagemath.org/ticket/14982#comment:29
https://trac.sagemath.org/ticket/14982#comment:29
<ul>
<li><strong>commit</strong>
changed from <em>b7816788a46fe54990c1f80d89836dc8674b1f89</em> to <em>873e359f39632b76dc4179db73a355563fc33e07</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=d73e507fe6b0adaff018b6fa16d65e5f189929da"><span class="icon"></span>d73e507</a></td><td><code>Cleanup after querying the coercion system with a temporary element_class in NumberField_cyclotomic</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=8318f0cee8441bd7b28982a1cb1c7b661b4e7cc6"><span class="icon"></span>8318f0c</a></td><td><code>Make coercions that go through lazy fields more robust</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=873e359f39632b76dc4179db73a355563fc33e07"><span class="icon"></span>873e359</a></td><td><code>14982: consider coercions of parents with embeddings that do not use the embedding</code>
</td></tr></table>
TicketmmezzarobbaThu, 05 Feb 2015 17:05:20 GMTsummary changed
https://trac.sagemath.org/ticket/14982#comment:30
https://trac.sagemath.org/ticket/14982#comment:30
<ul>
<li><strong>summary</strong>
changed from <em>Consider coercions of embedded parents that do not use the embedding</em> to <em>When a parent is equipped with an embedding, consider coercions that don't go through the embedding</em>
</li>
</ul>
<p>
Rebased. (Tests are still running.)
</p>
<hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=d73e507fe6b0adaff018b6fa16d65e5f189929da"><span class="icon"></span>d73e507</a></td><td><code>Cleanup after querying the coercion system with a temporary element_class in NumberField_cyclotomic</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=8318f0cee8441bd7b28982a1cb1c7b661b4e7cc6"><span class="icon"></span>8318f0c</a></td><td><code>Make coercions that go through lazy fields more robust</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=873e359f39632b76dc4179db73a355563fc33e07"><span class="icon"></span>873e359</a></td><td><code>14982: consider coercions of parents with embeddings that do not use the embedding</code>
</td></tr></table>
TicketmmezzarobbaThu, 05 Feb 2015 17:35:34 GMTstatus changed
https://trac.sagemath.org/ticket/14982#comment:31
https://trac.sagemath.org/ticket/14982#comment:31
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
TicketjpfloriThu, 26 Feb 2015 13:21:48 GMT
https://trac.sagemath.org/ticket/14982#comment:32
https://trac.sagemath.org/ticket/14982#comment:32
<p>
Needs work for what?
</p>
TicketmmezzarobbaThu, 26 Feb 2015 13:34:02 GMT
https://trac.sagemath.org/ticket/14982#comment:33
https://trac.sagemath.org/ticket/14982#comment:33
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:32" title="Comment 32">jpflori</a>:
</p>
<blockquote class="citation">
<p>
Needs work for what?
</p>
</blockquote>
<p>
There are new test failures with the rebase. I haven't had time to fix them yet...
</p>
TicketgitTue, 14 Apr 2015 16:15:12 GMTcommit changed
https://trac.sagemath.org/ticket/14982#comment:34
https://trac.sagemath.org/ticket/14982#comment:34
<ul>
<li><strong>commit</strong>
changed from <em>873e359f39632b76dc4179db73a355563fc33e07</em> to <em>3cfa5436266dcab268c98ac9627ecbb47c11415b</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=3d7d82383472bf19beb16860d5454af3271b3081"><span class="icon"></span>3d7d823</a></td><td><code>#14982 Clean up after querying the coercion system with a temporary element_class in NumberField_cyclotomic</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=434c73d0eaea158fe65f9c12c9eb6b81efe51db6"><span class="icon"></span>434c73d</a></td><td><code>#14982 Make coercions that go through lazy fields more robust</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=749ec1ff7bc390070b28c1aedca7b584c7a70e2f"><span class="icon"></span>749ec1f</a></td><td><code>#14982 Consider coercions of parents with embeddings that do not use the embedding</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=3cfa5436266dcab268c98ac9627ecbb47c11415b"><span class="icon"></span>3cfa543</a></td><td><code>#14982 Workaround for q_binomial doctest</code>
</td></tr></table>
TicketgitTue, 14 Apr 2015 16:37:21 GMTcommit changed
https://trac.sagemath.org/ticket/14982#comment:35
https://trac.sagemath.org/ticket/14982#comment:35
<ul>
<li><strong>commit</strong>
changed from <em>3cfa5436266dcab268c98ac9627ecbb47c11415b</em> to <em>b40ed6c6aafe4301a81213e6b581f89b4d6d2930</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=b40ed6c6aafe4301a81213e6b581f89b4d6d2930"><span class="icon"></span>b40ed6c</a></td><td><code>#14982 Robustify PolynomialRing_general._coerce_map_from_()</code>
</td></tr></table>
TicketmmezzarobbaTue, 14 Apr 2015 16:40:38 GMTstatus changed
https://trac.sagemath.org/ticket/14982#comment:36
https://trac.sagemath.org/ticket/14982#comment:36
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketmmezzarobbaTue, 14 Apr 2015 16:45:31 GMT
https://trac.sagemath.org/ticket/14982#comment:37
https://trac.sagemath.org/ticket/14982#comment:37
<p>
(I haven't had time to run all tests with the last commit yet, only those that failed before plus modules related to things that changed.)
</p>
TicketmmezzarobbaTue, 14 Apr 2015 16:46:47 GMTdescription changed
https://trac.sagemath.org/ticket/14982#comment:38
https://trac.sagemath.org/ticket/14982#comment:38
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/14982?action=diff&version=38">diff</a>)
</li>
</ul>
TicketmmezzarobbaWed, 15 Apr 2015 04:34:09 GMT
https://trac.sagemath.org/ticket/14982#comment:39
https://trac.sagemath.org/ticket/14982#comment:39
<p>
All tests pass.
</p>
TicketgitSat, 18 Apr 2015 10:47:45 GMTcommit changed
https://trac.sagemath.org/ticket/14982#comment:40
https://trac.sagemath.org/ticket/14982#comment:40
<ul>
<li><strong>commit</strong>
changed from <em>b40ed6c6aafe4301a81213e6b581f89b4d6d2930</em> to <em>f32b52f4176684d57e4e6b83f80c2c553ffcc8eb</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=ce4e0a883ad2da37a8546450e932100dc46666d1"><span class="icon"></span>ce4e0a8</a></td><td><code>#14982 Clean up after querying the coercion system with a temporary element_class in NumberField_cyclotomic</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=ce34b1e6729ad6b205bd71ca12a29b1ff0a5b309"><span class="icon"></span>ce34b1e</a></td><td><code>#14982 Make coercions that go through lazy fields more robust</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=16df5028cbeb6c5ffdd1d4208fa1d0ad6d0c8f92"><span class="icon"></span>16df502</a></td><td><code>#14982 Consider coercions of parents with embeddings that do not use the embedding</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=3f9b0965b511d8d203ca56b614fb89042f6e721b"><span class="icon"></span>3f9b096</a></td><td><code>#14982 Workaround for q_binomial doctest</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=f32b52f4176684d57e4e6b83f80c2c553ffcc8eb"><span class="icon"></span>f32b52f</a></td><td><code>#14982 Robustify PolynomialRing_general._coerce_map_from_()</code>
</td></tr></table>
TicketvdelecroixSat, 18 Apr 2015 11:52:43 GMT
https://trac.sagemath.org/ticket/14982#comment:41
https://trac.sagemath.org/ticket/14982#comment:41
<p>
Hello,
</p>
<p>
I have a question related to number fields (in particular <a class="new ticket" href="https://trac.sagemath.org/ticket/18226" title="enhancement: Native _mpfr_ method on quadratic number field elements (new)">#18226</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/17830" title="enhancement: Comparison of number field elements dependent of real embedding (closed: fixed)">#17830</a>). I do want a direct coercion <code>number field -> RIF</code> based on the (future) <code>_mpfr_</code> method of elements. Right now, the route is long as the discoevery algorithm tells me that I need to do <code>number field -> RLF -> RIF</code> (and actually there is <code>QQbar</code> hidden in the middle). I guess that the proposition here would solve this issue! That would be fantastic.
</p>
<p>
Vincent
</p>
TicketmmezzarobbaSat, 18 Apr 2015 17:03:46 GMT
https://trac.sagemath.org/ticket/14982#comment:42
https://trac.sagemath.org/ticket/14982#comment:42
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:41" title="Comment 41">vdelecroix</a>:
</p>
<blockquote class="citation">
<p>
I have a question related to number fields (in particular <a class="new ticket" href="https://trac.sagemath.org/ticket/18226" title="enhancement: Native _mpfr_ method on quadratic number field elements (new)">#18226</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/17830" title="enhancement: Comparison of number field elements dependent of real embedding (closed: fixed)">#17830</a>). I do want a direct coercion <code>number field -> RIF</code> based on the (future) <code>_mpfr_</code> method of elements. Right now, the route is long as the discoevery algorithm tells me that I need to do <code>number field -> RLF -> RIF</code> (and actually there is <code>QQbar</code> hidden in the middle). I guess that the proposition here would solve this issue! That would be fantastic.
</p>
</blockquote>
<p>
I don't know if the changes in this ticket would suffice to discover the right coercion path, but at least they make it <em>possible</em> to select a coercion path that doesn't use the embedding even when there is another path that uses it!
</p>
TicketvdelecroixTue, 21 Apr 2015 17:04:52 GMT
https://trac.sagemath.org/ticket/14982#comment:43
https://trac.sagemath.org/ticket/14982#comment:43
<p>
Hello,
</p>
<p>
In this comment
</p>
<pre class="wiki">+ # As a consequence, a value of __an_element with the wrong class
+ # is cached during the call to has_coerce_map_from. We reset the
+ # cache afterwards.
</pre><p>
you refer to <code>__an_element</code>. But you changed the attribute's name to <code>_cache_an_element</code>.
</p>
<p>
The hack for quadratic number field is horrible... At least could we replace
</p>
<div class="wiki-code"><div xmlns="http://www.w3.org/1999/xhtml" class="diff">
<ul class="entries">
<li class="entry">
<h2>
<a>src/sage/rings/number_field/number_field_element_quadratic.pyx</a>
</h2>
<pre>diff --git a/src/sage/rings/number_field/number_field_element_quadratic.pyx b/src/sage/rings/number_field/number_field_element_quadratic.pyx
index 32e1acc..692bae1 100644</pre>
<table class="trac-diff inline" summary="Differences" cellspacing="0">
<colgroup><col class="lineno" /><col class="lineno" /><col class="content" /></colgroup>
<thead>
<tr>
<th title="File a/src/sage/rings/number_field/number_field_element_quadratic.pyx">
a
</th>
<th title="File b/src/sage/rings/number_field/number_field_element_quadratic.pyx">
b
</th>
<td><em> cdef class NumberFieldElement_quadratic(NumberFieldElement_absolute):</em> </td>
</tr>
</thead>
<tbody class="unmod">
<tr>
<th>210</th><th>210</th><td class="l"><span> self._reduce_c_()</span></td>
</tr><tr>
<th>211</th><th>211</th><td class="l"><span></span></td>
</tr><tr>
<th>212</th><th>212</th><td class="l"><span> # set the attribute standard embedding which is used in the method</span></td>
</tr>
</tbody><tbody class="mod">
<tr class="first">
<th>213</th><th> </th><td class="l"><span> # __cmp__</span></td>
</tr><tr>
<th>214</th><th> </th><td class="l"><span> try:</span></td>
</tr><tr>
<th>215</th><th> </th><td class="l"><span> self.standard_embedding = parent._standard_embedding</span></td>
</tr><tr>
<th>216</th><th> </th><td class="l"><span> except AttributeError:</span></td>
</tr><tr>
<th>217</th><th> </th><td class="l"><span> emb = parent.coerce_embedding()</span></td>
</tr><tr>
<th>218</th><th> </th><td class="l"><span> if emb is None:</span></td>
</tr><tr>
<th>219</th><th> </th><td class="l"><span> self.standard_embedding = True</span></td>
</tr><tr>
<th>220</th><th> </th><td class="l"><span> try:</span></td>
</tr><tr>
<th>221</th><th> </th><td class="l"><span> parent._standard_embedding = True</span></td>
</tr><tr>
<th>222</th><th> </th><td class="l"><span> except AttributeError:</span></td>
</tr><tr>
<th>223</th><th> </th><td class="l"><span> pass</span></td>
</tr><tr>
<th>224</th><th> </th><td class="l"><span> else:</span></td>
</tr><tr>
<th>225</th><th> </th><td class="l"><span> raise ValueError("A parent of NumberFieldElement_quadratic with "</span></td>
</tr><tr>
<th>226</th><th> </th><td class="l"><span> "a canonical embedding should have an attribute "</span></td>
</tr><tr>
<th>227</th><th> </th><td class="l"><span> "_standard_embedding (used for comparisons of elements)")</span></td>
</tr>
<tr>
<th> </th><th>213</th><td class="r"><span> # __cmp__, sign, real, imag, floor, ceil, ...</span></td>
</tr><tr class="last">
<th> </th><th>214</th><td class="r"><span> self.standard_embedding = parent._standard_embedding</span></td>
</tr>
</tbody>
</table>
</li>
</ul>
</div></div><p>
Vincent
</p>
TicketmmezzarobbaWed, 22 Apr 2015 08:21:42 GMT
https://trac.sagemath.org/ticket/14982#comment:44
https://trac.sagemath.org/ticket/14982#comment:44
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:43" title="Comment 43">vdelecroix</a>:
</p>
<blockquote class="citation">
<p>
In this comment
</p>
<pre class="wiki">+ # As a consequence, a value of __an_element with the wrong class
+ # is cached during the call to has_coerce_map_from. We reset the
+ # cache afterwards.
</pre><p>
you refer to <code>__an_element</code>. But you changed the attribute's name to <code>_cache_an_element</code>.
</p>
</blockquote>
<p>
Thanks, fixed.
</p>
<blockquote class="citation">
<p>
The hack for quadratic number field is horrible...
</p>
</blockquote>
<p>
You are the one who introduced it, aren't you? <code>;-)</code> So I guess you understand what it is for better than I do. In any case I just tested the change you suggested (on top of my branch + the current master) and it doesn't seem to break anything.
</p>
TicketgitWed, 22 Apr 2015 08:22:08 GMTcommit changed
https://trac.sagemath.org/ticket/14982#comment:45
https://trac.sagemath.org/ticket/14982#comment:45
<ul>
<li><strong>commit</strong>
changed from <em>f32b52f4176684d57e4e6b83f80c2c553ffcc8eb</em> to <em>4b55415e1dd476e6010fb82b174d5fa455f963bd</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=7baf9d682508e6d6e1cac84fc0d5d3b1fe4fdf1b"><span class="icon"></span>7baf9d6</a></td><td><code>#14982 fix comment</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=4b55415e1dd476e6010fb82b174d5fa455f963bd"><span class="icon"></span>4b55415</a></td><td><code>#14982 simplify NFE_quadratic.__init__</code>
</td></tr></table>
TicketvdelecroixWed, 22 Apr 2015 09:24:42 GMT
https://trac.sagemath.org/ticket/14982#comment:46
https://trac.sagemath.org/ticket/14982#comment:46
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:44" title="Comment 44">mmezzarobba</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:43" title="Comment 43">vdelecroix</a>:
</p>
</blockquote>
<blockquote class="citation">
<blockquote class="citation">
<p>
The hack for quadratic number field is horrible...
</p>
</blockquote>
<p>
You are the one who introduced it, aren't you? <code>;-)</code> So I guess you understand what it is for better than I do. In any case I just tested the change you suggested (on top of my branch + the current master) and it doesn't seem to break anything.
</p>
</blockquote>
<p>
I am ;-(. Thanks for fixing it! I really do not like the loop here: coercion needs an element, but the elements need to know coercion... (certainly out of the scope of the ticket).
</p>
<p>
I fixed the <code>q_binomial</code> issue (see the commit on top <code>public/14982</code>, WARNING: it is merged with 6.7.beta2). If you like it just cherry-pick it on top of your branch. Otherwise, I will open a new ticket. I actually found another bug:
</p>
<pre class="wiki">sage: Sym = SymmetricFunctions(QQ)
sage: s = Sym.schur()
sage: 1 - s[2]
s[] - s[2]
sage: 1r - s[2]
Traceback (most recent call last):
...
TypeError: unsupported operand type(s) for -: 'int' and
'SymmetricFunctionAlgebra_schur_with_category.element_class'
</pre><p>
Vincent
</p>
TicketSimonKingWed, 22 Apr 2015 09:33:05 GMT
https://trac.sagemath.org/ticket/14982#comment:47
https://trac.sagemath.org/ticket/14982#comment:47
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:46" title="Comment 46">vdelecroix</a>:
</p>
<blockquote class="citation">
<p>
I am ;-(. Thanks for fixing it! I really do not like the loop here: coercion needs an element, but the elements need to know coercion... (certainly out of the scope of the ticket).
</p>
</blockquote>
<p>
It used to be worse in the past. You want to multiply an element x of parent P with an element y of parent Q. During multiplication, a coercion relating P and Q is sought, which used to involve a call to <code>P.an_element()</code> and <code>Q.an_element()</code>. But for some parents, this was not implemented or (worse) itself relied on coercion.
</p>
<p>
Today, the fact is used that one already has elements of P and Q in the situation above (namely x and y).
</p>
<p>
So, don't complain, as it could be worse <code>;-)</code>
</p>
TicketmmezzarobbaWed, 22 Apr 2015 11:48:47 GMT
https://trac.sagemath.org/ticket/14982#comment:48
https://trac.sagemath.org/ticket/14982#comment:48
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:46" title="Comment 46">vdelecroix</a>:
</p>
<blockquote class="citation">
<p>
I fixed the <code>q_binomial</code> issue (see the commit on top <code>public/14982</code>, WARNING: it is merged with 6.7.beta2).
</p>
</blockquote>
<p>
Thanks, your fix is much better than what I did (even though the particular issue would probably have been solved by the ticket about <code>I</code>). Just one question: why are you doing
</p>
<pre class="wiki"> try:
zero = R.zero()
except AttributeError:
zero = R(0)
try:
one = R.one()
except AttributeError:
one = R(1)
</pre><p>
instead of just
</p>
<pre class="wiki"> zero = R(0)
one = R(1)
</pre><p>
(Are there cases where <code>R.zero()</code> would work but not <code>R(0)</code>? Or is it faster that way when <code>R</code> is a <code>Parent</code>, despite the <code>try</code>/<code>catch</code> block? And then, does speed matter?)
</p>
<blockquote class="citation">
<p>
If you like it just cherry-pick it on top of your branch.
</p>
</blockquote>
<p>
I guess I will even rebase the whole thing and merge related commits when we're done talking about it.
</p>
<blockquote class="citation">
<p>
I actually found another bug:
</p>
<pre class="wiki">sage: Sym = SymmetricFunctions(QQ)
sage: s = Sym.schur()
sage: 1 - s[2]
s[] - s[2]
sage: 1r - s[2]
Traceback (most recent call last):
...
TypeError: unsupported operand type(s) for -: 'int' and
'SymmetricFunctionAlgebra_schur_with_category.element_class'
</pre></blockquote>
<p>
But this example already didn't work before this ticket, did it?
</p>
TicketmmezzarobbaWed, 22 Apr 2015 11:50:19 GMT
https://trac.sagemath.org/ticket/14982#comment:49
https://trac.sagemath.org/ticket/14982#comment:49
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:46" title="Comment 46">vdelecroix</a>:
</p>
<blockquote class="citation">
<blockquote class="citation">
<blockquote class="citation">
<p>
The hack for quadratic number field is horrible...
</p>
</blockquote>
<p>
You are the one who introduced it, aren't you? <code>;-)</code> So I guess you understand what it is for better than I do. In any case I just tested the change you suggested (on top of my branch + the current master) and it doesn't seem to break anything.
</p>
</blockquote>
<p>
I am ;-(. Thanks for fixing it!
</p>
</blockquote>
<p>
Well, I didn't really fix it: I just made it slightly less fragile...
</p>
TicketgitWed, 22 Apr 2015 12:00:42 GMTcommit changed
https://trac.sagemath.org/ticket/14982#comment:50
https://trac.sagemath.org/ticket/14982#comment:50
<ul>
<li><strong>commit</strong>
changed from <em>4b55415e1dd476e6010fb82b174d5fa455f963bd</em> to <em>ee261a37a06b53106c1640c7ef13b913f7cc2532</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=ee261a37a06b53106c1640c7ef13b913f7cc2532"><span class="icon"></span>ee261a3</a></td><td><code>Trac 14982: fix q_binomial</code>
</td></tr></table>
TicketmmezzarobbaWed, 22 Apr 2015 12:04:09 GMTauthor changed
https://trac.sagemath.org/ticket/14982#comment:51
https://trac.sagemath.org/ticket/14982#comment:51
<ul>
<li><strong>author</strong>
changed from <em>Marc Mezzarobba</em> to <em>Marc Mezzarobba, Vincent Delecroix</em>
</li>
</ul>
TicketvdelecroixWed, 22 Apr 2015 12:15:52 GMT
https://trac.sagemath.org/ticket/14982#comment:52
https://trac.sagemath.org/ticket/14982#comment:52
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:48" title="Comment 48">mmezzarobba</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:46" title="Comment 46">vdelecroix</a>:
</p>
<blockquote class="citation">
<p>
I fixed the <code>q_binomial</code> issue (see the commit on top <code>public/14982</code>, WARNING: it is merged with 6.7.beta2).
</p>
</blockquote>
<p>
Thanks, your fix is much better than what I did (even though the particular issue would probably have been solved by the ticket about <code>I</code>). Just one question: why are you doing
...
instead of just
</p>
<blockquote>
<p>
...
</p>
</blockquote>
<p>
(Are there cases where <code>R.zero()</code> would work but not <code>R(0)</code>? Or is it faster that way when <code>R</code> is a <code>Parent</code>, despite the <code>try</code>/<code>catch</code> block? And then, does speed matter?)
</p>
</blockquote>
<p>
Here is an example:
</p>
<pre class="wiki">sage: C = GF(5).cartesian_product(GF(5))
sage: C(0)
Traceback (most recent call last):
...
TypeError: 'sage.rings.integer.Integer' object is not iterable
sage: C(1)
Traceback (most recent call last):
...
TypeError: 'sage.rings.integer.Integer' object is not iterable
sage: C.zero()
(0, 0)
sage: C.one()
(1, 1)
</pre><p>
But I guess that it should work because there always is a canonical morphism <code>ZZ -> ring</code>. And here not
</p>
<pre class="wiki">sage: C.has_coerce_map_from(ZZ)
False
</pre><p>
So do not worry about this example. The version with <code>zero = R(0)</code> and <code>one = R(1)</code> is much simpler. And anyway it is completely broken
</p>
<pre class="wiki">sage: cyclotomic_value(4, C.one())
Traceback (most recent call last):
...
TypeError: unsupported operand parent(s) for '-':
'The cartesian product of (Finite Field of size 5, Finite Field of size 5)'
and '<type 'int'>'
sage: q_binomial(4, 2, C.one())
Traceback (most recent call last):
..
RuntimeError: maximum recursion depth exceeded while calling a Python object
</pre><p>
Actually, this last example with infinite recursion is related to coercions... so potentially there is something to dig further.
</p>
<blockquote class="citation">
<p>
But this example already didn't work before this ticket, did it?
</p>
</blockquote>
<p>
Nope. Completely unrelated. Actually, it is related in the sense that the was appearing in <code>prod(1 - q**i ...)</code>. And apparently it was the unique reason of this huge <code>try/except</code> block that is removed by my commit.
</p>
<p>
Vincent
</p>
TicketvdelecroixWed, 22 Apr 2015 12:28:09 GMT
https://trac.sagemath.org/ticket/14982#comment:53
https://trac.sagemath.org/ticket/14982#comment:53
<p>
Haha
</p>
<pre class="wiki">sage: one = C.one()
sage: one - one
Traceback (most recent call last):
...
RuntimeError: maximum recursion depth exceeded in __instancecheck__
</pre><p>
see <a class="closed ticket" href="https://trac.sagemath.org/ticket/18275" title="defect: subtraction fails for cartesian products of rings (closed: fixed)">#18275</a>
</p>
TicketgitWed, 22 Apr 2015 13:49:25 GMTcommit changed
https://trac.sagemath.org/ticket/14982#comment:54
https://trac.sagemath.org/ticket/14982#comment:54
<ul>
<li><strong>commit</strong>
changed from <em>ee261a37a06b53106c1640c7ef13b913f7cc2532</em> to <em>d99564520a5fd599c0fea62b86d0bdbfbd35c1a4</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=47b6a0e84a2faa4eb7cf934b79eadc3a2ad9e6cf"><span class="icon"></span>47b6a0e</a></td><td><code>#14982 Clean up after querying the coercion system with a temporary element_class in NumberField_cyclotomic</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=61c73bb1e313f39737c24d8ffa34aaa0d5634f49"><span class="icon"></span>61c73bb</a></td><td><code>#14982 Make coercions that go through lazy fields more robust</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=0aa11af6392cc649b9030bb53843152ebc96b25e"><span class="icon"></span>0aa11af</a></td><td><code>#14982 fix q_binomial</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=a3227dd071f7efc9822a66beb977607aee2f5ac5"><span class="icon"></span>a3227dd</a></td><td><code>#14982 Robustify PolynomialRing_general._coerce_map_from_()</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=d99564520a5fd599c0fea62b86d0bdbfbd35c1a4"><span class="icon"></span>d995645</a></td><td><code>#14982 Consider coercions of parents with embeddings that do not use the embedding</code>
</td></tr></table>
TicketmmezzarobbaThu, 23 Apr 2015 07:24:51 GMT
https://trac.sagemath.org/ticket/14982#comment:55
https://trac.sagemath.org/ticket/14982#comment:55
<p>
Vincent, do your comments count as a review? (If so, you may want to check that I didn't do anything wrong with my last rebase.) Or are there still things to be checked/addressed?
</p>
TicketvdelecroixFri, 24 Apr 2015 23:13:53 GMT
https://trac.sagemath.org/ticket/14982#comment:56
https://trac.sagemath.org/ticket/14982#comment:56
<p>
Hello,
</p>
<p>
I am not sure I am the best person to do the complete review. But...
</p>
<ol><li>Is this really the example you wanted to show <code>MatrixSpace(L, 2, 2).coerce_map_from(L)</code>. Wouldn't be better to test <code>MatrixSpace(K, 2, 2).coerce_map_from(L)</code>? Idem for the one with <code>PowerSeriesRing</code> that follows.
</li></ol><ol start="2"><li>With your modifications the following changes. Old version:
<pre class="wiki">sage: C12.<zeta12> = CyclotomicField(12)
sage: C4.<zeta4> = CyclotomicField(4, embedding=zeta12**3)
sage: PowerSeriesRing(C12, 'x').coerce_map_from(C4)
Composite map:
From: Cyclotomic Field of order 4 and degree 2
To: Power Series Ring in x over Cyclotomic Field of order 12 and degree 4
Defn: Generic morphism:
From: Cyclotomic Field of order 4 and degree 2
To: Cyclotomic Field of order 12 and degree 4
Defn: zeta4 -> zeta12^3
then
Conversion map:
From: Cyclotomic Field of order 12 and degree 4
To: Power Series Ring in x over Cyclotomic Field of order 12 and degree 4
</pre>Versus new version:
<pre class="wiki">sage: PowerSeriesRing(C12, 'x').coerce_map_from(C4)
Conversion map:
From: Cyclotomic Field of order 4 and degree 2
To: Power Series Ring in x over Cyclotomic Field of order 12 and degree 4
sage: timeit("mor(a)", number=5000, repeat=6)
5000 loops, best of 6: 120 µs per loop
</pre>whereas for <code>PolynomialRing</code> the behavior is identical (similar to the "old version" above). Is it the kind of thing that you expected?
</li></ol><p>
Vincent
</p>
TicketvdelecroixFri, 24 Apr 2015 23:14:01 GMTstatus changed
https://trac.sagemath.org/ticket/14982#comment:57
https://trac.sagemath.org/ticket/14982#comment:57
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_info</em>
</li>
</ul>
TicketmmezzarobbaSat, 25 Apr 2015 14:59:41 GMTstatus changed
https://trac.sagemath.org/ticket/14982#comment:58
https://trac.sagemath.org/ticket/14982#comment:58
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:56" title="Comment 56">vdelecroix</a>:
</p>
<blockquote class="citation">
<ol><li>Is this really the example you wanted to show <code>MatrixSpace(L, 2, 2).coerce_map_from(L)</code>. Wouldn't be better to test <code>MatrixSpace(K, 2, 2).coerce_map_from(L)</code>? Idem for the one with <code>PowerSeriesRing</code> that follows.
</li></ol></blockquote>
<p>
Yes, I think it is, because I wanted to illustrate that the way coercions are discovered in the presence of an embedding makes it difficult to work with structures that have the “embedded” parent as a base ring, even if the parent into which it is embedded plays no role in the construction. But I wrote this comment more than 1½ years ago, so I'm not sure I remember correctly!
</p>
<blockquote class="citation">
<ol start="2"><li>With your modifications the following changes.
[...]
whereas for <code>PolynomialRing</code> the behavior is identical (similar to the "old version" above). Is it the kind of thing that you expected?
</li></ol></blockquote>
<p>
Sorry, I'm not sure I understand the question. Let me try to answer it anyway: the goal of these patches is to make the coercion discovery algorithm <em>consider</em> paths that don't start with the embedding. These paths may or may not be selected depending on each particular case.
</p>
<blockquote class="citation">
<pre class="wiki">sage: timeit("mor(a)", number=5000, repeat=6)
5000 loops, best of 6: 120 µs per loop
</pre></blockquote>
<p>
Are these lines a copy-paste error, or did you observe a speed regression with some coercions? (On my machine, applying the morphism returned by <code>PowerSeriesRing(C12, 'x').coerce_map_from(C4)</code> takes almost exactly as much time before and after my patches.)
</p>
TicketvdelecroixSat, 25 Apr 2015 15:05:47 GMT
https://trac.sagemath.org/ticket/14982#comment:59
https://trac.sagemath.org/ticket/14982#comment:59
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:58" title="Comment 58">mmezzarobba</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:56" title="Comment 56">vdelecroix</a>:
</p>
<blockquote class="citation">
<pre class="wiki">sage: timeit("mor(a)", number=5000, repeat=6)
5000 loops, best of 6: 120 µs per loop
</pre></blockquote>
<p>
Are these lines a copy-paste error, or did you observe a speed regression with some coercions? (On my machine, applying the morphism returned by <code>PowerSeriesRing(C12, 'x').coerce_map_from(C4)</code> takes almost exactly as much time before and after my patches.)
</p>
</blockquote>
<p>
Oops sorry. Yes, I observed a tiny regression speed of roughly 4% (120 µs vs 115 µs). Not very significant and that's the reason why I erased half of it.
</p>
TicketvdelecroixSat, 02 May 2015 11:05:53 GMT
https://trac.sagemath.org/ticket/14982#comment:60
https://trac.sagemath.org/ticket/14982#comment:60
<p>
Salut Marc,
</p>
<p>
I am quite confused by the following
</p>
<pre class="wiki">sage: K = QuadraticField(2)
sage: RR.coerce_map_from(K)
Composite map:
From: Number Field in a with defining polynomial x^2 - 2
To: Real Field with 53 bits of precision
Defn: Generic morphism:
From: Number Field in a with defining polynomial x^2 - 2
To: Real Lazy Field
Defn: a -> 1.414213562373095?
then
Conversion via _mpfr_ method map:
From: Real Lazy Field
To: Real Field with 53 bits of precision
</pre><p>
But elements of <code>K</code> do implement directly an <code>_mpfr_</code> method
</p>
<pre class="wiki">sage: K.an_element()._mpfr_
<built-in method _mpfr_ of sage.rings.number_field.number_field_element_quadratic.NumberFieldElement_quadratic object at 0x7f3e3d94ff68>
</pre><p>
So why the discovery does not choose this direct conversion map instead of going through the real lazy field? (note: this method <code>_mpfr_</code> is completely broken, but it does not change anything to the story)
</p>
<p>
Vincent
</p>
TicketmmezzarobbaSat, 02 May 2015 11:37:20 GMT
https://trac.sagemath.org/ticket/14982#comment:61
https://trac.sagemath.org/ticket/14982#comment:61
<p>
Salut Vincent,
</p>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:60" title="Comment 60">vdelecroix</a>:
</p>
<blockquote class="citation">
<p>
So why the discovery does not choose this direct conversion map instead of going through the real lazy field? (note: this method <code>_mpfr_</code> is completely broken, but it does not change anything to the story)
</p>
</blockquote>
<p>
I think the reason is that <code>RR</code> explicitly asks for coercions into it through <code>RLF</code> (see the very last line of <code>RealField_class._coerce_map_from_</code>. So the step of the coercion discovery algorithm that calls <code>_coerce_map_from_</code> (step 2 with my patch) succeeds. The map it finds is of type <code>FormalCompositeMap</code>, so the next step (which would consider the coercions defined using <code>_populate_coercion_lists_</code>, including <code>_mpfr_</code>) is not even tried.
</p>
<p>
As far as I understand, this is the intended behavior from the point of view of the coercion discovery algorithm—<code>_coerce_map_from_</code> “knows better”, do as it says. In any case the fact that the coercion from <code>K</code> to <code>RLF</code> is an embedding doesn't come into play. (But this ticket should make it possible to chose the coercion via <code>_mpfr_</code> by modifying <code>RealField_class._coerce_map_from_</code>, while this wouldn't work now due to the special treatment of embeddings.)
</p>
TicketvdelecroixSat, 02 May 2015 12:34:38 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/14982#comment:62
https://trac.sagemath.org/ticket/14982#comment:62
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>reviewer</strong>
set to <em>Vincent Delecroix</em>
</li>
</ul>
<p>
Final remark: I understand what you are doing with the newly defined method <code>domains</code> but I found the specifications rather unclear
</p>
<pre class="wiki">Yield the domain of self. In general, iterate over the domains of
the factors of a composite map.
</pre><p>
Could you be more precise in the documentation and precise why is it here?
</p>
<p>
Vincent
</p>
TicketgitSat, 02 May 2015 13:13:03 GMTcommit changed
https://trac.sagemath.org/ticket/14982#comment:63
https://trac.sagemath.org/ticket/14982#comment:63
<ul>
<li><strong>commit</strong>
changed from <em>d99564520a5fd599c0fea62b86d0bdbfbd35c1a4</em> to <em>a0ac11b26f4ded7bbcbef5bb6a1f81ae8ed354f6</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=a0ac11b26f4ded7bbcbef5bb6a1f81ae8ed354f6"><span class="icon"></span>a0ac11b</a></td><td><code>#14982 Map.domains(): improve documentation</code>
</td></tr></table>
TicketmmezzarobbaSat, 02 May 2015 13:15:33 GMTstatus changed
https://trac.sagemath.org/ticket/14982#comment:64
https://trac.sagemath.org/ticket/14982#comment:64
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
Thanks again for your comments!
</p>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:62" title="Comment 62">vdelecroix</a>:
</p>
<blockquote class="citation">
<p>
Final remark: I understand what you are doing with the newly defined method <code>domains</code> but I found the specifications rather unclear
</p>
</blockquote>
<p>
Is it better now?
</p>
TicketvdelecroixSat, 02 May 2015 13:24:02 GMTstatus changed
https://trac.sagemath.org/ticket/14982#comment:65
https://trac.sagemath.org/ticket/14982#comment:65
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Yep. Good for me!
</p>
TicketmmezzarobbaSat, 02 May 2015 13:27:11 GMT
https://trac.sagemath.org/ticket/14982#comment:66
https://trac.sagemath.org/ticket/14982#comment:66
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14982#comment:65" title="Comment 65">vdelecroix</a>:
</p>
<blockquote class="citation">
<p>
Yep. Good for me!
</p>
</blockquote>
<p>
Thanks a lot!
</p>
TicketvbraunSat, 02 May 2015 19:28:34 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/14982#comment:67
https://trac.sagemath.org/ticket/14982#comment:67
<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/14982-coerce_embeddings</em> to <em>a0ac11b26f4ded7bbcbef5bb6a1f81ae8ed354f6</em>
</li>
</ul>
Ticket