Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#18008 closed enhancement (fixed)

Periodic points for projective morphisms

Reported by: gjorgenson Owned by:
Priority: minor Milestone: sage-6.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 gjorgenson)

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 2 years ago by gjorgenson

  • 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 2 years ago by gjorgenson

  • Commit set to e1299ee4b9c418eeb233156477b05916e16307a8
  • Status changed from new to needs_review

New commits:

bfc6c1d18008: First part of periodic points implementation
e1299ee18008: first pass at implementing periodic points function.

comment:3 Changed 2 years ago by bhutz

  • 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)
  • clean-up 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^2-2*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^2-z^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 2 years ago by gjorgenson

  • Description modified (diff)
  • Summary changed from Periodic points for projective/affine morphisms to Periodic points for projective morphisms

comment:5 Changed 2 years ago by git

  • Commit changed from e1299ee4b9c418eeb233156477b05916e16307a8 to 1aa42e43ea904f765c36034490a63de1c3ff5813

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

1aa42e418008: implemented changes from review

comment:6 Changed 2 years ago by gjorgenson

  • Status changed from needs_work to needs_review

comment:7 Changed 2 years ago by bhutz

  • 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 2 years ago by git

  • Commit changed from 1aa42e43ea904f765c36034490a63de1c3ff5813 to e07cfed68d2cb88bc97e2370b58780b65ed03c0e

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

e07cfed18008: modified iteration method in periodic_points and fixed spacing in examples

comment:9 Changed 2 years ago by gjorgenson

  • Status changed from needs_work to needs_review

comment:10 Changed 2 years ago by bhutz

  • 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 2 years ago by git

  • Commit changed from e07cfed68d2cb88bc97e2370b58780b65ed03c0e to 20826af069a90f4dd7895098961f290ba2437688

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

20826af18008: removed redundant function calls and added sorting for returned lists

comment:12 Changed 2 years ago by gjorgenson

  • 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 2 years ago by bhutz

The functionality is fine, but what is this

from pickle import INST

on the first line of projective_homset.py?

comment:14 Changed 2 years ago by gjorgenson

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 2 years ago by bhutz

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 2 years ago by git

  • Commit changed from 20826af069a90f4dd7895098961f290ba2437688 to 3620a01bc805211f2fdf7ffbce7eee16f1908771

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

3620a0118008: removed unnecessary import

comment:17 Changed 2 years ago by gjorgenson

Oops, I didn't notice that it was added in an earlier commit. I don't think it was intentional.

comment:18 Changed 2 years ago by bhutz

  • Status changed from needs_review to positive_review

comment:19 Changed 2 years ago by vbraun

  • 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/site-packages/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 'doc-html' failed
make: *** [doc-html] Error 1

comment:20 Changed 2 years ago by git

  • Commit changed from 3620a01bc805211f2fdf7ffbce7eee16f1908771 to cfc0310f9efded39d56de4e9f67ec8285403867f

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

cfc031018008: fixed indent in documentation

comment:21 Changed 2 years ago by gjorgenson

  • Status changed from needs_work to needs_review

Sorry. I should have caught that.

comment:22 Changed 2 years ago by bhutz

  • 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 2 years ago by vbraun

  • Branch changed from u/gjorgenson/ticket/18008 to cfc0310f9efded39d56de4e9f67ec8285403867f
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:24 Changed 2 years ago by bhutz

  • Commit cfc0310f9efded39d56de4e9f67ec8285403867f deleted
  • Milestone changed from sage-6.6 to sage-6.7
Note: See TracTickets for help on using tickets.