Sage: Ticket #10888: problem in evaluation dual isogeny
https://trac.sagemath.org/ticket/10888
<p>
The following bug was reported by J. Gillibert.
</p>
<pre class="wiki">sage: K.<th> = NumberField(x^2+3)
sage: E = EllipticCurve(K,[7,0])
sage: phi = E.isogeny(E(0,0))
sage: P = E(3,4*th)
sage: phi(P)
(16/3 : 8/9*th : 1)
sage: Q = phi(P)
sage: phihat = phi.dual()
sage: phihat(Q)
</pre><p>
This gives back a
</p>
<pre class="wiki">TypeError: <type 'sage.rings.polynomial.multi_polynomial_libsingular.MPolynomial_libsingular'>
</pre>
enus
Sage
https://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/10888
Trac 1.1.6

wuthrich
Wed, 09 Mar 2011 12:35:45 GMT
https://trac.sagemath.org/ticket/10888#comment:1
https://trac.sagemath.org/ticket/10888#comment:1
<p>
The problem is very simply that evaluation in a polynomial with two variables does not give back a constant, but a constant polynomial. Is this a bug ?
</p>
<pre class="wiki">sage: R.<x,y> = PolynomialRing(ZZ,2)
sage: f(x=2,y=0)
2
sage: type(f(x=2,y=0))
<type 'sage.rings.polynomial.multi_polynomial_libsingular.MPolynomial_libsingular'>
</pre><p>
It is certainly not consitent with this :
</p>
<pre class="wiki">sage: R.<x> = PolynomialRing(ZZ)
sage: f= x^2+3
sage: type(f(x=2))
<type 'sage.rings.integer.Integer'>
</pre>
Ticket

cremona
Wed, 09 Mar 2011 16:38:34 GMT
https://trac.sagemath.org/ticket/10888#comment:2
https://trac.sagemath.org/ticket/10888#comment:2
<p>
I came across something very similar indeed before and thought I had fixed it. I should check to see if the patch I made ever got merged.
</p>
Ticket

cremona
Wed, 09 Mar 2011 16:40:36 GMT
https://trac.sagemath.org/ticket/10888#comment:3
https://trac.sagemath.org/ticket/10888#comment:3
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10888#comment:2" title="Comment 2">cremona</a>:
</p>
<blockquote class="citation">
<p>
I came across something very similar indeed before and thought I had fixed it. I should check to see if the patch I made ever got merged.
</p>
</blockquote>
<p>
See <a class="closed ticket" href="https://trac.sagemath.org/ticket/8502" title="defect: evaluating multivariate polynomials yields nonconstant (closed: fixed)">#8502</a>.
</p>
Ticket

wuthrich
Fri, 11 Mar 2011 10:41:13 GMT
https://trac.sagemath.org/ticket/10888#comment:4
https://trac.sagemath.org/ticket/10888#comment:4
<p>
John, thanks to your link I found quickly what is the problem. Consider
</p>
<pre class="wiki">f(2,3)
f(x=2,y=3)
f.subs(x=2,y=3)
</pre><p>
The first returns an element in the base ring, while the others are still polynomials. This is correct with respect to the documentation apart from the middle one. <code>f(x=2,y=3)</code> is the <code>__call__</code> method and there, in the first lines, it jumps to execute <code>subs</code>.
</p>
<p>
So something has to be changed there. I am uncertain what. I attach a patch that changes <code>__call__</code> to make it coherent with the documentation, but it is not ready for review because it will most certainly produce many problems in other places...
</p>
<p>
I will aks this to sagedevel.
</p>
Ticket

wuthrich
Fri, 11 Mar 2011 10:42:05 GMT
attachment set
https://trac.sagemath.org/ticket/10888
https://trac.sagemath.org/ticket/10888
<ul>
<li><strong>attachment</strong>
set to <em>trac_10888_evaluation_of_multi_polynomials.patch</em>
</li>
</ul>
<p>
only uploaded for discussion
</p>
Ticket

wuthrich
Wed, 16 Mar 2011 14:55:20 GMT
attachment set
https://trac.sagemath.org/ticket/10888
https://trac.sagemath.org/ticket/10888
<ul>
<li><strong>attachment</strong>
set to <em>trac_10888.patch</em>
</li>
</ul>
<p>
exported against 4.6.2. Only this patch should be applied.
</p>
Ticket

wuthrich
Wed, 16 Mar 2011 14:58:29 GMT
status changed; milestone, author set
https://trac.sagemath.org/ticket/10888#comment:5
https://trac.sagemath.org/ticket/10888#comment:5
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>milestone</strong>
set to <em>sage4.7</em>
</li>
<li><strong>author</strong>
set to <em>wuthrich</em>
</li>
</ul>
<p>
From the discussion on sagedevel, I learned that I should not change the functioning of evaluation in this ticket. To solve the bug in the isogenies, I simply make the evaluations in the code to be evaluations without keywords. Then it is no longer ambiguous.
</p>
<p>
I will open another ticket about the issue with evaluation and substitution.
</p>
Ticket

