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

Status badges

Description (last modified by falque)

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 G-orbits of n-subsets, for the induced action of G on all subsets of its domain.)

Change History (22)

comment:1 Changed 3 years ago by falque

  • Branch set to u/falque/profile_of_a_finite_permutation_group

comment:2 Changed 3 years ago by chapoton

  • Commit set to fde25b1affa36b2f1749f23b6ae8754eefc77217

after EXAMPLES:: the doctests must be indented by 4 spaces

in "profile", add INPUT to explain whar is n

add doctests with another variable name

add doc and doctests for "profile_series"


New commits:

e9b8464added profile and polynomial_of_profile methods to class PermutationGroup_generic
fde25b128172 added methods profile and profile_polynomial to class PermutationGroup_generic

comment:3 follow-up: Changed 3 years ago by chapoton

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 nthiery

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 falque

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

  • Branch set to u/falque/profile_of_a_finite_permutation_group

comment:7 Changed 3 years ago by git

  • Commit set to 6ede59a1baece3680d4c96be2fd0a07d13c2b7db

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

6ede59aImprovements in the documentation and more flexibility about the nature of variable

comment:8 Changed 3 years ago by nadialafreniere

  • Keywords fpsac2019 added

comment:9 Changed 3 years ago by chapoton

needs review ?

comment:10 Changed 3 years ago by falque

  • Status changed from new to needs_review

comment:11 Changed 3 years ago by falque

  • Cc nadialafreniere added

comment:12 Changed 3 years ago by nadialafreniere

  • 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 as profile_polynomial? If so, I like the idea of an alias. If you are worried about someone changing profile_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 git

  • Commit changed from 6ede59a1baece3680d4c96be2fd0a07d13c2b7db to 012740bb88cdf49e0b6eac46b9762776e4e6c168

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

012740bRevised 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 git

  • Commit changed from 012740bb88cdf49e0b6eac46b9762776e4e6c168 to fd6fbb40d98e575758a82e9e9e1c2c0c4259ff4c

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

fd6fbb4wrong file, sorry; corrected

comment:15 Changed 3 years ago by falque

  • 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
Last edited 3 years ago by falque (previous) (diff)

comment:16 Changed 3 years ago by chapoton

  • 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 cross-references using

.. SEEALSO:: :meth:`profile` and .. SEEALSO:: :meth:`profile_series` in the appropriate place of the docs.

comment:17 Changed 3 years ago by git

  • Commit changed from fd6fbb40d98e575758a82e9e9e1c2c0c4259ff4c to 798f9c055317a3398622236eb58423ab8a12faf8

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

798f9c0Added references, fixed one or two typos and errors

comment:18 Changed 3 years ago by chapoton

comment:19 Changed 3 years ago by git

  • Commit changed from 798f9c055317a3398622236eb58423ab8a12faf8 to 609ea592ee2d098ba27f0cd8d4161ff9e074a0db

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

609ea59typo

comment:20 follow-up: Changed 3 years ago by chapoton

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

Replying to chapoton:

ok, looks good to me. Merci !

Thank you !!!

comment:22 Changed 3 years ago by vbraun

  • Branch changed from u/falque/profile_of_a_finite_permutation_group to 609ea592ee2d098ba27f0cd8d4161ff9e074a0db
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.