Periodic Proportion Homomorphism over Finite Fields

[sagetrac-rlmiller]

Given a homormorphism, a prime p, and a degree n. Returns a table of the ratio of periodic
points to the number of points in a field of size p^n, as it cycles through the range of n or through the primes up to p.
One can organize table by ascending primes or by ascending degree. Skips the prime 2. Only works over finite fields.

        From paper by Zoë Bell, Karina Cho, Rebecca Lauren Miller, and Bianca Thompson.  Author Rebecca Lauren Miller. Implimented at
        Sage Days 94, Algorithm developed as part of Sage Days 90.

CC: @sagetrac-bthompson

Component: dynamics

Keywords: dynamical systems, sagedays90, days94, days90

Author: Rebecca Lauren Miller

Branch/Commit: u/rlmiller/bianca @ 32ffa30

Reviewer: Ben Hutz

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

[sagetrac-rlmiller]

Branch: u/rlmiller/bianca

[sagetrac-rlmiller]

New commits:

a7597f5

25745 periodic proportion hoomorphism

[sagetrac-rlmiller]

Commit: a7597f5

[bhutz]

comment:3

I can't look at this for a few days, so if someone else wants to review in the mean time go right ahead.

Just a couple quick comments

• this seems to return a table not a homomorphism, so I would name the function differently.

• It looks like you're expecting the field to be QQ (which is fine), but the docs should say where this should work and perhaps do a little input checking on the base field.

• docstrings need to start with a single line description

• paper should be added to reference file and referenced.

• go ahead and put the input checking doctests into TESTS rather than EXAMPLES

[sagetrac-git]

Changed commit from a7597f5 to 32ffa30

[sagetrac-git]

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

32ffa30

25745 updated tests and description

[bhutz]

Reviewer: Ben Hutz

[bhutz]

comment:5

I haven't built and checked values yet, since there are a bunch of things from looking through the code first:

• 'orderByPrime' -- camel case is reserved for classes, so don't use it in a parameter
  or as a variable 'perRatio'

• docs missing 's' -- Return a table of ratios of periodic point(s) to size of finite field.

• seems like you might want to specify a list/range of primes/n as input.

f.periodic_proportion_table(p=[3,7,13],n=[1,3])

having the short cut

f.periodic_proportion_table(p=3, n=4)

mean all primes up to p and all exp up to n is fine

• don't really understand the point of this test. The function isn't even in that class, so testing that the function isn't in the class is a little redundant since it will never fail.

sage: f.periodic_proportion_table(n=4)
Traceback (most recent call last):
...
AttributeError: 'DynamicalSystem_projective' object has no attribute 'periodic_proportion_table'

• Inputs need a blank line between them.

• I think the lines of the warning are not indented properly, but I didn't build the docs yet to check

• for

if is_prime(p) == False:

should be

if not is_prime():

• for

if n<=0 or not n in ZZ:

better as

n = ZZ(n)
if n <= 0:

• there is a table constructor, so if you sy you return a table, you should be returning a table

sage: rows = [['a', 'b', 'c'], [100,2,3], [4,5,60]]
sage: table(rows)

• space typos

NT. append([p, i+1, perRatio])
g=fp.cyclegraph()
PT. append([i, n, perRatio])

• the try/except seems to be just trying to catch indeterminacy locus issues and the error it raises seems to be specific to dimension 1.

cyclegraph can handle indeterminacy, so I don't really understand why you need this block. The warning you have seems to say that having an indeterminancy is ok, so I'm not sure what you really want here since the docs say you want a morphism? In my opinion, you are writing a function that is just returning the values to fill out the table, whether those values having any meaning is not really your business, so if it works for rational maps as well as morphisms, I see no reason to restrict to morphisms.

Also, I see no reason this can't be made to work in higher dimensions (other than it will be slow). You just need to adjust the cardinality calculation.

• you're comments in the doctests are not formatted properly

comment::

test

• the [4] mapping example needs the line wrapped it is much too long

• TESTS:: -- needs the double colon

• the big one that would help this function is to do these calculations in parallel. You are computing each p/n pair as completely independent, so this is best split into separate processes. The possible_periods() function in projective_ds gives a fairly simple example of this.