Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#6616 closed enhancement (fixed)

refactor heegner points code out of ell_rational_field and support computing higher heegner points

Reported by: robertwb Owned by: was
Priority: major Milestone: sage-4.3.1
Component: number theory Keywords:
Cc: was, cremona Merged in: sage-4.3.1.rc0
Authors: William Stein Reviewers: John Cremona
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by was)

I made a clean hg bundle against sage-4.1.1 that includes patches 1-16 along with the patches from 6745 and 6751:

http://sage.math.washington.edu/home/wstein/patches/heegner-1-to-16.hg

NOTE: If you apply patches, the code after part 7 depends on #6745, #6751, so make sure you apply those patches!

PLAN:

  1. Refactor ell_rational_field, so the Heegner points code (nearly 1000 lines!) is in a new file heegner.py.
  1. Extend the heegner point code to compute y_lambda for lambda > 1.
  1. Implement first ever algorithm for provable verification of Kolyvagin's conjecture for specific rank 2 curves.

I'm doing 1 and 2 together, since 2 is a fairly small change, but to do it right and make it at all fast requires restructuring the code a lot, otherwise one ends up fighting a lot with badly structured code instead of focusing on optimizing algorithms.

Also, this refactoring is a long time coming!

Attachments (25)

6616-higher-heegner.patch (2.8 KB) - added by robertwb 10 years ago.
trac_6616-part3.patch (27.8 KB) - added by was 10 years ago.
trac_6616-part5.patch (14.6 KB) - added by was 10 years ago.
trac_6616-part6.patch (37.6 KB) - added by was 10 years ago.
trac_6616-part2.patch (68.4 KB) - added by was 10 years ago.
rebased against 4.1.1
trac_6616-part4.patch (33.4 KB) - added by was 10 years ago.
rebased against 4.1.1
trac_6616-part7.patch (120.9 KB) - added by was 10 years ago.
rebased against 4.1.1
trac_6616-part8.patch (54.1 KB) - added by was 10 years ago.
trac_6616-part10.patch (25.7 KB) - added by was 10 years ago.
trac_6616-part11.patch (55.9 KB) - added by was 10 years ago.
trac_6616-part12.patch (36.0 KB) - added by was 10 years ago.
trac_6616-patch13.patch (15.7 KB) - added by was 10 years ago.
trac_6616-part14.patch (10.5 KB) - added by was 10 years ago.
trac_6616-part15.patch (4.0 KB) - added by was 10 years ago.
trac_6616-part16.patch (78.7 KB) - added by was 10 years ago.
trac_6616-part9.patch (28.4 KB) - added by was 10 years ago.
heegner-1-to-16.hg (73.4 KB) - added by was 10 years ago.
clean bundle with 1-16 against sage-4.1.1
heegner-4.2.1.hg (127.9 KB) - added by was 10 years ago.
rebased against 4.2.1
trac_6616_part17.patch (11.1 KB) - added by was 10 years ago.
this is based against 4.2.1 after applying the hg bundle right above this patch.
trac_6616-part18.patch (6.9 KB) - added by was 10 years ago.
trac-6616-rebase_complete-4.3.hg (623.1 KB) - added by was 10 years ago.
trac-6616-everything_rebased_against_4.3.patch (360.8 KB) - added by was 10 years ago.
apply just this one patch against a clean sage-4.3
trac-6616-everything_rebased_against_4.3-part2.patch (12.5 KB) - added by was 10 years ago.
apply this and the previous patch; then this should past all tests on 32-bit as well as 64-bit.
trac-6616-everything_rebased_against_4.3-part2.2.patch (12.5 KB) - added by cremona 10 years ago.
apply instead of previous one
trac-6616-review.patch (8.9 KB) - added by cremona 10 years ago.
Apply after previous (reviewer's patch fixing some docstring issues)

Download all attachments as: .zip

Change History (47)

Changed 10 years ago by robertwb

comment:1 Changed 10 years ago by was

  • Description modified (diff)
  • Summary changed from [with patch] higher heegner points to [with patch] refactor heegner points code out of ell_rational_field and support computing higher heegner points

Changed 10 years ago by was

comment:2 Changed 10 years ago by was

Next (?):

  • computing y_c by computing all the conjugates of y_c numerically
  • computing the (huge) Kolyvagin point P_c
  • choosing y-coordinate of y_c correctly.
  • computing degree of ring class field K[c] in all cases (not just Kolyvagin c)
  • optimization

comment:3 Changed 10 years ago by was

This calculation uses massive RAM:

sage: E = EllipticCurve([877,0])
sage: P = E.heegner_point(-7)
sage: P
Heegner point of discriminant -7 on curve of conductor 49224256
sage: N(P)
[approx 5GB RAM (?) -- didn't wait for it to crash my laptop]

This should get dealt with. No clue what causes this.

Changed 10 years ago by was

Changed 10 years ago by was

comment:4 Changed 10 years ago by was

  • Description modified (diff)

Changed 10 years ago by was

rebased against 4.1.1

comment:5 Changed 10 years ago by was

  • Description modified (diff)

Changed 10 years ago by was

rebased against 4.1.1

Changed 10 years ago by was

rebased against 4.1.1

comment:6 Changed 10 years ago by was

  • Description modified (diff)

Changed 10 years ago by was

comment:7 Changed 10 years ago by was

  • Description modified (diff)

Changed 10 years ago by was

comment:8 Changed 10 years ago by was

  • Description modified (diff)

Changed 10 years ago by was

Changed 10 years ago by was

Changed 10 years ago by was

comment:9 Changed 10 years ago by was

  • Description modified (diff)

Changed 10 years ago by was

Changed 10 years ago by was

Changed 10 years ago by was

comment:10 Changed 10 years ago by was

  • Description modified (diff)

Changed 10 years ago by was

comment:11 Changed 10 years ago by was

  • Description modified (diff)

comment:12 Changed 10 years ago by was

  • Description modified (diff)

Changed 10 years ago by was

clean bundle with 1-16 against sage-4.1.1

Changed 10 years ago by was

rebased against 4.2.1

Changed 10 years ago by was

this is based against 4.2.1 after applying the hg bundle right above this patch.

Changed 10 years ago by was

Changed 10 years ago by was

Changed 10 years ago by was

apply just this one patch against a clean sage-4.3

comment:14 Changed 10 years ago by was

  • Report Upstream set to N/A
  • Status changed from needs_work to needs_review

This is *finally* ready for review. Apply just this patch against a clean sage-4.3 build:

trac-6616-everything_rebased_against_4.3.patch

comment:15 Changed 10 years ago by was

  • Summary changed from [with patch] refactor heegner points code out of ell_rational_field and support computing higher heegner points to [with patch; needs review] refactor heegner points code out of ell_rational_field and support computing higher heegner points

comment:16 Changed 10 years ago by cremona

  • Reviewers set to John Cremona
  • Status changed from needs_review to needs_work

Review: I am immensely impressed by the huge amount of work which has gone into this ten thousand-line patch! It goes far beyond the refactoring of the title.

I read through the patch itself, and liked what I saw. (There are a few minor typos in the docstrings, which I did not note as I went through -- sorry). Some docstrings which did not format properly: numerical_approx, rational_kolyvagin_divisor, KolyvaginPoint?.

There are a lot if interesting mathematical and algorithmic issues which are highlighted very well in comments and TODO blocks, which is good.

Testing all files in the elliptic_curves directory I found these problems.

	sage -t  "devel/sage-tests/sage/schemes/elliptic_curves/lseries_ell.py"
	sage -t  "devel/sage-tests/sage/schemes/elliptic_curves/heegner.py"
	sage -t  "devel/sage-tests/sage/schemes/elliptic_curves/period_lattice.py"
	sage -t  "devel/sage-tests/sage/schemes/elliptic_curves/ell_rational_field.py"

In heegner.py it's mainly a mixture of numerical noise and hash differences (this is on a 32-bit machine), but not all. Details:

sage -t  "devel/sage-tests/sage/schemes/elliptic_curves/heegner.py"
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 2582:
    sage: hash(y)
Expected:
    -5236815264926108755
Got:
    733770669
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 2886:
    sage: hash(EllipticCurve('389a').heegner_point(-7,5))
Expected:
    ???                              
Got:
    733770669
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 3010:
    sage: P.numerical_approx()
Expected:
    (-3.41893279409096e-16 - 2.00036208715867e-16*I : 3.42282625853674e-16 + 2.00035300823576e-16*I : 1.00000000000000)
Got:
    (-3.41901661684534e-16 - 2.00073402416951e-16*I : 3.42011575310552e-16 + 2.00035300823576e-16*I : 1.00000000000000)
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 3032:
    sage: P = E.heegner_point(-19); y = P._trace_numerical_conductor_1(); y
Expected:
    (-9.52657106432722e-17 - 1.11102282864639e-16*I : -1.00000000000000 + 2.21773554381910e-16*I : 1.00000000000000)
Got:
    (-9.53960112385537e-17 - 1.11192361105451e-16*I : -1.00000000000000 + 2.21827764490534e-16*I : 1.00000000000000)
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 3039:
    sage: P = E.heegner_point(-68); P._trace_numerical_conductor_1()
Expected:
    (9.20680004622768e28 - 1.55670186905154e28*I : -2.76369626392776e43 + 7.09357788995373e42*I : 1.00000000000000)
Got:
    (9.22516515637394e28 - 1.48330779290131e28*I : -2.77483419563894e43 + 6.76511130239484e42*I : 1.00000000000000)
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 3106:
    sage: P.numerical_approx()
Expected:
    (-3.41893279409096e-16 - 2.00036208715867e-16*I : 3.42282625853674e-16 + 2.00035300823576e-16*I : 1.00000000000000)
Got:
    (-3.41901661684534e-16 - 2.00073402416951e-16*I : 3.42011575310552e-16 + 2.00035300823576e-16*I : 1.00000000000000)
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 3110:
    sage: P.numerical_approx(100)[0]
Expected:
    8.4419827889841225189186778139e-31 + 6.0876476174448148263632780203e-31*I
Got:
    8.4419827889841225189186778139e-31 + 6.0876476185623182015101748774e-31*I
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 3114:
    sage: P.numerical_approx()
Expected:
    (-6.68094502209485e-16 + 1.41421356237310*I : 1.00000000000000 - 1.41421356237309*I : 1.00000000000000)
Got:
    (-6.68276051475152e-16 + 1.41421356237310*I : 1.00000000000000 - 1.41421356237309*I : 1.00000000000000)
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 3123:
    sage: P.numerical_approx()
Expected:
    (4.08580183114324e28 + 1.50348132882460e28*I : -7.84283601876376e42 - 4.58366020722762e42*I : 1.00000000000000)
Got:
    (0 : 1.00000000000000 : 0)
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 3558:
    sage: P._trace_numerical_conductor_1()
Expected:
    (1.00000000000000 + 4.49510220712490e-16*I : 1.77646525961750e-16 - 4.49510220712490e-16*I : 1.00000000000000)
Got:
    (1.00000000000000 + 4.49618640929739e-16*I : 1.77809156287623e-16 - 4.49672851038363e-16*I : 1.00000000000000)
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 3939:
    sage: P.numerical_approx()
Expected:
    (-3.41893279409096e-16 - 2.00036208715867e-16*I : 3.42282625853674e-16 + 2.00035300823576e-16*I : 1.00000000000000)
Got:
    (-3.41901661684534e-16 - 2.00073402416951e-16*I : 3.42011575310552e-16 + 2.00035300823576e-16*I : 1.00000000000000)
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 3943:
    sage: P.numerical_approx(100)[0]
Expected:
    8.4419827889841225189186778139e-31 + 6.0876476174448148263632780203e-31*I
Got:
    8.4419827889841225189186778139e-31 + 6.0876476185623182015101748774e-31*I
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 3989:
    sage: P.point_exact()
Expected:
    Traceback (most recent call last):
    ...
    RuntimeError: insufficient precision to find exact point
Got:
    (0 : 1 : 0)
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 4082:
    sage: P.numerical_approx()
Expected:
    (6.00000000000000 + 8.04701070793563e-16*I : -15.0000000000000 - 2.96897922913431e-15*I : 1.00000000000000)
Got:
    (6.00000000000000 + 8.05181817160869e-16*I : -15.0000000000000 - 2.96897922913431e-15*I : 1.00000000000000)
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 605:
    sage: hash(G)
Expected:
    -6198252699510613726
Got:
    1905285410
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 607:
    sage: hash((G.field(), G.base_field()))
Expected:
    -6198252699510613726
Got:
    1905285410
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 6189:
    sage: P.numerical_approx()
Expected:
    (6.00000000000000 + 8.04701070793563e-16*I : -15.0000000000000 - 2.96897922913431e-15*I : 1.00000000000000)
Got:
    (6.00000000000000 + 8.05181817160869e-16*I : -15.0000000000000 - 2.96897922913431e-15*I : 1.00000000000000)
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 1253:
    sage: conj = G.complex_conjugation(); hash(conj)
Expected:
    1347197483068745902
Got:
    480045230
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 1447:
    sage: hash(s)
Expected:
    ??                      
Got:
    -1994029223
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 274:
    sage: hash(K5)
Expected:
    -3713088127102618519
Got:
    1817441385
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 276:
    sage: hash((-7,5))
Expected:
    -3713088127102618519
Got:
    1817441385
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 1688:
    sage: hash(H)
Expected:
    6187687223143458874
Got:
    -458201030
**********************************************************************
15 items had failures:

Incidentally that test takes 90s which is rather long (I have not tested it with "-long" yet!)

Some of this might be caused by the fact that I have a new version of lcalc (from reviewing another ticket) although this is a fresh clone?

sage -t  "devel/sage-tests/sage/schemes/elliptic_curves/lseries_ell.py"
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/lseries_ell.py", line 258:
    sage: E.lseries().zeros_in_interval(6, 10, 0.1)      # long
Exception raised:
    Traceback (most recent call last):
      File "/home/john/sage-4.3/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/home/john/sage-4.3/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/home/john/sage-4.3/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_10[3]>", line 1, in <module>
        E.lseries().zeros_in_interval(Integer(6), Integer(10), RealNumber('0.1'))      # long###line 258:
    sage: E.lseries().zeros_in_interval(6, 10, 0.1)      # long
      File "/home/john/sage-4.3/local/lib/python/site-packages/sage/schemes/elliptic_curves/lseries_ell.py", line 262, in zeros_in_interval
        return lcalc.zeros_in_interval(x, y, stepsize, L=self.__E)
      File "/home/john/sage-4.3/local/lib/python/site-packages/sage/lfunctions/lcalc.py", line 162, in zeros_in_interval
        return [tuple([RR(z) for z in t.split()]) for t in X.split('\n')]
      File "parent.pyx", line 538, in sage.structure.parent.Parent.__call__ (sage/structure/parent.c:4956)
      File "coerce_maps.pyx", line 82, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (sage/structure/coerce_maps.c:3142)
      File "coerce_maps.pyx", line 77, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (sage/structure/coerce_maps.c:3040)
      File "real_mpfr.pyx", line 384, in sage.rings.real_mpfr.RealField._element_constructor_ (sage/rings/real_mpfr.c:5053)
      File "real_mpfr.pyx", line 1009, in sage.rings.real_mpfr.RealNumber._set (sage/rings/real_mpfr.c:8794)
    TypeError: Unable to convert x (='You') to real number.
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/lseries_ell.py", line 311:
    sage: print "ignore this";  E.lseries().twist_values(1, -12, -4)    # slightly random output depending on architecture
Exception raised:
    Traceback (most recent call last):
      File "/home/john/sage-4.3/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/home/john/sage-4.3/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/home/john/sage-4.3/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_12[3]>", line 1, in <module>
        print "ignore this";  E.lseries().twist_values(Integer(1), -Integer(12), -Integer(4))    # slightly random output depending on architecture###line 311:
    sage: print "ignore this";  E.lseries().twist_values(1, -12, -4)    # slightly random output depending on architecture
      File "/home/john/sage-4.3/local/lib/python/site-packages/sage/schemes/elliptic_curves/lseries_ell.py", line 321, in twist_values
        return lcalc.twist_values(s - RationalField()('1/2'), dmin, dmax, L=self.__E)
      File "/home/john/sage-4.3/local/lib/python/site-packages/sage/lfunctions/lcalc.py", line 296, in twist_values
        d,x,y = a.split()
    ValueError: too many values to unpack
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/lseries_ell.py", line 343:
    sage: E.lseries().twist_zeros(3, -4, -3)         # long
Exception raised:
    Traceback (most recent call last):
      File "/home/john/sage-4.3/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/home/john/sage-4.3/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/home/john/sage-4.3/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_13[3]>", line 1, in <module>
        E.lseries().twist_zeros(Integer(3), -Integer(4), -Integer(3))         # long###line 343:
    sage: E.lseries().twist_zeros(3, -4, -3)         # long
      File "/home/john/sage-4.3/local/lib/python/site-packages/sage/schemes/elliptic_curves/lseries_ell.py", line 347, in twist_zeros
        return lcalc.twist_zeros(n, dmin, dmax, L=self.__E)
      File "/home/john/sage-4.3/local/lib/python/site-packages/sage/lfunctions/lcalc.py", line 342, in twist_zeros
        d, x = a.split()
    ValueError: too many values to unpack
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/lseries_ell.py", line 226:
    sage: E.lseries().zeros(2)
Exception raised:
    Traceback (most recent call last):
      File "/home/john/sage-4.3/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/home/john/sage-4.3/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/home/john/sage-4.3/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_9[3]>", line 1, in <module>
        E.lseries().zeros(Integer(2))###line 226:
    sage: E.lseries().zeros(2)
      File "/home/john/sage-4.3/local/lib/python/site-packages/sage/schemes/elliptic_curves/lseries_ell.py", line 236, in zeros
        return lcalc.zeros(n, L=self.__E)
      File "/home/john/sage-4.3/local/lib/python/site-packages/sage/lfunctions/lcalc.py", line 126, in zeros
        return [RR(z) for z in X.split()]
      File "parent.pyx", line 538, in sage.structure.parent.Parent.__call__ (sage/structure/parent.c:4956)
      File "coerce_maps.pyx", line 82, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (sage/structure/coerce_maps.c:3142)
      File "coerce_maps.pyx", line 77, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (sage/structure/coerce_maps.c:3040)
      File "real_mpfr.pyx", line 384, in sage.rings.real_mpfr.RealField._element_constructor_ (sage/rings/real_mpfr.c:5053)
      File "real_mpfr.pyx", line 1009, in sage.rings.real_mpfr.RealNumber._set (sage/rings/real_mpfr.c:8794)
    TypeError: Unable to convert x (='You') to real number.
**********************************************************************
4 items had failures:

The perennial noise issues in period_lattice.py:

sage -t  "devel/sage-tests/sage/schemes/elliptic_curves/period_lattice.py"
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/period_lattice.py", line 1111:
    sage: L.elliptic_exponential(z)
Expected:
    (1.06844510091205e-15 : 2.00000000000000 : 1.00000000000000)
Got:
    (1.06663809729124e-15 : 2.00000000000000 : 1.00000000000000)
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/period_lattice.py", line 1119:
    sage: L.elliptic_exponential(z)
Expected:
    (-1.0773137765183430387827930528831385613292568270511670949401e-60 : 2.0000000000000000000000000000000000000000000000000000000000 : 1.0000000000000000000000000000000000000000000000000000000000)
Got:
    (-1.0773140994173990301711557437150639986876632707626675784012e-60 : 2.0000000000000000000000000000000000000000000000000000000000 : 1.0000000000000000000000000000000000000000000000000000000000)

Lastly, in ell_rational_field.py, we have some more of the same plus a very weird issue with "rubenstein":

sage -t  "devel/sage-tests/sage/schemes/elliptic_curves/ell_rational_field.py"
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/ell_rational_field.py", line 1334:
    sage: E.analytic_rank(algorithm='rubinstein')
Exception raised:
    Traceback (most recent call last):
      File "/home/john/sage-4.3/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/home/john/sage-4.3/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/home/john/sage-4.3/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_28[4]>", line 1, in <module>
        E.analytic_rank(algorithm='rubinstein')###line 1334:
    sage: E.analytic_rank(algorithm='rubinstein')
      File "/home/john/sage-4.3/local/lib/python/site-packages/sage/schemes/elliptic_curves/ell_rational_field.py", line 1363, in analytic_rank
        raise RuntimeError, "unable to compute analytic rank using rubinstein algorithm ('%s')"%msg
    RuntimeError: unable to compute analytic rank using rubinstein algorithm ('unable to convert x (=eed to uncomment the line: PARI_DEFINE = -DINCLUDE_PARI
    in the Makefile and do: 'make clean', then 'make' if you wish to use
    elliptic curve L-functions. Requires that you already have pari installed
    on your machine.) to an integer')
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/ell_rational_field.py", line 1340:
    sage: E.analytic_rank(algorithm='all')
Exception raised:
    Traceback (most recent call last):
      File "/home/john/sage-4.3/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/home/john/sage-4.3/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/home/john/sage-4.3/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_28[6]>", line 1, in <module>
        E.analytic_rank(algorithm='all')###line 1340:
    sage: E.analytic_rank(algorithm='all')
      File "/home/john/sage-4.3/local/lib/python/site-packages/sage/schemes/elliptic_curves/ell_rational_field.py", line 1372, in analytic_rank
        self.analytic_rank('rubinstein'), self.analytic_rank('sympow')]))
      File "/home/john/sage-4.3/local/lib/python/site-packages/sage/schemes/elliptic_curves/ell_rational_field.py", line 1363, in analytic_rank
        raise RuntimeError, "unable to compute analytic rank using rubinstein algorithm ('%s')"%msg
    RuntimeError: unable to compute analytic rank using rubinstein algorithm ('unable to convert x (=eed to uncomment the line: PARI_DEFINE = -DINCLUDE_PARI
    in the Makefile and do: 'make clean', then 'make' if you wish to use
    elliptic curve L-functions. Requires that you already have pari installed
    on your machine.) to an integer')
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/ell_rational_field.py", line 1347:
    sage: EllipticCurve([1234567,89101112]).analytic_rank(algorithm='rubinstein')
