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

MaximalSubgroupClassReps uses AtlasRep if available #5485

Merged
merged 2 commits into from
Aug 25, 2023

Conversation

hulpke
Copy link
Contributor

@hulpke hulpke commented Aug 12, 2023

MaximalSubgroupClassReps method for simple groups that uses AtlasRep, if thee package is available and can handle the group.

@hulpke hulpke added release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes kind: performance improvement labels Aug 12, 2023
@hulpke hulpke changed the title MaximalSubgroupClassReps uses AtlasRep MaximalSubgroupClassReps uses AtlasRep if available Aug 12, 2023
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.

Thank you! Looks good to me but it wouldn't hurt if @ThomasBreuer had another look, perhaps?

Copy link
Contributor

@ThomasBreuer ThomasBreuer left a comment

Choose a reason for hiding this comment

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

The idea is:
If AtlasRep provides information about the group in question then does it have SLPs for computing representatives for all classes of maximal subgroups?
In order to decide this, one can do the following.

info:= AtlasRepInfoRecord( name );
IsBound( info.nrMaxes ) and info.slpMaxes[1] = [ 1 .. info.nrMaxes ] and ForAll( info.slpMaxes[2], l -> 1 in l );

If the result is true then one can proceed as in the proposed code: Compute standard generators and use the SLPs directly if there is a find script, and compute an isomorphisms of groups and use AtlasSubgroup otherwise.

lib/grplatt.gi Outdated Show resolved Hide resolved
@fingolfin
Copy link
Member

@ThomasBreuer does this seem OK to you now?

Copy link
Contributor

@ThomasBreuer ThomasBreuer left a comment

Choose a reason for hiding this comment

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

Thanks.
The code looks correct now, just two lines could be simplified.

lib/grplatt.gi Outdated Show resolved Hide resolved
lib/grplatt.gi Outdated Show resolved Hide resolved
@fingolfin fingolfin enabled auto-merge (squash) August 25, 2023 11:03
@fingolfin
Copy link
Member

Thank you! I applied @ThomasBreuer tweaks (after double checking), this will auto-merge once the test suite passed

@fingolfin fingolfin merged commit 9409d3e into gap-system:master Aug 25, 2023
21 checks passed
@fingolfin fingolfin added the topic: performance bugs or enhancements related to performance (improvements or regressions) label Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: performance bugs or enhancements related to performance (improvements or regressions)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants