Skip to content

Fix FactorCosetAction(G, G) for perm. groups#5903

Merged
fingolfin merged 1 commit intogap-system:masterfrom
ThomasBreuer:TB_FactorCosetAction
Jan 11, 2025
Merged

Fix FactorCosetAction(G, G) for perm. groups#5903
fingolfin merged 1 commit intogap-system:masterfrom
ThomasBreuer:TB_FactorCosetAction

Conversation

@ThomasBreuer
Copy link
Contributor

Catch the case of an action with trivial image in DoFactorCosetActionPerm, and delegate to DoFactorCosetAction in this case.

resolves #5902

(Just for curiosity:
The code for DoFactorCosetAction and DoFactorCosetActionPerm deals with the case that the second argument is a list (not a group), but only in order to call Error if this list is empty. The functions called later, such as Core and ActionRefinedSeries, do not accept a list as the value of this argument.
Is this list situation meaningful at all?)

Catch the case of an action with trivial image in
`DoFactorCosetActionPerm`,
and delegate to `DoFactorCosetAction` in this case.
@ThomasBreuer ThomasBreuer added kind: bug Issues describing general bugs, and PRs fixing them kind: bug: unexpected error Issues describing bugs in which computation unexpectedly encounters an error, and PRs fixing them topic: library release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes backport-to-4.14 labels Jan 10, 2025
@ThomasBreuer ThomasBreuer requested a review from hulpke January 10, 2025 12:47
@ThomasBreuer
Copy link
Contributor Author

I do not understand the remark that the added lines are not covered by tests.
The added test in teststandard should execute these lines of code, I have checked this locally.
What am I missing?

@lgoettgens
Copy link
Member

I do not understand the remark that the added lines are not covered by tests. The added test in teststandard should execute these lines of code, I have checked this locally. What am I missing?

If we have a look at the codecov webpage (https://app.codecov.io/gh/gap-system/gap/pull/5903/blob/lib/csetperm.gi#L886), it there states that your added lines are indeed covered (aka are green), while the error case two lines above is uncovered. My assumption is that there is a small offset somewhere in the codecov-to-github interface that mistakenly reports this

@fingolfin fingolfin merged commit 90743e6 into gap-system:master Jan 11, 2025
fingolfin pushed a commit that referenced this pull request Jan 11, 2025
Catch the case of an action with trivial image in
`DoFactorCosetActionPerm`,
and delegate to `DoFactorCosetAction` in this case.
@fingolfin fingolfin added backport-to-4.14-DONE-unused Renamed so release scripts ignore them; we skipped 4.14.1 and did not use backports and removed backport-to-4.14 labels Jan 11, 2025
@fingolfin
Copy link
Member

Backported to stable-4.14 in fb105c9

@ThomasBreuer ThomasBreuer deleted the TB_FactorCosetAction branch January 12, 2025 20:44
@mantepse
Copy link

Please excuse me for commenting under a closed issue - I'm not sure whether I should open a new one or ask on the mailing list first, please let me know.

I think I found an example where the issue still occurfor the following question:

(Just for curiosity: The code for DoFactorCosetAction and DoFactorCosetActionPerm deals with the case that the second argument is a list (not a group), but only in order to call Error if this list is empty. The functions called later, such as Core and ActionRefinedSeries, do not accept a list as the value of this argument. Is this list situation meaningful at all?)

I have code (in sagemath, sagemath/sage#40186) that produces such a situation - although I am not sure whether or not I might be abusing FactorCosetAction. More precisely, I have a list of stabilizer subgroups of a symmetric group, and I want to create the corresponding action. In the trivial case, this fails:

gap> S := SymmetricGroup(1);
Group(())

gap> FactorCosetAction(S, S);
<action epimorphism>

gap> FactorCosetAction(S, [S]);
Error, usage: Group(<gen>,...), Group(<gens>), Group(<gens>,<id>) at /home/martin/sage/local/share/gap/lib/grp.gi:4989 called from
Group( d ) at /home/martin/sage/local/share/gap/lib/factgrp.gi:555 called from
<function "FactorCosetAction On cosets of list of groups">( <arguments> )
 called from read-eval loop at *stdin*:15
you can 'quit;' to quit to outer loop, or
you can 'return;' to continue
brk> quit;

gap> FactorCosetAction(S, [S, S]);
Error, usage: Group(<gen>,...), Group(<gens>), Group(<gens>,<id>) at /home/martin/sage/local/share/gap/lib/grp.gi:4989 called from
Group( d ) at /home/martin/sage/local/share/gap/lib/factgrp.gi:555 called from
<function "FactorCosetAction On cosets of list of groups">( <arguments> )
 called from read-eval loop at *stdin*:15
you can 'quit;' to quit to outer loop, or
you can 'return;' to continue

Apparantly, this only happens for the trivial group:

gap> S := SymmetricGroup(2);
Sym( [ 1 .. 2 ] )

gap> FactorCosetAction(S, S);
<action epimorphism>

gap> FactorCosetAction(S, [S]);
[ (1,2) ] -> [ () ]

gap> FactorCosetAction(S, [S, S]);
[ (1,2) ] -> [ () ]

@hulpke
Copy link
Contributor

hulpke commented Feb 16, 2026

@mantepse Yes, if there is a chance that this operation gets called for the trivial group, this case (which will always just map to the trivial permutation group on 0 points), one needs to catch it at the start. (One of the tricksy bits is that the group on 1 point acts actually on 0.)

The list functionality is definitive needed -- it is used to construct (subdirectories product) permutation actions on the cosets of multiple subgroups.

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

Labels

backport-to-4.14-DONE-unused Renamed so release scripts ignore them; we skipped 4.14.1 and did not use backports kind: bug: unexpected error Issues describing bugs in which computation unexpectedly encounters an error, 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 topic: library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FactorCosetAction fails for non-proper subgroups

5 participants