Expected:
    Traceback (most recent call last):
    ...
    RuntimeError: unable to compute analytic rank using rubinstein algorithm ('unable to convert x (= 6.19283e+19 and is too large) to an integer')
Got:
    Traceback (most recent call last):
      File "/home/john/sage-4.3/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/home/john/sage-4.3/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/home/john/sage-4.3/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_28[7]>", line 1, in <module>
        EllipticCurve([Integer(1234567),Integer(89101112)]).analytic_rank(algorithm='rubinstein')###line 1347:
    sage: EllipticCurve([1234567,89101112]).analytic_rank(algorithm='rubinstein')
      File "/home/john/sage-4.3/local/lib/python/site-packages/sage/schemes/elliptic_curves/ell_rational_field.py", line 1363, in analytic_rank
        raise RuntimeError, "unable to compute analytic rank using rubinstein algorithm ('%s')"%msg
    RuntimeError: unable to compute analytic rank using rubinstein algorithm ('unable to convert x (=eed to uncomment the line: PARI_DEFINE = -DINCLUDE_PARI
    in the Makefile and do: 'make clean', then 'make' if you wish to use
    elliptic curve L-functions. Requires that you already have pari installed
    on your machine.) to an integer')
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/ell_rational_field.py", line 2901:
    sage: E.elliptic_exponential(z)
