#11975: Chow-Heegner points
-------------------------------+--------------------------------------------
Reporter: was | Owner: cremona
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-4.8
Component: elliptic curves | Keywords:
Work_issues: | Upstream: N/A
Reviewer: | Author:
Merged: | Dependencies:
-------------------------------+--------------------------------------------
Comment(by was):
This is John's second review, which he couldn't post because of a DB
error. I'm posting it for him:
1. With the second patch, all tests pass except for one (see below) and
the docs build OK. I have not yet actually looked at the ref-man
pages. And the new files have 100% coverage.
{{{
sage -t devel/sage-main/sage/schemes/elliptic_curves/chow_heegner.py
**********************************************************************
File "/home/jec/sage-4.8.rc0/devel/sage-
main/sage/schemes/elliptic_curves/chow_heegner.py", line 236:
sage: sl2z_rep_in_fundom(complex(2.17,.19))
Exception raised:
Traceback (most recent call last):
File "/home/jec/sage-4.8.rc0/local/bin/ncadoctest.py", line 1231, in
run_one_test
self.run_one_example(test, example, filename, compileflags)
File "/home/jec/sage-4.8.rc0/local/bin/sagedoctest.py", line 38, in
run_one_example
OrigDocTestRunner.run_one_example(self, test, example, filename,
compileflags)
File "/home/jec/sage-4.8.rc0/local/bin/ncadoctest.py", line 1172, in
run_one_example
compileflags, 1) in test.globs
File "<doctest __main__.example_2[28]>", line 1, in <module>
sl2z_rep_in_fundom(complex(RealNumber('2.17'),RealNumber('.19')))###line
236:
sage: sl2z_rep_in_fundom(complex(2.17,.19))
File "/home/jec/sage-4.8.rc0/local/lib/python/site-
packages/sage/schemes/elliptic_curves/chow_heegner.py", line 240, in
sl2z_rep_in_fundom
if isinstance(z, (float, int, long)) or z.imag() <= 0:
TypeError: 'float' object is not callable
}}}
The problem is that when z has type complex, z.imag is not a function
so z.imag() raises an error. I don't know what to do: imag(z) works
for CDF and complex, but for complex it returns a complex number!!!
{{{
sage: imag(CDF(1,1))
1.0
sage: imag(complex(1,1))
(1+0j)
}}}
2. In the comp method for X0NPoint you get an assertion error when the
levels are different. Would it not be better to do something
different?
3. Your Atkin-Lehner method for X0NPoint is more general than the doc
says: it says that q should be a prime power, but it works fine for
any q|N coprime to N/q (as it should!).
{{{
sage: z = CDF(1,1)
sage: P = X0NPoint(z,90,30) # N = 90 = 10*9
sage: P.atkin_lehner(10)
[0.11049724 + 0.00055248619*I]
sage: P.atkin_lehner(10).atkin_lehner(10)
[0.10554020 + 0.000013888696*I]
sage: P.atkin_lehner(10).atkin_lehner(10) == P
True
}}}
Since you'll be editing the INPUT anyway, I suggest adding an example such
as this.
4. Function close_points: (1) missing explanation of the search_bound
input param. (2) I guess you mean to use this function only for rank1
curves? You only use the first generator. And it's inefficient to
compute n*Q for each n in the range [-k..k] separately, which is
*precisely* why Iwrote this a few years ago:
{{{
sage: multiples?
File: /home/jec/sage-4.8.rc0/local/lib/python2.6/site-
packages/sage/groups/generic.py
Docstring:
Return an iterator which runs through "P0+i*P" for "i" in
"range(n)".
}}}
For example:
{{{
sage: P = EllipticCurve('389a1').gens()[0]
sage: [Q[0] for Q in multiples(P,11,-5*P)]
[-22625407/11397376, 47503/16641, 26/361, 10/9, -1, 0, -1, 10/9, 26/361,
47503/16641, -22625407/11397376]
}}}
5. The last example for the identify function calls point_exact() not
identify()! And point_exact seems to be a methd of a different class.
Did you mean to say c.numerical_approx().identify()?
6. I checked that EllipticCurve([0,2011]) is still not in the database
(no: it has conductor 1.7 billion). I won't ask for this example to
be changed to [0,2012] (which has a smaller conductor!).
7. check_optimal: special case needed for 990h3. sorry.....
8. "Return the elliptic curve that this is the modular parametrization
of."
should be
"Return the elliptic curve of which this is the modular parametrization."
(There are some lapses of grammar up with which I will not put!)
--- reached line 1681 of the first patch, just before ChowHeegnerPoint
class ---
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11975#comment:10>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
