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: sage-8.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:

Status badges

Description (last modified by bhutz)

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 bhutz

  • Branch set to u/bhutz/molien_series
  • Commit set to ecfa8456ea4c1ae75d7cdf131b94bcf7b0b9142c
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

270b66e22783: add character functionality for GAP groups
2a3053d22783: rename function and add finite check
81dcaba22783: make error message lowercase
14b132aMerge commit '81dcaba8f63d5ba43f52d80f20474d7859806ee2' of git://trac.sagemath.org/sage into molien
ecfa84522835: molien series for finite matrix groups

comment:2 Changed 4 years ago by bhutz

  • Dependencies set to #22783

This requires #22783, which is already closed.

comment:3 Changed 4 years ago by tscrim

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 bhutz

  • 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 non-divisibility, I think it is somewhat more complicated that than. Dekker-deJong 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 tscrim

Ah, okay. Don't worry too much if positive characteristic seems like a lot of work.

comment:6 Changed 4 years ago by git

  • Commit changed from ecfa8456ea4c1ae75d7cdf131b94bcf7b0b9142c to a282b5e61fb968dd7eaeb20e4bf18a06ecfe69c7

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

a282b5e22853: add support for char p>0

comment:7 Changed 4 years ago by bhutz

  • 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 non-trivial computation.

comment:8 Changed 4 years ago by tscrim

  • 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 bhutz

  • Reviewers Travis Scrimshaw deleted

err...where is the master reference file?

comment:10 Changed 4 years ago by tscrim

  • Reviewers set to Travis Scrimshaw

src/doc/en/reference/references/index.rst

comment:11 Changed 4 years ago by bhutz

  • Reviewers Travis Scrimshaw deleted

ok. will do.

comment:12 Changed 4 years ago by git

  • Commit changed from a282b5e61fb968dd7eaeb20e4bf18a06ecfe69c7 to 6f8c944e2801f37ebee75b32e45438d1fe30f453

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

6f8c94422853: fix documentation, add character to mod p calc

comment:13 Changed 4 years ago by bhutz

  • 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 tscrim

  • Status changed from needs_review to positive_review

comment:15 Changed 4 years ago by vbraun

  • Branch changed from u/bhutz/molien_series to 6f8c944e2801f37ebee75b32e45438d1fe30f453
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.