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

Fix stabilizer computation in alternating groups which could sometimes return a wrong result #5811

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

fingolfin
Copy link
Member

Fixes #5808

@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them kind: bug: wrong result Issues describing bugs that result in mathematically or otherwise wrong results, and PRs fixing them labels Oct 12, 2024
@fingolfin fingolfin requested a review from hulpke October 12, 2024 12:24
@fingolfin fingolfin added this to the GAP 4.14.0 milestone Oct 13, 2024
@fingolfin
Copy link
Member Author

@stertooy can you confirm this fixes the issue for you?

Copy link
Contributor

@hulpke hulpke 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 -- indeed this has been an awkward oversight.

Copy link

@stertooy stertooy left a comment

Choose a reason for hiding this comment

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

The fix works perfectly, many thanks!

I do have one small suggestion for the test.

Comment on lines +5 to +9
gap> stab := Stabilizer( G, One(G), tc );;
gap> Size(stab);
5
Copy link

@stertooy stertooy Oct 14, 2024

Choose a reason for hiding this comment

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

Perhaps something along the lines of

gap> stab := Stabilizer( G, One(G), tc );;
gap> Set( stab ) = Set( Filtered( G, g -> tc( One( G ), g ) = One( G ) ) );
true

would be safer?

This would ensure the following case (which I only discovered just now, and which is fixed by this PR) is also caught:

gap> G := AlternatingGroup( 6 );;
gap> hom1 := GroupHomomorphismByImages( G, G, [ (1,2,3,4,5), (4,5,6) ], [ (1,2,6,3,5), (1,4,5) ] );;
gap> hom2 := IdentityMapping( G );;
gap> tc := function ( g, h ) return (h^hom2)^-1 * g * h^hom1; end;;
gap> stab := Stabilizer( G, One(G), tc );;
gap> Size(stab);
5
gap> Set( stab ) = Set( Filtered( G, g -> tc( One(G), g ) = One(G) ) );
false

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added your second test case, and made the checks more robust

@fingolfin fingolfin added the release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes label Oct 15, 2024
@fingolfin fingolfin enabled auto-merge (squash) October 15, 2024 15:46
@fingolfin fingolfin merged commit 2bb3b57 into gap-system:master Oct 15, 2024
33 checks passed
@fingolfin fingolfin deleted the mh/fix-stab-alt branch October 15, 2024 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug: wrong result Issues describing bugs that result in mathematically or otherwise wrong results, and PRs fixing them kind: bug Issues describing general bugs, and PRs fixing them release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stabiliser for AlternatingGroup produces incorrect result
3 participants