#18008 closed enhancement (fixed)
Periodic points for projective morphisms
Reported by:  gjorgenson  Owned by:  

Priority:  minor  Milestone:  sage6.7 
Component:  algebraic geometry  Keywords:  
Cc:  bhutz  Merged in:  
Authors:  Grayson Jorgenson  Reviewers:  Ben Hutz 
Report Upstream:  N/A  Work issues:  
Branch:  cfc0310 (Commits)  Commit:  
Dependencies:  Stopgaps: 
Description (last modified by )
Implement a function to compute the periodic points of a given period of a projective morphism. Should have the option to return only the minimal points of a given period.
Change History (24)
comment:1 Changed 3 years ago by
 Branch set to u/gjorgenson/ticket/18008
 Created changed from 03/20/15 16:48:48 to 03/20/15 16:48:48
 Modified changed from 03/20/15 16:48:48 to 03/20/15 16:48:48
comment:2 Changed 3 years ago by
 Commit set to e1299ee4b9c418eeb233156477b05916e16307a8
 Status changed from new to needs_review
comment:3 Changed 3 years ago by
 Reviewers set to Ben Hutz
 Status changed from needs_review to needs_work
 title: no affine
 spaces around =
 Fields, PolynomialRing? imports at top of file
 Fields should be Fields() in if (same for projective_homset)
 cleanup dsecription of periodic_points
 use set_verbose(None) instead of 1
 QQbar import at top of file
 both True and False example for periodic points
 Here is an interesting example:
P.<x,y,z>=ProjectiveSpace(QQ,2) H=Hom(P,P) f=H([x^2  21/16*z^2,y^22*z^2,z^2]) f.periodic_points(1)
 This example fails:
P.<x,y,z>=ProjectiveSpace(QuadraticField(5,'t'),2) H=Hom(P,P) f=H([x^2  21/16*z^2,y^2z^2,z^2]) f.periodic_points(2, True)
Some of the points returned are fixed points. It is probably from how .remove() is functioning
 A couple more examples, using number fields would be good too.
comment:4 Changed 3 years ago by
 Description modified (diff)
 Summary changed from Periodic points for projective/affine morphisms to Periodic points for projective morphisms
comment:5 Changed 3 years ago by
 Commit changed from e1299ee4b9c418eeb233156477b05916e16307a8 to 1aa42e43ea904f765c36034490a63de1c3ff5813
Branch pushed to git repo; I updated commit sha1. New commits:
1aa42e4  18008: implemented changes from review

comment:6 Changed 3 years ago by
 Status changed from needs_work to needs_review
comment:7 Changed 3 years ago by
 Status changed from needs_review to needs_work
A couple issues here. The new examples are good, but you need to fix the spacing. Also, while the new version of the .remove() should give expected results, it would be better to use a loop that simply worked backwards and removed them as you went.
range(len(points),1,1)
comment:8 Changed 3 years ago by
 Commit changed from 1aa42e43ea904f765c36034490a63de1c3ff5813 to e07cfed68d2cb88bc97e2370b58780b65ed03c0e
Branch pushed to git repo; I updated commit sha1. New commits:
e07cfed  18008: modified iteration method in periodic_points and fixed spacing in examples

comment:9 Changed 3 years ago by
 Status changed from needs_work to needs_review
comment:10 Changed 3 years ago by
 Status changed from needs_review to needs_work
Just a couple things here. The new removal does now get the correct points returned, so that's good.
However, there is now a doctest failure in algebraic_scheme.py. I assume this is because it is calling the dimension 0 version instead of enumeration and so is getting the points in a different order.
File "src/sage/schemes/generic/algebraic_scheme.py", line 1461, in sage.schemes.generic.algebraic_scheme.AlgebraicScheme_subscheme.rational_points Failed example: X.rational_points(3) Expected: [(1 : 1), (1 : 1)] Got: [(1 : 1), (1 : 1)]
Also, I missed this last time, but the call
X.defining_ideal().dimension()
is a groebner basis calculation. There two places where you call it just a couple lines apart. You should only call it once and store the value.
comment:11 Changed 3 years ago by
 Commit changed from e07cfed68d2cb88bc97e2370b58780b65ed03c0e to 20826af069a90f4dd7895098961f290ba2437688
