Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

elliptic curves -- heegner_index command gives nonsense when rank > 1 #4453

Closed
williamstein opened this issue Nov 6, 2008 · 29 comments
Closed

Comments

@williamstein
Copy link
Contributor

For any elliptic curve over QQ of rank >= 2 the heegner_index command must always give +Infinity as output. So the following 1 at the end is just wrong.

sage: E = EllipticCurve('389a')
sage: D = E.heegner_discriminants_list(1)[0]
sage: D
-7
sage: E.heegner_index(D)
1

Apply trac_4453-rebased.patch and 4453-doctest-fix.patch

Component: elliptic curves

Author: William Stein

Reviewer: John Cremona, Robert Bradshaw

Merged: sage-5.5.beta1

Issue created by migration from https://trac.sagemath.org/ticket/4453

@williamstein williamstein added this to the sage-4.5.2 milestone Nov 6, 2008
@williamstein williamstein self-assigned this Nov 6, 2008
@loefflerd loefflerd mannequin assigned loefflerd and unassigned williamstein Jul 20, 2009
@loefflerd loefflerd mannequin removed their assignment Oct 9, 2009
@robertwb
Copy link
Contributor

comment:4

Depends on #6616?

@JohnCremona
Copy link
Member

Author: was

@JohnCremona
Copy link
Member

Reviewer: cremona

@JohnCremona
Copy link
Member

comment:5

Positive review: looks good (apart from a docstring typo "We higher rank examples") and applies fine to 4.3.1, tests pass and docs build & look good.

I added a new patch which changes the above to "Some higher rank examples" with no further changes.

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Feb 13, 2010

comment:6

I get two hunk failures when applying trac_4453.2.patch on top of Sage 4.3.3.alpha0:

[mvngu@sage sage-main]$ pwd
/dev/shm/mvngu/sage-4.3.3.alpha0/devel/sage-main
[mvngu@sage sage-main]$ hg qimport https://github.com/sagemath/sage/files/ticket4453/trac_4453.2.patch.gz && hg qpush
adding trac_4453.2.patch to series file
applying trac_4453.2.patch
patching file sage/schemes/elliptic_curves/heegner.py
Hunk #3 FAILED at 6350
Hunk #4 succeeded at 6370 with fuzz 2 (offset -12 lines).
Hunk #5 FAILED at 6430
2 out of 5 hunks FAILED -- saving rejects to file sage/schemes/elliptic_curves/heegner.py.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh trac_4453.2.patch

The attachment trac_4453.2.patch needs rebase against Sage 4.3.3.alpha0.

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Feb 13, 2010

Work Issues: rebase against Sage 4.3.3.alpha0

@robertwb
Copy link
Contributor

Attachment: trac_4453-rebased.patch.gz

@robertwb
Copy link
Contributor

comment:7

Rebased, I also fixed a small error when check_rank=False.

@qed777
Copy link
Mannequin

qed777 mannequin commented Aug 7, 2010

Changed reviewer from cremona to John Cremona

@qed777
Copy link
Mannequin

qed777 mannequin commented Aug 7, 2010

Changed author from was to William Stein

@qed777
Copy link
Mannequin

qed777 mannequin commented Aug 7, 2010

Changed work issues from rebase against Sage 4.3.3.alpha0 to none

@qed777
Copy link
Mannequin

qed777 mannequin commented Aug 7, 2010

comment:8

With the forthcoming 4.5.2 (4.5.2.rc0 + #9226) and the rebased patch, I get a doctest failure:

sage -t -long "devel/sage/sage/schemes/elliptic_curves/heegner.py"
**********************************************************************
File "/mnt/usb1/scratch/mpatel/apps/sage-4.5.2/devel/sage/sage/schemes/elliptic_curves/heegner.py", line 6465:
    sage: E.heegner_index(-8, descent_second_limit=16)
Exception raised:
    Traceback (most recent call last):
      File "/mnt/usb1/scratch/mpatel/apps/sage-4.5.2/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/mnt/usb1/scratch/mpatel/apps/sage-4.5.2/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/mnt/usb1/scratch/mpatel/apps/sage-4.5.2/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_229[14]>", line 1, in <module>
        E.heegner_index(-Integer(8), descent_second_limit=Integer(16))###line 6465: 
    sage: E.heegner_index(-8, descent_second_limit=16)
      File "/mnt/usb1/scratch/mpatel/apps/sage-4.5.2/local/lib/python/site-packages/sage/schemes/elliptic_curves/heegner.py", line 6485, in heegner_index
        if check_rank and self.rank() >= 2:
      File "/mnt/usb1/scratch/mpatel/apps/sage-4.5.2/local/lib/python/site-packages/sage/schemes/elliptic_curves/ell_rational_field.py", line 1741, in rank
        raise RuntimeError, 'Rank not provably correct.'
    RuntimeError: Rank not provably correct.
**********************************************************************
1 items had failures:
   1 of  21 in __main__.example_229
***Test Failed*** 1 failures.

@qed777 qed777 mannequin removed this from the sage-4.5.2 milestone Aug 7, 2010
@robertwb
Copy link
Contributor

comment:14

Attachment: 4453-doctest-fix.patch.gz

Apply trac_4453-rebased.patch, 4453-doctest-fix.patch

@robertwb
Copy link
Contributor

Changed reviewer from John Cremona to John Cremona, Robert Bradshaw

@robertwb
Copy link
Contributor

Changed work issues from one doctest fails to none

@zimmermann6
Copy link

comment:16

Robert, I find it strange that you give a positive review to your own patch...

Paul

@robertwb
Copy link
Contributor

comment:17

I was giving a positive review to the original patch; I suppose someone should review my 1-line doctest fix as well.

@JohnCremona
Copy link
Member

comment:19

I'll check Robert's one-liner but not this week.

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Oct 24, 2012

comment:20

Rob's patch looks fine to me, and patchbot's happy with it, so let's get this in.

@jdemeyer jdemeyer modified the milestones: sage-5.4, sage-5.5 Oct 29, 2012
@jdemeyer
Copy link

comment:22

Please specify which patch(es) to apply.

@robertwb

This comment has been minimized.

@jdemeyer
Copy link

jdemeyer commented Nov 1, 2012

Merged: sage-5.5.beta1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants