Skip to content

Move the library of perfect groups into the PerfGrp package - #6488

Open
fingolfin wants to merge 3 commits into
masterfrom
claude/perfgrp-package-extraction-002a39
Open

Move the library of perfect groups into the PerfGrp package#6488
fingolfin wants to merge 3 commits into
masterfrom
claude/perfgrp-package-extraction-002a39

Conversation

@fingolfin

@fingolfin fingolfin commented Aug 5, 2026

Copy link
Copy Markdown
Member

Summary

This moves the Holt/Plesken/Hulpke library of finite perfect groups out of the GAP core system and into a new package PerfGrp, in the same way that the small groups, transitive groups and primitive groups libraries were moved into SmallGrp, TransGrp and PrimGrp some years ago.

The library keeps working exactly as before for anyone running a normal GAP: PerfGrp is added to GAP's list of needed packages, so it is loaded at startup just like the other three.

Why now

Thomas Breuer and I have talked about this on and off over the years and agreed it was probably a good idea, but neither of us ever got as far as filing an issue about it. I was about to finally do that, and instead decided to find out whether Claude Opus 5 could carry out the split. It could, so here is the pull request rather than the issue.

What moves

  • grp/perf.gd and grp/perf.grp, i.e. the declarations and the access functions for the library.
  • The 33 data files grp/perf0.grpgrp/perf34.grp, about 28 MB.
  • RepsPerfSimpSub from lib/grplatt.gi, together with the methods it backs for RepresentativesPerfectSubgroups and RepresentativesSimpleSubgroups.
  • The section "Finite Perfect Groups" of doc/ref/grplib.xml, which becomes a chapter of the package manual.
  • The corresponding tests.

In total this removes 35 files and about 530 000 lines from the repository, and shrinks grp/ from roughly 35 MB to 6.8 MB. Outside the data files the diff is 24 files, +66/−948 lines.

What stays

  • IsPerfectGroup and PerfectResiduum, which have nothing to do with the data library.
  • The declarations of RepresentativesPerfectSubgroups, RepresentativesSimpleSubgroups and ConjugacyClassesPerfectSubgroups in lib/grp.gd, along with their documentation in the reference manual. These are part of GAP's group theory interface; only the implementation that needs the data library moves out.
  • The generic ConjugacyClassesPerfectSubgroups method and the RepresentativesPerfectSubgroups method for solvable groups.

Behaviour without the package

Since PerfGrp is a needed package, this only matters for gap --bare and similar developer scenarios. There:

  • RepresentativesPerfectSubgroups and RepresentativesSimpleSubgroups fall back to methods that answer the solvable case directly and otherwise raise an error naming the package. PerfGrp installs the real methods at a higher rank.
  • lib/init.g binds PerfectGroup, PerfectIdentification, PerfGrpLoad, NumberPerfectGroups/NrPerfectGroups and PERFRec to dummy values, in the same style as the existing stubs for IdGroup, SmallGroup and PrimitiveGroup. Without these, library references to those names produce "unbound global variable" syntax warnings at every startup.
  • Two places used the library purely as an optimisation and now check whether the package is loaded before doing so: IsomorphismFpGroup for nonabelian simple permutation groups (lib/gpfpiso.gi) and StructureDescription (lib/grpnames.gi). Both keep working without PerfGrp, just without the shortcut.
  • Two test files that reach the perfect subgroup search indirectly are guarded with #@if IsPackageMarkedForLoading( "perfgrp", "" ), as is already done for smallgrp and primgrp elsewhere in tst/.

The package

The package is prepared at gap-packages/perfgrp, next to SmallGrp and PrimGrp and set up the same way: the shared workflow files, the same test layout, an AutoDoc manual, TestFile in PackageInfo.g. Releases are made with the usual gap-actions/release-pkg automation, which also compresses the data files and updates the package website.

Alexander Hulpke is listed as author and maintainer, and Holt, Plesken and Felsch are credited as authors for the original library and its conversion to GAP format. "The GAP Team" is listed as a further maintainer, as with the other group libraries distributed with GAP, so that routine upkeep — CI, release mechanics, keeping the package working with new GAP versions — does not have to wait on any one person.

The manual now also cites The perfect groups of order up to two million where it describes the classification of the orders not covered by Holt and Plesken, which the reference manual never did.

The package can be loaded into GAP 4.16 and earlier, where it is a no-op because GAP still provides the library itself. This is meant to make the transition easier: the package can be released and enter the distribution before this pull request is merged.

It has not been released yet, so there is still room to adjust details: how the repository is laid out, how the manual is organised, the exact author and credit lines. Suggestions welcome.

Sequencing

This cannot be merged before PerfGrp has been released and added to the package distribution, since GAP will refuse to start without it. CI here will fail until then.

Testing

  • testinstall passes, both normally and with --bare.
  • testbugfix passes (517 files).
  • The teststandard files that exercise the perfect groups library pass.
  • The reference manual builds, with the cross references into the new package manual resolving, and the extracted manual examples for the affected chapters pass.
  • The package's own test suite passes against this branch, and against GAP 4.15.0, 4.16.0 and a stable-4.16 build, where it loads as a no-op.

AI disclosure

The split was carried out by Claude Opus 5 running in Claude Code: locating everything that had to move, moving it, setting up the package, adjusting the documentation and tests, and running the test suites. I directed the work and reviewed the result. The commits are co-authored accordingly.

🤖 Generated with Claude Code

The Holt/Plesken/Hulpke library of finite perfect groups, its access
functions, and the methods for computing perfect and simple subgroups of a
finite group now live in the new PerfGrp package, which is added to the list
of packages needed by GAP. This removes about 29 MB of data from the core
system and allows the library to be updated independently of GAP releases.

`IsPerfectGroup` and `PerfectResiduum` stay in the library, as do the
declarations of `RepresentativesPerfectSubgroups`,
`RepresentativesSimpleSubgroups` and `ConjugacyClassesPerfectSubgroups`.
The latter two attributes get generic fallback methods that answer the
solvable case and otherwise report that PerfGrp is required, so that GAP
started via `gap --bare` still behaves sensibly.

Prepared with the help of Claude Code (Claude Opus 5), which performed the
extraction, wired up the package, and adjusted documentation and tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@fingolfin fingolfin changed the title Move the library of perfect groups into the PerfGrp package Move the library of perfect groups into the PerfGrp package Aug 5, 2026
@fingolfin fingolfin added topic: packages issues or PRs related to package handling, or specific to a package (for packages w/o issue tracker) topic: library release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes labels Aug 5, 2026
@stertooy

stertooy commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Perhaps a good idea to check in advance which packages should have PerfGrp as a needed, suggested or test-package? If it's possible to run PackageDistro's "test all" on this PR, that would probably help detect the most important cases.

@limakzi limakzi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not reviewed the code; I like the change to move perfect-groups to package.

@fingolfin

Copy link
Copy Markdown
Member Author

@gap-package-distribution-bot test

@gap-package-distribution-bot

gap-package-distribution-bot Bot commented Aug 5, 2026

Copy link
Copy Markdown

PackageDistro test for #6488

Full report

Package Evaluation Report for GAP gap-system/gap#6488 @ d4473c37 (base master)

Job Properties

Testing: pr-gap-system-gap-6488-d4473c37/2026/08/05_19-10-29 vs master/2026/08/05_09-42-07

Distribution revision: 8393e591

Generated by Workflow: https://github.com/gap-system/PackageDistro/actions/runs/31037865274

In total, 172 packages were tested, out of which 0 succeeded, 0 failed and 172 were skipped.

✖️ ✖️ Packages that now skipped

169 package(s) skipped tests only on the current version.

Click to show packages!

➖ Packages that were skipped

3 package(s) skipped tests also on the previous version.

Click to show packages!

@fingolfin

Copy link
Copy Markdown
Member Author

Actually, no, wait, that can't work now, without the PerfGrp package in the distro. My bad. I'll cancel that CI run. We first need to generally agree on this.

In particular, that definitely needs @hulpke to weigh in, who after all did most of the work on the perfect groups library in GAP; and in general I'd like to let this sit a while to give more people a chance to ponder this and to comment. No rush on this anyway.

@hulpke hulpke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no problem with moving this into a package if someone considers it as important.

Just the caveat that this data is absolutely necessary for computing subgroups -- there is no fallback possibility in the subgroup lattice algorithm, so this package cannot become optional.

