Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added the missing perfect groups of order up to a million #3925

Merged
merged 4 commits into from
Oct 1, 2020

Conversation

hulpke
Copy link
Contributor

@hulpke hulpke commented Mar 25, 2020

Added perfect groups of orders that had been missing in the Holt/Plesken book (newly computed). This increases the number of groups in the perfect groups library by a factor close to 6.

Also added 5 groups that were found missing in the existing lists.

@hulpke hulpke added the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Mar 25, 2020
@fingolfin
Copy link
Member

This needs to be rebased on latest master: package bootstrap was temporarily broken because we moved file downloads to its own subdomain; now, of course we also set up a redirect, but it turns out we didn't configure curl to follow redirects... This was quickly fixed on master, but now this PR needs to be rebased to get the fix.

Sorry for the inconvenience. Also, 10% or so of Travis builds currently randomly fail because the downloads from gap-system.org alternate between blazing fast (5 seconds to download package tarball) to incredible slow (> 40 minutes do download). We are working during the ongoing virtual reality GAP Days on addressing this in some way.

grp/perf.gd Show resolved Hide resolved
@hulpke
Copy link
Contributor Author

hulpke commented Mar 26, 2020

Sorry for the inconvenience. Also, 10% or so of Travis builds currently randomly fail because the downloads from gap-system.org alternate between blazing fast (5 seconds to download package tarball) to incredible slow (> 40 minutes do download). We are working during the ongoing virtual reality GAP Days on addressing this in some way.

Thank you for the explanation. I observed this weird behavior and was wondering whether anyone was aware of it. I suspect you have good ideas what to do about it and won't need me to second-guess possible workarounds.

@fingolfin
Copy link
Member

At the core, it seems that the VM the University of St Andrews gives us to host the GAP web server is flaky (for many months now, it has been offline for a few seconds or minutes, multiple times per day); and recently, the bandwidth breaks down frequently, too. I have no idea what exactly is the cause, but in the end, it doesn't matter.

Right now, we are considering to just move the "default" downloads to the GitHub release system, which has been very reliable for us so far, and use the St Andrews only as backup. In addition, I may be able to offer hosting in Kaiserslautern, but maybe only after the Corona pandemic, as right now it is difficult to coordinate this with the computer admins.

@coveralls
Copy link

coveralls commented Mar 27, 2020

Coverage Status

Coverage increased (+8.0%) to 93.729% when pulling 1b19738 on hulpke:fixes into dc28742 on gap-system:master.

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

A typos and a remark on the docs, but overall this is fine and of course a great addition to GAP!

doc/ref/grplib.xml Outdated Show resolved Hide resolved
doc/ref/grplib.xml Outdated Show resolved Hide resolved
grp/perf.gd Outdated Show resolved Hide resolved
@fingolfin fingolfin added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes topic: library labels Apr 3, 2020
@fingolfin
Copy link
Member

@hulpke this still has the "do not merge" label; on purpose?

@hulpke
Copy link
Contributor Author

hulpke commented Apr 6, 2020

Yes, the "Do not merge" is since I still want to run correctness tests.

Aded, Oct 1: Tests have run. I found a bug and this is fixed and the results corrected/rechecked.

doc/ref/grplib.xml Outdated Show resolved Hide resolved
@fingolfin
Copy link
Member

These seem to be the relevant commits:

commit db7a8df486e0ca4a1f59b351f7340b4927be1008
Author: Jack Schmidt <[email protected]>
Date:   2005-11-24 18:51:28 +0000

    Added missing perfect group of order 450000, A6 2^1 # 5^4. Restricts nicely to A5 2^1 # 5^2, which is its minimal index corefree subgroup. JS

and

commit 2d5f94f342bd412cff4b95fb33fe3fd599a73e67
Author: Jack Schmidt <[email protected]>
Date:   2005-09-10 17:53:48 +0000

    First draft of adding in the perfect group of order 962280. JS

