docs: document the integer encoding used by CodePcGroup and PcGroupCode#6441
Conversation
|
I suggest to add a citation of the abovementioned paper. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6441 +/- ##
==========================================
- Coverage 78.89% 78.88% -0.01%
==========================================
Files 685 685
Lines 293520 293520
Branches 8667 8667
==========================================
- Hits 231558 231553 -5
+ Misses 60162 60157 -5
- Partials 1800 1810 +10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Added a citation to Besche-Eick, "Construction of finite groups" ( |
ThomasBreuer
left a comment
ThomasBreuer
left a comment
There was a problem hiding this comment.
I think the fact that the relative orders must be prime integers should be mentioned, see the comment.
| ## <P/> | ||
| ## More precisely, let | ||
| ## <M>g_1,\ldots,g_l</M> be the entries of <A>pcgs</A>, and let | ||
| ## <M>r_1,\ldots,r_l</M> be their relative orders. If <M>l=0</M>, |
There was a problem hiding this comment.
In [BescheEick98], it is assumed that the relative orders of the pcgs are prime integers.
Not every pcgs in GAP has this property, there is a GAP function IsPrimeOrdersPcgs for checking it.
If one admits non-prime relative orders then one cannot read off the relative orders from the order of a given p-group. Thus this assumption should be mentioned in the documentation. Perhaps CodePcgs should check the relative orders, and signal an error if not all of them are prime integers.
There was a problem hiding this comment.
Adding that check would certainly be a good idea (but of course this goes beyond the scope of this PR, i.e., is not a blocker).
There was a problem hiding this comment.
Such a check has now been added by Thomas in PR #6451
| ## More precisely, let | ||
| ## <M>g_1,\ldots,g_l</M> be the entries of <A>pcgs</A>, and let | ||
| ## <M>r_1,\ldots,r_l</M> be their relative orders. If <M>l=0</M>, | ||
| ## the code is <M>0</M>. Otherwise put |
There was a problem hiding this comment.
In fact, the code is 0 for any elementary abelian p-group.
There was a problem hiding this comment.
@ThomasBreuer By the way, is there a reason why the group order is not encoded in this code as well?
There was a problem hiding this comment.
I do not know why the code was defined like this.
One reason to leave out the group order could be that whenever one stores many groups of the same order (which is not unlikely for p-groups), including the group order in each code would be redundant.
There was a problem hiding this comment.
Ah, that seems like a good reason.
fingolfin
added
topic: documentation
ThomasBreuer
left a comment
ThomasBreuer
left a comment
There was a problem hiding this comment.
This pull request provides the documentation asked for in #6419.
The two suggested improvements concerning IsPrimeOrdersPcgs and the formulation of the (lexicographic) order of group elements should be added, and then the pull request can be merged.
Thanks for this work.
Co-authored-by: Max Horn <max@quendi.de> Co-authored-by: Thomas Breuer <sam@math.rwth-aachen.de>
4 participants
The reference manual now specifies the integer encoding behind
CodePcgs/CodePcGroup/PcGroupCode, which #6419 (from @fingolfin) asked for: the best existing reference is half a page in Besche-Eick "Construction of finite groups", which leaves key details underspecified, including the canonically ordered element list used for tail encoding. The expanded GAPDoc blocks inlib/randiso.gdcover how relative orders are packed, how the right-hand sides of power and commutator relations are enumerated against the canonically ordered element list, and how the pieces combine into one integer, with a worked round-trip example and an interoperability note naming Magma'sSmallGroupEncoding/SmallGroupDecodingand the OSCAR counterparts.Fixes #6419
Text for release notes
The integer encoding used by
CodePcgs,CodePcGroupandPcGroupCodeis now fully documented in the reference manual, including a round-trip example and pointers to the Magma and OSCAR counterparts.Further details
The specification prose was derived from the implementation in
lib/randiso.gi, not from the paper. The new<Example>blocks follow the manual conventions so the example extraction test verifies them against actual GAP output.