Expected:
    (-7.4445166218537606141680653627e-30 : 2.0000000000000000000000000000 : 1.0000000000000000000000000000)
Got:
    (-7.4445166228333392396252290308e-30 : 2.0000000000000000000000000000 : 1.0000000000000000000000000000)
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/ell_rational_field.py", line 2904:
    sage: E.elliptic_exponential(z)
Expected:
    (-1.0773137765183430387827930528831385613292568270511670949401e-60 : 2.0000000000000000000000000000000000000000000000000000000000 : 1.0000000000000000000000000000000000000000000000000000000000)
Got:
    (-1.0773140994173990301711557437150639986876632707626675784012e-60 : 2.0000000000000000000000000000000000000000000000000000000000 : 1.0000000000000000000000000000000000000000000000000000000000)
**********************************************************************
File "/home/john/sage-4.3/devel/sage-tests/sage/schemes/elliptic_curves/ell_rational_field.py", line 2916:
    sage: E.elliptic_exponential(P.elliptic_logarithm())
Expected:
    (-1.0000000000000000000000000000 + 4.3761255281366123414673626379e-31*I : 1.0000000000000000000000000000 - 1.4587084969798853407242847702e-31*I : 1.0000000000000000000000000000)
Got:
    (-1.0000000000000000000000000000 + 4.3761255295105644085291631657e-31*I : 1.0000000000000000000000000000 - 1.4587084969798853407242847702e-31*I : 1.0000000000000000000000000000)
