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 Curve Enumeration by height #13436

Open
haikona mannequin opened this issue Sep 6, 2012 · 28 comments
Open

Elliptic Curve Enumeration by height #13436

haikona mannequin opened this issue Sep 6, 2012 · 28 comments

Comments

@haikona
Copy link
Mannequin

haikona mannequin commented Sep 6, 2012

Add functionality to Sage that allows one to enumerate elliptic curves over Q by height for a range of Weierstrass families of curve.

This code has grown from work with Wei Ho, who desired a way to enumerate curves over Q by height (where the height of a curve is a function of the coefficients in its Weierstrass model) so that the average value of some datum (e.g.rank, 2-Selmer size) could be computed for curves up to a given height. This code introduces a class, CurveEnumerator(), that computes lists of curves ordered by height for this purpose.

https://github.com/haikona/CBH

Component: elliptic curves

Keywords: enumeration

Author: Simon Spicer

Branch/Commit: u/chapoton/13436 @ 8721f1d

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

@haikona haikona mannequin added this to the sage-5.11 milestone Sep 6, 2012
@haikona haikona mannequin self-assigned this Sep 6, 2012
@haikona
Copy link
Mannequin Author

haikona mannequin commented Sep 17, 2012

Attachment: trac_13436_curve_enumerator.patch.gz

@haikona
Copy link
Mannequin Author

haikona mannequin commented Sep 17, 2012

comment:1

Version 1.0 patch posted. All new code (except for an added import statement in sage.schemes.elliptic_curves.all) is in a new file, curve_enumerator.py, in the sage.schemes.elliptic_curves directory. It's thoroughly documented and doctested, so hopefully its logic will straightforward enough to follow.

@haikona haikona mannequin added the s: needs review label Sep 17, 2012
@williamstein

This comment has been minimized.

@williamstein
Copy link
Contributor

comment:3

For the three_selmer function, you need a doctest.

sage: magma('2+2')  # optional - magma

Then

sage -t --only_optional=magma curve_enumerator.py 

to test.

@williamstein
Copy link
Contributor

comment:4