wuthrich
Wed, 16 Mar 2011 15:10:05 GMT
https://trac.sagemath.org/ticket/10888#comment:6
https://trac.sagemath.org/ticket/10888#comment:6
<p>
The other, more general issue is now <a class="new ticket" href="https://trac.sagemath.org/ticket/10946" title="defect: Evaluation and subsitution of multivariable polynomials (new)">#10946</a>.
</p>
Ticket

defeo
Thu, 21 Apr 2011 14:40:50 GMT
status changed; cc set
https://trac.sagemath.org/ticket/10888#comment:7
https://trac.sagemath.org/ticket/10888#comment:7
<ul>
<li><strong>cc</strong>
<em>defeo</em> added
</li>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
The patch works and I'm willing to give a positive review, but the doctest takes 2.6 cpu secs on my laptop (64bit Intel Core2 U9400 @ 1.40GHz... not the fastest cpu ever, but still decent), it would make sense to tag this test as long.
</p>
<p>
Alternatively, though I don't know if this is acceptable for Sage standards, the doctest could be slightly changed. The code below fails in 4.6.2 and is fixed by this patch; it takes only 0.2 secs, because it uses Kohel's algorithm (instead of VĂ©lu's formulae) to compute <code>phi</code> and because avoids the call to <code>phi.dual()</code> (which uses Stark's algorithm). It tests exactly the same bug, because it crosses the same conditional branches modified by the patch.
</p>
<pre class="wiki">sage: K.<th> = NumberField(x^2+3)
sage: E = EllipticCurve(K,[7,0])
sage: _.<X> = K[]
sage: phi = E.isogeny(X)
sage: phi.set_pre_isomorphism(E.automorphisms()[1])
sage: phi.set_post_isomorphism(phi.codomain().automorphisms()[1])
sage: P = E(3,4*th)
sage: phi(P)
(16/3 : 8/9*th : 1)
</pre><p>
Finally, the doctest does not show up in the reference manual, because it is in the docstring of the method <code>__call__</code>. I don't know if this really is a problem.
</p>
Ticket

cremona
Fri, 22 Apr 2011 15:16:42 GMT
status changed; reviewer set; cc deleted
https://trac.sagemath.org/ticket/10888#comment:8
https://trac.sagemath.org/ticket/10888#comment:8
<ul>
<li><strong>cc</strong>
<em>defeo</em> removed
</li>
<li><strong>reviewer</strong>
set to <em>Luca De Feo, John Cremona</em>
</li>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>positive_review</em>
</li>
</ul>
<p>
I have also tested the patch and agree that it works (applied to 4.7.alpha5, all tests in elliptic curves pass).
</p>
<p>
I don't think that the test is too long. This whole file is quite long as a whole (nearly 30s) but several other tests are longer! I expect to be splitting this file into two before long, so I am happy to leave off the "long time" tag. I would also be happy to have defeo's additional code added.
</p>
<p>
It doesn't matter that this example does not appear in the reference manual. It stays in as a test, to make sure this will continue to work in future; but it does not add a lot to for the user, I think. If you want it to appear in the manual, insert it in the header section of the file (instead, or as well).
</p>
<p>
I'll set this to "positive review" now; if you (defeo) wants to make further changes to the tests put it back to "needs review" and I'll look at it again.
</p>
Ticket

jdemeyer
Fri, 22 Apr 2011 15:51:59 GMT
milestone changed
https://trac.sagemath.org/ticket/10888#comment:9
https://trac.sagemath.org/ticket/10888#comment:9
<ul>
<li><strong>milestone</strong>
changed from <em>sage4.7</em> to <em>sage4.7.1</em>
</li>
</ul>
Ticket

jdemeyer
Sat, 30 Apr 2011 11:04:53 GMT
author changed
https://trac.sagemath.org/ticket/10888#comment:10
https://trac.sagemath.org/ticket/10888#comment:10
<ul>
<li><strong>author</strong>
changed from <em>wuthrich</em> to <em>Chris Wuthrich</em>
</li>
</ul>
Ticket

jdemeyer
Wed, 04 May 2011 06:51:02 GMT
status changed; resolution, merged set
https://trac.sagemath.org/ticket/10888#comment:11
https://trac.sagemath.org/ticket/10888#comment:11
<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>merged</strong>
set to <em>sage4.7.1.alpha0</em>
</li>
</ul>
Ticket