@hulpke
Copy link
Contributor Author

hulpke commented May 30, 2020

@fingolfin Perfect (pun intened). Thank you! I'll add a remark.
[So it was 2005, which actually predates my current email archive...]

@ThomasBreuer
Copy link
Contributor

@hulpke One more minor remark:
The changed behaviour of NumberPerfectLibraryGroups may break existing code, in the sense that the old variant allows one to run over the available perfect groups of order n via

List( [ 1 .. NumberPerfectLibraryGroups( n ) ], i -> PerfectGroup( IsPermGroup, i );

whereas the new variant forces one to check for fail before forming the range;
try for example

[ 1 .. NumberPerfectLibraryGroups( 1451520 ) ];

@hulpke
Copy link
Contributor Author

hulpke commented Jun 5, 2020

@ThomasBreuer
I look at this as more of a hypothetical issue, but would be happy to reconsider if someone can point to concrete code out there that does this.

The issue of "available perfect groups" was much more a concern with the holes in the library. OTOH, the old convention of NumberPerfectLibraryGroups has the risk of a user misinterpreting it as NumberPerfectGroups and making wrong conclusions about existence of such groups.

The new behavior also is more consistent with the other group libraries, none of which distinguishes "Groups" from "LibraryGroups".

What probably however should be done with this change is to move NumberPerfectLibraryGroups on the way to obsolescence, e.g. have it issue a warning that points to NrPerfectGroups.

@ThomasBreuer
Copy link
Contributor

@hulpke Well, this is how I found the hypothetical problem with the changed behaviour of NumberPerfectLibraryGroups:
I wanted to include the IDs of the relevant new groups from the perfect groups library in the character table library, and ran into an error (see ctbllib/gap4/ctdbattr.g, attribute "perf", function create).

@hulpke hulpke removed the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Sep 30, 2020
Remove the `notAvailable` component of `PerfectGroup`
Basic test of creating new groups (reading files in)

Removed constructing all perfect groups, as this will be longer now

Remove now pointless tests that checked for the discrepancy between claimed
list (up to 10^6) and existing data.
Two perfect groups of orders 243000, and one each of order 729000, 871200 and 878460.
They are clearly new by number of conjugacy classes and minimal failthful permdegrees.
@hulpke hulpke merged commit 4be5d2f into gap-system:master Oct 1, 2020
@fingolfin
Copy link
Member

Great job! That's definitely going to be a top tier in the release notes for 4.12 ;-)

@danielrademacher danielrademacher self-assigned this Feb 16, 2021
@danielrademacher danielrademacher changed the title Add the missing perfect groups of order up to a million Added the missing perfect groups of order up to a million Feb 16, 2021
@danielrademacher danielrademacher added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Feb 16, 2021
@danielrademacher danielrademacher removed their assignment Feb 16, 2021
@danielrademacher danielrademacher added the release notes: highlight PRs introducing changes that should be highlighted at the top of the release notes label Feb 19, 2021
ThomasBreuer added a commit to ThomasBreuer/gap that referenced this pull request Feb 24, 2021
- gap-system#3925 belongs to 4.12.0 not 4.11.1,
- gap-system#4053 now belongs to `topic: julia`, not to `kind: bug: crash`
- removed a superfluous "in"
ThomasBreuer added a commit to ThomasBreuer/gap that referenced this pull request Feb 25, 2021
- gap-system#3925 belongs to 4.12.0 not 4.11.1,
- gap-system#4053 now belongs to `topic: julia`, not to `kind: bug: crash`
- removed a superfluous "in"
fingolfin pushed a commit that referenced this pull request Feb 28, 2021
- #3925 belongs to 4.12.0 not 4.11.1,
- #4053 now belongs to `topic: julia`, not to `kind: bug: crash`
- removed a superfluous "in"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: added PRs introducing changes that have since been mentioned in the release notes release notes: highlight PRs introducing changes that should be highlighted at the top of the release notes topic: library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants