Return the points in the ring of integers after renormalization

[guojing0]

📚 Description

points_of_bounded_height function in proj_bdd_height.py now returns correct results after renormalization by scaling, if in the ring of integers.

📝 Checklist

• The title is concise, informative, and self-explanatory.
• The description explains in detail what this PR is about.
• I have linked a relevant issue or discussion.
• I have created tests covering the changes.
• I have updated the documentation accordingly.

⌛ Dependencies

[bhutz]

Two comments on this:

1. Just scaling by the lcm of the denominator does not guarantee that you will get something in the ring of integers:

R.=QQ[]
K. = NumberField(3*x^2 + 1)
O=K.maximal_order()
P.<z,w> = ProjectiveSpace(K, 1)
for Q in list(P.points_of_bounded_height(bound=2)):
Q.scale_by(lcm([idx.denominator() for idx in Q]))
print(Q, [t in O for t in Q])`

2. You should only be clearing denominators like this when getting points in the ring of integers. So the output of

sage: P.<x,y> = ProjectiveSpace(QQ, 1)
sage: sorted(list(P.points_of_bounded_height(bound=2)))

should remain unchanged.

[bhutz]

As I said earlier, scaling by the lcm of the denominators is not enough. You changed the example to include the additional 1/3 term, which is masking the underlying issue. This needs to work for the map

phi = DynamicalSystem_projective([a*(z^2 + w^2),z*w])

[guojing0]

I have simplified the code as mentioned in the meeting, however, the output for this example is still unchanged.

sage: K.<v> = QuadraticField(2)
sage: P.<x,y> = ProjectiveSpace(K, 1)
sage: sorted(list(P.points_of_bounded_height(bound=2)))
[(-v - 2 : 1), (-v - 2 : 2), (-v - 1 : 1), (-2 : 1), (-2 : 4), (-v : 1),
 (-v : 2), (-1 : 1), (v - 2 : 1), (v - 2 : 2), (-v + 1 : 1), (0 : 1),
 (v - 1 : 1), (-v + 2 : 1), (-v + 2 : 2), (1 : 0), (1 : 1), (v : 1),
 (v : 2), (2 : 1), (2 : 4), (v + 1 : 1), (v + 2 : 1), (v + 2 : 2)]

[bhutz]

A few comments here:

1. The new function for ZZ only works for dimension 1. You need to do something similar to the QQ function.

The following give incorrect results and an error, respectively.

sorted(list(ZZ_points_of_bounded_height(2, 1)))
sorted(list(ZZ_points_of_bounded_height(3, 1)))

2. need an example for maximal orders in the help functions. In general, the github code review is pointing out a number of branches in the code that are not being covered by the tests.

3. line 434. This function should never be called with O=ZZ, so this part of the check is not needed

4. In the user exposed function in projective_space.py You get errors when the base ring is a more general ring (i.e. not a number field or a ring of integers):

R.<x> = QQ[]
P.<z,w> = ProjectiveSpace(R, 1)
P.points_of_bounded_height(bound=2)

These types of rings should return a more helpful error (such as something about it must be a numberfield or maximal order)

[bhutz]

I only found a couple things in my testing:

• line 312 O is not used in this function so should be removed

• The QQ and ZZ helper functions are having domain problems. Since they are creating their own projective spaces, the points returned are not in the correct space: This needs to be fixed. Looks like this is an old problem for the QQ function, but should be fixed here too.

P.<z,w> = ProjectiveSpace(ZZ, 1)
for Q in P.points_of_bounded_height(bound=2):
    print(Q in P,Q.codomain() is P)

• line1166 change the grouping to (if ZZ) or (is order and is int closed):

• In looking at the normalize, I was surprised that you weren't getting errors thrown since you are defining the projective point in projective space over the ring of integers prior to doing the normalization step (i.e., normalization isn't doing anything). So I looked back at Krumm's algorithm and he is in fact choosing coordinates in the ring of integers already. So for IQ_points and the general points_of it looks like normalization is not actually needed.

• All docs built ok for me.

[github-actions]

Documentation preview for this PR (built with commit 0ed83e9; changes) is ready! 🎉

[bhutz]

I'm not getting any more issues when I'm testing. Looks like all docs pass.

[fchapoton]

this has broken our linter for rst syntax

[guojing0]

this has broken our linter for rst syntax

Fixed by #35974