Opened 4 years ago
Last modified 4 years ago
#25745 needs_work enhancement
Periodic Proportion Homomorphism over Finite Fields
Reported by:  Rebecca Lauren Miller  Owned by:  

Priority:  minor  Milestone:  sage8.3 
Component:  dynamics  Keywords:  dynamical systems, sagedays90, days94, days90 
Cc:  Bianca Thompson  Merged in:  
Authors:  Rebecca Lauren Miller  Reviewers:  Ben Hutz 
Report Upstream:  N/A  Work issues:  
Branch:  u/rlmiller/bianca (Commits, GitHub, GitLab)  Commit:  32ffa3092164fc097cd77beeac91eb93902fa926 
Dependencies:  Stopgaps: 
Description
Given a homormorphism, a prime
p
, and a degreen
. Returns a table of the ratio of periodicpoints to the number of points in a field of size
p^n
, as it cycles through the range ofn
or through the primes up top
. 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.
Change History (5)
comment:1 Changed 4 years ago by
Branch:  → u/rlmiller/bianca 

comment:2 Changed 4 years ago by
Commit:  → a7597f58e880ef76c1e3ce8f9e6fa95c0571a82d 

Status:  new → needs_review 
comment:3 Changed 4 years ago by
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
comment:4 Changed 4 years ago by
Commit:  a7597f58e880ef76c1e3ce8f9e6fa95c0571a82d → 32ffa3092164fc097cd77beeac91eb93902fa926 

Branch pushed to git repo; I updated commit sha1. New commits:
32ffa30  25745 updated tests and description

comment:5 Changed 4 years ago by
Reviewers:  → Ben Hutz 

Status:  needs_review → needs_work 
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.
New commits:
25745 periodic proportion hoomorphism