Opened 3 years ago
Closed 3 years ago
#28172 closed enhancement (fixed)
Profile of a Finite Permutation Group
Reported by:  falque  Owned by:  

Priority:  minor  Milestone:  sage8.9 
Component:  group theory  Keywords:  permutation groups, profile, fpsac2019 
Cc:  nthiery, nadialafreniere  Merged in:  
Authors:  Justine Falque  Reviewers:  Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  609ea59 (Commits, GitHub, GitLab)  Commit:  609ea592ee2d098ba27f0cd8d4161ff9e074a0db 
Dependencies:  Stopgaps: 
Description (last modified by )
Add methods profile and profile_series (= profile_polynomial) in category finite_permutation_groups, in order to be able to compute the orbital profile, and its generating series/polynomial, of a finite permutation group.
(The profile of G maps any non negative integer n onto the number of Gorbits of nsubsets, for the induced action of G on all subsets of its domain.)
Change History (22)
comment:1 Changed 3 years ago by
 Branch set to u/falque/profile_of_a_finite_permutation_group
comment:2 Changed 3 years ago by
 Commit set to fde25b1affa36b2f1749f23b6ae8754eefc77217
comment:3 followup: ↓ 4 Changed 3 years ago by
if profile_series is just an alias, you can write
profile_series = profile_polynomial
comment:4 in reply to: ↑ 3 Changed 3 years ago by
Thanks Frédéric for the review! I also gave some oral feedback to Justine.
if profile_series is just an alias, you can write
profile_series = profile_polynomial
Yup, with the caveat that if a subclass overrides profile_polynomial, profile_series will still point to the one here.
comment:5 Changed 3 years ago by
 Branch u/falque/profile_of_a_finite_permutation_group deleted
 Commit fde25b1affa36b2f1749f23b6ae8754eefc77217 deleted
 Description modified (diff)
Methods should be added to the category of finite permutation groups rather than the class PermutationGroup_generic
comment:6 Changed 3 years ago by
 Branch set to u/falque/profile_of_a_finite_permutation_group
comment:7 Changed 3 years ago by
 Commit set to 6ede59a1baece3680d4c96be2fd0a07d13c2b7db
Branch pushed to git repo; I updated commit sha1. New commits:
6ede59a  Improvements in the documentation and more flexibility about the nature of variable

comment:8 Changed 3 years ago by
 Keywords fpsac2019 added
comment:9 Changed 3 years ago by
needs review ?
comment:10 Changed 3 years ago by
 Status changed from new to needs_review
comment:11 Changed 3 years ago by
 Cc nadialafreniere added
comment:12 Changed 3 years ago by
 Status changed from needs_review to needs_work
Great job Justine! I'm glad you added it to Sage. I have some comments for you:
 It should be clear in the doc what the profile is. Please, add it to the docstring of
profile_polynomial
.  There could be more examples in
profile_polynomial
. For now, the only example is with the fifth cyclic group, but it would be nice to have it with something else. For example, C6 would be interesting, since it makes it clear that we are looking only at cycles and no other symmetries, like with D6 (it was not exactly clear to me after the computation for C5, since the profile polynomial is the same as for D5). The Symmetric or Dihedral groups could be good examples.  It is not clear to me what the added value of the
profile_series
function is. Doesn't it do the same asprofile_polynomial
? If so, I like the idea of an alias. If you are worried about someone changingprofile_polynomial
, it is still an issue in the way it is written right now.  I don't know how to do it efficiently, but I am wondering if it really is a good idea to compute the whole polynomial just to get one coefficient in
profile
. If there is a faster way to do it, especially when the group is big, please change the code.  There are some trailing spaces at the end of lines, and they should be erased.
Congrats againt for this contribution!
comment:13 Changed 3 years ago by
 Commit changed from 6ede59a1baece3680d4c96be2fd0a07d13c2b7db to 012740bb88cdf49e0b6eac46b9762776e4e6c168
Branch pushed to git repo; I updated commit sha1. New commits:
012740b  Revised docstring, particularly in profile_series, with new examples and a description of what the profile is; profile_polynomial becomes an alias; an optional argument is added in method profile to allow the user to change the computation method.

comment:14 Changed 3 years ago by
 Commit changed from 012740bb88cdf49e0b6eac46b9762776e4e6c168 to fd6fbb40d98e575758a82e9e9e1c2c0c4259ff4c
Branch pushed to git repo; I updated commit sha1. New commits:
fd6fbb4  wrong file, sorry; corrected

comment:15 Changed 3 years ago by
 Description modified (diff)
 Status changed from needs_work to needs_review
 I made the suggested additions to the documentation of profile_series.
 "If you are worried about someone changing
profile_polynomial
, it is still an issue in the way it is written right now." > I was told differently indeed; anyway, I think it is clearer with an alias for now, so I changed it.
 I checked the useless spaces, it should be ok.
 "I don't know how to do it efficiently, but I am wondering if it really
is a good idea to compute the whole polynomial just to get one coefficient
in
profile
. If there is a faster way to do it, especially when the group is big, please change the code." > There is indeed a more direct method, although I would call it the
"naive" method since I am not sure it is really faster than Polyà enumeration, which presents in addition the bonus of computing all values of the profile in a row. I added nevertheless an optional argument to allow the user to use their preferred method. This can also make what the profile is clearer to the user !
 some micro changes here and there
comment:16 Changed 3 years ago by
 Latex syntax is not allowed in the doc, but unicode works fine : instead of
P\'{o}ly\`{a
}, just use Pólya.
 You could add some crossreferences using
.. SEEALSO:: :meth:`profile`
and
.. SEEALSO:: :meth:`profile_series`
in the appropriate place of the docs.
comment:17 Changed 3 years ago by
 Commit changed from fd6fbb40d98e575758a82e9e9e1c2c0c4259ff4c to 798f9c055317a3398622236eb58423ab8a12faf8
Branch pushed to git repo; I updated commit sha1. New commits:
798f9c0  Added references, fixed one or two typos and errors

comment:18 Changed 3 years ago by
Pólya takes just one accent:
comment:19 Changed 3 years ago by
 Commit changed from 798f9c055317a3398622236eb58423ab8a12faf8 to 609ea592ee2d098ba27f0cd8d4161ff9e074a0db
Branch pushed to git repo; I updated commit sha1. New commits:
609ea59  typo

comment:20 followup: ↓ 21 Changed 3 years ago by
 Reviewers set to Frédéric Chapoton
 Status changed from needs_review to positive_review
ok, looks good to me. Merci !
comment:21 in reply to: ↑ 20 Changed 3 years ago by
comment:22 Changed 3 years ago by
 Branch changed from u/falque/profile_of_a_finite_permutation_group to 609ea592ee2d098ba27f0cd8d4161ff9e074a0db
 Resolution set to fixed
 Status changed from positive_review to closed
after
EXAMPLES::
the doctests must be indented by 4 spacesin "profile", add INPUT to explain whar is n
add doctests with another variable name
add doc and doctests for "profile_series"
New commits:
added profile and polynomial_of_profile methods to class PermutationGroup_generic
28172 added methods profile and profile_polynomial to class PermutationGroup_generic