Branch pushed to git repo; I updated commit sha1. New commits:
20826af  18008: removed redundant function calls and added sorting for returned lists

comment:12 Changed 3 years ago by
 Status changed from needs_work to needs_review
The enumerate functions sort their lists of points before returning, so I added sorting for the dimension zero cases. I updated the output in the examples for periodic_points to match the new sorted lists.
comment:13 Changed 3 years ago by
The functionality is fine, but what is this
from pickle import INST
on the first line of projective_homset.py?
comment:14 Changed 3 years ago by
I'm not sure. I think it has something to do with testing pickles, but I don't know much about that. It's defined in misc/explain_pickle.py, and the only place in the sage directory that the import is present is projective_homset. Everything appears to run fine without it; do you think it would be okay to remove it? It might just be remnants from testing that was done before.
comment:15 Changed 3 years ago by
I was asking since it looks like you added it in the e1299ee commit. It doesn't seem to be used in the file. If you did not add it for a particular purpose, then I would remove it.
comment:16 Changed 3 years ago by
 Commit changed from 20826af069a90f4dd7895098961f290ba2437688 to 3620a01bc805211f2fdf7ffbce7eee16f1908771
Branch pushed to git repo; I updated commit sha1. New commits:
3620a01  18008: removed unnecessary import

comment:17 Changed 3 years ago by
Oops, I didn't notice that it was added in an earlier commit. I don't think it was intentional.
comment:18 Changed 3 years ago by
 Status changed from needs_review to positive_review
comment:19 Changed 3 years ago by
 Status changed from positive_review to needs_work
Documentation does not build
[plane_cur] reading sources... [ 82%] sage/schemes/hyperelliptic_curves/monsky_washnitzer Error building the documentation. Traceback (most recent call last): File "/mnt/disk/home/release/Sage/src/doc/common/builder.py", line 1626, in <module> getattr(get_builder(name), type)() File "/mnt/disk/home/release/Sage/src/doc/common/builder.py", line 292, in _wrapper getattr(get_builder(document), 'inventory')(*args, **kwds) File "/mnt/disk/home/release/Sage/src/doc/common/builder.py", line 503, in _wrapper x.get(99999) File "/mnt/disk/home/release/Sage/local/lib/python/multiprocessing/pool.py", line 558, in get raise self._value OSError: [schemes ] /mnt/disk/home/release/Sage/local/lib/python2.7/sitepackages/sage/schemes/projective/projective_morphism.py:docstring of sage.schemes.projective.projective_morphism.SchemeMorphism_polynomial_projective_space.periodic_points:9: WARNING: Bullet list ends without a blank line; unexpected unindent. Makefile:60: recipe for target 'dochtml' failed make: *** [dochtml] Error 1
comment:20 Changed 3 years ago by
 Commit changed from 3620a01bc805211f2fdf7ffbce7eee16f1908771 to cfc0310f9efded39d56de4e9f67ec8285403867f
Branch pushed to git repo; I updated commit sha1. New commits:
cfc0310  18008: fixed indent in documentation

comment:21 Changed 3 years ago by
 Status changed from needs_work to needs_review
Sorry. I should have caught that.
comment:22 Changed 3 years ago by
 Status changed from needs_review to positive_review
oops, I thought I'd checked the docs, but apparently not. They build now with the latest fix.
comment:23 Changed 3 years ago by
 Branch changed from u/gjorgenson/ticket/18008 to cfc0310f9efded39d56de4e9f67ec8285403867f
 Resolution set to fixed
 Status changed from positive_review to closed
comment:24 Changed 3 years ago by
 Commit cfc0310f9efded39d56de4e9f67ec8285403867f deleted
 Milestone changed from sage6.6 to sage6.7
New commits:
18008: First part of periodic points implementation
18008: first pass at implementing periodic points function.