#17386 closed enhancement (fixed)
Enumerate points of bounded height in projective/affine space over number fields
Reported by:  gjorgenson  Owned by:  

Priority:  minor  Milestone:  sage6.6 
Component:  algebraic geometry  Keywords:  
Cc:  bhutz  Merged in:  
Authors:  Grayson Jorgenson  Reviewers:  Ben Hutz 
Report Upstream:  N/A  Work issues:  
Branch:  63b6422 (Commits)  Commit:  
Dependencies:  Stopgaps: 
Description
Add functionality to iterate over points of bounded height in projective/affine space over a number field. Also add functionality to list the points of height less than a given bound on a scheme defined over a number field.
Change History (13)
comment:1 Changed 4 years ago by
 Branch set to u/gjorgenson/ticket/17386
 Created changed from 11/24/14 05:26:20 to 11/24/14 05:26:20
 Modified changed from 11/24/14 05:26:20 to 11/24/14 05:26:20
comment:2 Changed 4 years ago by
 Commit set to 456c7213880388ca6311348c55a798cf5a9b1187
comment:3 Changed 4 years ago by
 Status changed from new to needs_review
comment:4 Changed 4 years ago by
 Reviewers set to Ben Hutz
 Status changed from needs_review to needs_work
Here is a first pass at reviewing this.
First you need to be careful with the difference between relative height and absolute height. The function K.elements_of_bounded_height() is returning with respect to a bound of relative height (as its documentation says). I would think most users would be interested in absolute height rational points for schemes. So you'll need to fix the functionality here, but also fix the documentation to specify which height in all appropriate places.
Also, specify whether the bound is <= or <
numberField
is not a good variable name as CamelCase is reserved for class names.
In projective_homset.py put the imports near where they are needed so you only import what you need (this is done correctly in affine_homset.py)
Some examples that I expected to work that didn't
u = QQ['u'].0 K.<v> = NumberField(u^2 + 3) A.<x,y> = ProjectiveSpace(K,1) X=A.subscheme(xy) X.rational_points(3)
same for affine
from sage.schemes.projective.projective_rational_point import enum_projective_number_field enum_projective_number_field(X,3)
from sage.schemes.affine.affine_rational_point import enum_affine_number_field enum_affine_number_field(X,3)
I think the issues with the enum_ functions is whether the input should be the scheme X
or the point set X(K)
. Clarify which in the documentation for the functions. However, the X.rational_points ones seem to actually be failing.
comment:5 Changed 4 years ago by
 Commit changed from 456c7213880388ca6311348c55a798cf5a9b1187 to 003884c155f7fc53bd649617fc5c983e545aefbd
comment:6 Changed 4 years ago by
 Status changed from needs_work to needs_review
comment:7 Changed 4 years ago by
 Status changed from needs_review to needs_work
Looks like we are almost there with this one. The functionality all is in working order. But, I came across a few minor things to fix.
affine_rational_point example line 168 is a long test so add > # long time probably makes more sense to make the bound smaller and get the test < 1s
affine_space > line 777 not needed, but move the comment to 779
affine_space > points_of_bounded_height > add comment that this is using the KrummDoyle algorithm for number fields (get the citation from number fields where they implemented it).
algebraic_scheme > line 1502 no longer need is_RationalField(F)
projective_space > points_of_bounded_height > add comment that this is using the KrummDoyle algorithm for number fields (get the citation from number fields where they implemented it).
projective_space > line 94 not needed > move comment to 951
There are a few places where the lines in the docs are too long. You need to wrap those. Such as line 748 in affine_space.py
Another bad example:
u = QQ['u'].0 K.<v> = NumberField(u^2 + 3) A.<x,y> = ProjectiveSpace(K,1) X=A.subscheme(xy) X.rational_points(3) from sage.schemes.affine.affine_rational_point import enum_affine_number_field enum_affine_number_field(X,3)
oops...X is projective and we called enum_affine successfully!! Need a check in those functions to make sure the ambient space is Affine or Projective
comment:8 Changed 4 years ago by
 Commit changed from 003884c155f7fc53bd649617fc5c983e545aefbd to f786719b1183eb412e2898deb002b9e3cf3603d4
Branch pushed to git repo; I updated commit sha1. New commits:
f786719  17386: Implemented changes from second review. In addition to these changes, also added checks in

comment:9 Changed 4 years ago by
 Status changed from needs_work to needs_review
comment:10 Changed 4 years ago by
 Commit changed from f786719b1183eb412e2898deb002b9e3cf3603d4 to 63b6422759a26bc2393a952b186731296e14fe48
Branch pushed to git repo; I updated commit sha1. New commits:
63b6422  17386: removed uneeded references

comment:11 Changed 4 years ago by
 Status changed from needs_review to positive_review
Ok. this looks good now.
comment:12 Changed 4 years ago by
 Branch changed from u/gjorgenson/ticket/17386 to 63b6422759a26bc2393a952b186731296e14fe48
 Resolution set to fixed
 Status changed from positive_review to closed
comment:13 Changed 4 years ago by
 Commit 63b6422759a26bc2393a952b186731296e14fe48 deleted
 Milestone changed from sage6.5 to sage6.6
Branch pushed to git repo; I updated commit sha1. New commits:
Added enumeration functionality for affine space over number fields.