Opened 4 years ago
Closed 4 years ago
#22835 closed enhancement (fixed)
molien series for finite matrix gap groups in char 0
Reported by:  bhutz  Owned by:  

Priority:  minor  Milestone:  sage8.0 
Component:  group theory  Keywords:  
Cc:  rlmiller  Merged in:  
Authors:  Ben Hutz  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  6f8c944 (Commits, GitHub, GitLab)  Commit:  6f8c944e2801f37ebee75b32e45438d1fe30f453 
Dependencies:  #22783  Stopgaps: 
Description (last modified by )
Compute the molien series for any finite matrix group in characteristic zero that has a _gap_ function. This includes the computation of molien series relative to nontrivial characters
Change History (15)
comment:1 Changed 4 years ago by
 Branch set to u/bhutz/molien_series
 Commit set to ecfa8456ea4c1ae75d7cdf131b94bcf7b0b9142c
 Description modified (diff)
 Status changed from new to needs_review
comment:2 Changed 4 years ago by
 Dependencies set to #22783
This requires #22783, which is already closed.
comment:3 Changed 4 years ago by
A few comments:
 Why restrict to characteristic
0
? From the Wikipedia page, it just requires the characteristic of the field not to divide the order of the group (which you need anyways).  You should use:
INPUT:  ``xi``  (default: trivial character) a linear group character of this group  ``return_series``  boolean (default: ``True``) if ``True``, then returns the Molien series as a power series, ``False`` as a rational function  ``prec``  integer (default: 20); power series default precision  ``variable``  string (default: ``'t'``); Variable name for the Molien series
 I would use
mol = 1/self.order() * mol +mol /= self.order()
 You have some inconsistent spacing around the operators, and there are a number of unnecessary parentheses that (IMO) makes it harder to read.
comment:4 Changed 4 years ago by
 Status changed from needs_review to needs_work
Thanks for the comments.
For the characteristic: there are a number of problems with the Molien series entry in wikipedia, one of which is the formula for char p. While there is a formula assuming nondivisibility, I think it is somewhat more complicated that than. DekkerdeJong is the reference. I was just going to leave it for the future, but I'll take another look at their paper and see how hard it would be.
comment:5 Changed 4 years ago by
Ah, okay. Don't worry too much if positive characteristic seems like a lot of work.
comment:6 Changed 4 years ago by
 Commit changed from ecfa8456ea4c1ae75d7cdf131b94bcf7b0b9142c to a282b5e61fb968dd7eaeb20e4bf18a06ecfe69c7
Branch pushed to git repo; I updated commit sha1. New commits:
a282b5e  22853: add support for char p>0

comment:7 Changed 4 years ago by
 Status changed from needs_work to needs_review
It wasn't too bad to add char>0 support. I also added a substantial amount of documentation to go with as it is a nontrivial computation.
comment:8 Changed 4 years ago by
 Reviewers set to Travis Scrimshaw
Great, thanks. Three small comments: `N`th root
(although this isn't such a big deal, but I think it looks a little better), typo copmputed
, and put the reference in the master reference file (and a nitpick, missing commas after each math formula). You can set a positive review on my behalf.
comment:9 Changed 4 years ago by
 Reviewers Travis Scrimshaw deleted
err...where is the master reference file?
comment:10 Changed 4 years ago by
 Reviewers set to Travis Scrimshaw
src/doc/en/reference/references/index.rst
comment:12 Changed 4 years ago by
 Commit changed from a282b5e61fb968dd7eaeb20e4bf18a06ecfe69c7 to 6f8c944e2801f37ebee75b32e45438d1fe30f453
Branch pushed to git repo; I updated commit sha1. New commits:
6f8c944  22853: fix documentation, add character to mod p calc

comment:13 Changed 4 years ago by
 Reviewers set to Travis Scrimshaw
You might want to look at it again since I forgot to add the characters for the relative series in the mod p case earlier.
comment:14 Changed 4 years ago by
 Status changed from needs_review to positive_review
comment:15 Changed 4 years ago by
 Branch changed from u/bhutz/molien_series to 6f8c944e2801f37ebee75b32e45438d1fe30f453
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
22783: add character functionality for GAP groups
22783: rename function and add finite check
22783: make error message lowercase
Merge commit '81dcaba8f63d5ba43f52d80f20474d7859806ee2' of git://trac.sagemath.org/sage into molien
22835: molien series for finite matrix groups