Fix some docstring formatitng, e.g., the funny indent:

    def heights(self, lowerbound, upperbound):  
        """
        Return a list of permissable curve heights in the specified range
         (bounds inclusive), and for each height the equation coefficients
         that produce curves of that height.

This should output "100%":

...elliptic_curves$ sage -coverage curve_enumerator.py 
----------------------------------------------------------------------
curve_enumerator.py
ERROR: Please add a `TestSuite(s).run()` doctest.
SCORE curve_enumerator.py: 87% (35 of 40)

Missing documentation:
	 * __init__(self):
	 * __repr__(self):
	 * _coeffs_to_a_invariants(self, c):


Missing doctests:
	 * __init__(self):
	 * three_selmer(self, curves, rank=True, reduced=False, output_filename=None, problems_filename=None, \ proof=True, return_data=True, print_timing=True):

@williamstein
Copy link
Contributor

comment:5
  • Here: over Q up I would put `\QQ` or at QQ.

  • Either: Or: -- maybe instead a sublist of two things?

  • weird formatting:

sage: C = EllipticCurveEnumerator(family="short_weierstrass")
sage: C.n_torsion?

       * "rank"              -- (Default True): Compute the rank versus
         size of the curve's torsion/n-torsion subgroup. If True, the
         n-torsion rank is computed (i.e. 0, 1 or 2). If set to False, the
         size of the n-torsion subgroup is computed instead (i.e. n^rank)
    
         * "output_filename" -- (Default None): If not None, the string
    
         name of the file to which the output will be saved.
  • Restructure code so that this is natural:
v = EllipticCurveEnumerator(family="short_weierstrass", height=1000).ranks()

Will need caching:

sage: from sage.misc.cachefunc import cached_method
sage: cached_method?

@ohanar
Copy link
Member

ohanar commented Sep 25, 2012

comment:6

There are three AssertionErrors that should be ValueErrors.

also two_selmer and three_selmer have an extra space for the OUTPUT portion of the docstring.

@fchapoton
Copy link
Contributor

comment:8

I have uploaded a patch that cleans up the documentation.

Still needs works, I think

@fchapoton
Copy link
Contributor

comment:9

could please someone with magma add the missing doctest for "three_selmer" ?

@JohnCremona
Copy link
Member

comment:10

I applied the patches to 5.10 and ran the doctest and report the result here.

sage: C = EllipticCurveEnumerator(family="short_weierstrass") 
sage: L = C.coefficients_over_height_range(4, 4)                                                               
sage: R, P = C.three_selmer(L, rank=True,return_data=True,print_timing=False)                                  
sage: R                                                                                                        
[(4, [0, 0, 0, -1, -2], 1),
 (4, [0, 0, 0, -1, 2], 0),
 (4, [0, 0, 0, 0, -2], 1),
 (4, [0, 0, 0, 0, 2], 1),
 (4, [0, 0, 0, 1, -2], 0),
 (4, [0, 0, 0, 1, 2], 0)]
sage: P                                                                                                        
[]

@fchapoton
Copy link
Contributor

comment:11

Attachment: trac_13436_curve_enumerator-addon1.patch.gz

thanks for the help !

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@fchapoton
Copy link
Contributor

Commit: a3af039

@fchapoton
Copy link
Contributor

New commits:

a3af039trac 13436 : doc corrections
e405730Add functionality to Sage that allows one to enumerate elliptic curves over Q by height for a range of Weierstrass families of curve.

@fchapoton
Copy link
Contributor

Branch: u/chapoton/13436

@JohnCremona
Copy link
Member

comment:14

Thanks for converting this to git and fixing the docstring formatting (which I will trust has been done properly)!

Authors: can you remove the place(s) where there are comments "this should be checked" or "todo"? And in several similar places you can delete the comment that some integers must be Sage integers or ... some rather frivolous consequence will occur?

I see that you have carefully allowed for the unavoidable fact that there will be "problem curves" and have an option output filename for these. But I don't think that any of the doctests illustrate this -- please add one or more doctests which do. Meanwhile I am trying the functions out.

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 7, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

ba097ecMerge branch 'u/chapoton/13436' of ssh://trac.sagemath.org:22/sage into 13436
0f75b4ftrac #13436 minor changes after some of the reviewer's comments

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 7, 2014

Changed commit from a3af039 to 0f75b4f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 3, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

b4b547bMerge branch 'u/chapoton/13436' into 6.8.b2
512eb17trac #13436 fixing one error

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 3, 2015

Changed commit from 0f75b4f to 512eb17

@fchapoton
Copy link
Contributor

comment:20

There is still one failing doctest, that I do not understand.

@fchapoton fchapoton modified the milestones: sage-6.4, sage-6.8 Jun 3, 2015
@fchapoton fchapoton modified the milestones: sage-6.8, sage-6.10 Nov 14, 2015
@fchapoton fchapoton modified the milestones: sage-6.10, sage-7.0 Jan 6, 2016
@fchapoton fchapoton modified the milestones: sage-7.0, sage-7.1 Jan 21, 2016
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 5, 2016

Changed commit from 512eb17 to 3fc917c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 5, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

d4fe9b6Merge branch 'u/chapoton/13436' into 7.1.b2
73502c0trac #13436 updating cartesian_products
3fc917ctrac #13436 pep8

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 13, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

850d7a3Merge branch 'u/chapoton/13436' into 7.2.b4
3785184trac #13426 fixing one import

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 13, 2016

Changed commit from 3fc917c to 3785184

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 29, 2016

Changed commit from 3785184 to 23fa861

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 29, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

23fa861Merge branch 'u/chapoton/13436' in 7.3.rc0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 1, 2017

Branch pushed to git repo; I updated commit sha1. New commits:

04d7e40Merge branch 'u/chapoton/13436' in 7.6.b5
8721f1dtrac 13436 details

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 1, 2017

Changed commit from 23fa861 to 8721f1d

@mkoeppe mkoeppe removed this from the sage-7.1 milestone Dec 29, 2022
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

6 participants