**********************************************************************
2 items had failures:
   3 of   9 in __main__.example_28
   3 of  22 in __main__.example_61
***Test Failed*** 6 failures.
For whitespace errors, see the file /home/john/.sage//tmp/.doctest_ell_rational_field.py
	 [69.0 s]

So the conclusion is, regretfully, "needs work", though the work that needs doing is trivial compared with what has been done already (which is probably worth a PhD thesis for someone!).

Changed 10 years ago by was

apply this and the previous patch; then this should past all tests on 32-bit as well as 64-bit.

comment:17 Changed 10 years ago by was

  • Status changed from needs_work to needs_review

comment:18 Changed 10 years ago by was

I posted a new patch fixing all the issues above, except possibly the "very weird" ell_rational_field issue. I couldn't replicate that on any test machine yet.

I also didn't carefully reformat the docstrings. Which ones aren't formatted correctly? I thought I had got them right.

Changed 10 years ago by cremona

apply instead of previous one

comment:19 Changed 10 years ago by cremona

cremona>> The patches say they are based on 4.3 rather than 4.3.1. the first

one produced one failed hunk when I applied it to 4.3.1 (see attached -- looks minor).

was> That's in a doctest for something probably unrelated, so can be safely

ignored for refereeing.

Ignoring that, there was just one small thing (testing on 64-bit), namely

File "/home/jec/sage-4.3.1.alpha1/devel/sage-tests/sage/schemes/elliptic_curves/heegner.py", line 277:
    sage: hash((-7,5))
Expected:
    -3713088127102618519     
    1817441385
Got:
    -3713088127102618519

which is just because someone forgot to add the tag # 32-bit. I added that by editing the patch, and the corrected version is now attached here.

So: positive review on 64-bit. I'll test on 32-bit when I get home (leaving now, hope I do not get buried in the snow...)

Changed 10 years ago by cremona

Apply after previous (reviewer's patch fixing some docstring issues)

comment:20 Changed 10 years ago by cremona

  • Status changed from needs_review to positive_review

Positive review after testing on 32-bit, and also fixing lots of documentation glitches (in the reviewer's patch). These were mainly found by actually running "sage -docbuild all html" and reading the error messages (hint hint). Most common error was double :: where they shouldn't be.

Great work!

comment:21 Changed 10 years ago by rlm

  • Authors set to William Stein
  • Merged in set to sage-4.3.1.rc0
  • Resolution set to fixed
  • Status changed from positive_review to closed

Applied:

trac-6616-everything_rebased_against_4.3.patch
trac-6616-everything_rebased_against_4.3-part2.2.patch
trac-6616-review.patch

comment:22 Changed 10 years ago by mvngu

  • Summary changed from [with patch; needs review] refactor heegner points code out of ell_rational_field and support computing higher heegner points to refactor heegner points code out of ell_rational_field and support computing higher heegner points
Note: See TracTickets for help on using tickets.