@fingolfin

Copy link
Copy Markdown
Member Author

@hulpke glad to hear! BTW I've sent you an invite for https://github.com/gap-packages/perfgrp so you have full access to that. If you have some time, please have a look at it, even a brief one, at least at README at PackageInfo.g -- just to see if everything is OK for you, if credits are given correctly etc. Of course feel free to have a deeper look, too, but besides some plumbing it should just be identical to the code removed here.

And yeah with this PR, perfgrp has the same status as smallgrp, primgrp, transgrp. It is supposed to be possible to run GAP without it, with the --bare option, but indeed computations that need it will run into an error (which hopefully will be clear enough to indicate what's going on). But the mode is not something recommended for casual users, it's only for experts who know what they are doing (or at least think so ;-) ).

Right now the testinstall test with --bare still fail but that'll be fixed in the next update to this PR.

fingolfin and others added 2 commits August 5, 2026 23:38
Two of the five classes of maximal subgroups of A_6 are perfect, and perfect
subgroups cannot be reached by cyclic extension, so the lattice fallback used
by `MaximalSubgroupClassReps` cannot find them once the perfect groups library
lives in a package that is not loaded. Guard the test accordingly.

Also use `ErrorNoReturn` rather than `Error` in the fallback methods for
`RepresentativesPerfectSubgroups` and `RepresentativesSimpleSubgroups`, since
there is nothing sensible to return from them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@fingolfin

Copy link
Copy Markdown
Member Author

Lost test coverage for the MaximalSubgroupClassReps recursion fix

Merging with master brought in the regression test added by #6482, and I had to
guard it with #@if IsPackageMarkedForLoading( "perfgrp", "" ). That is
unavoidable, but it costs us the coverage that test was written to provide, so
I would rather flag it than let it pass silently.

Why the test cannot run without the package. Two of the five classes of
maximal subgroups of A_6 are perfect (the A_5s of index 6), and perfect
subgroups cannot be reached by cyclic extension — seeding them from the library
is exactly what RepresentativesPerfectSubgroups is for. So once the perfect
groups library is no longer part of GAP, the lattice fallback genuinely cannot
answer this, and there is no library-free route to the result.

Why that leaves a gap. The three relevant configurations are:

PrimGrp PerfGrp what happens
yes yes MaximalSubgroupsSymmAlt succeeds, the recursion path is never entered
no no path entered, but cannot complete — only a clean error is observable
no yes path entered and completes — this is the real test

testinstall covers the first row and testinstall-bare the second, so after
this PR the fix from #6482 is exercised by no CI configuration. Note also that
the third row cannot be reached by a normal GAP start, because PrimGrp is
itself a needed package; it takes gap --bare plus an explicit
LoadPackage("perfgrp"). I checked that it does work:

$ gap -q --bare -c 'LoadPackage("perfgrp");; SetInfoLevel(InfoPerformance,0); \
    G:=AlternatingGroup(6);; m:=MaximalSubgroupClassReps(G);; \
    Print(SortedList(List(m,H->Index(G,H))),"\n"); QUIT;'
[ 6, 6, 10, 15, 15 ]

Options, roughly in increasing order of effort:

  1. Accept the gap. The fix in lib/maxsub.gi is small and unlikely to regress
    unnoticed, and an infinite recursion would show up as a CI timeout anyway.
  2. Assert only that it fails cleanly in the bare case, which at least rules out
    the recursion. I tried this and backed out: #@ commands may not appear
    inside an expected-output block, so an #@else branch matching the error
    text does not parse without contortions, and matching the message text is
    brittle.
  3. Add a small test that runs in the third configuration above, i.e. --bare
    plus LoadPackage("perfgrp"). This is the only option that restores the
    original assertion. It does not fit tst/testinstall/, so it would need
    either a tst/testspecial/-style script or an extra CI step.

I have no strong preference and have gone with (1) for now, since it keeps this
PR focused. Happy to implement (3) here or in a follow-up if you would rather
not lose the coverage.

Written with Claude Opus 5 in Claude Code, which also carried out the
investigation and the test runs quoted above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes topic: library topic: packages issues or PRs related to package handling, or specific to a package (for packages w/o issue tracker)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants