Improve conjugation test for subgroups with different orders

[limakzi]

Conjugate subgroups have the same order.
When the sizes of both subgroups are already known (via HasSize), IsConjugate and RepresentativeActionOp now return immediately if the sizes differ, avoiding expensive conjugacy computations.

Fixes #6266

[Copilot]

Pull request overview

Adds early-exit checks in IsConjugate and RepresentativeActionOp to immediately return when two subgroups have different known orders, avoiding expensive conjugacy computations.

Changes:

• Add size-comparison short-circuit in IsConjugate for groups (lib/grp.gi)
• Add size-comparison short-circuit in RepresentativeActionOp for permutation groups (lib/oprtperm.gi)
• Add a test case for the new behavior (tst/testinstall/ConjNatSym.tst)

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
lib/grp.gi	Early return false when subgroup sizes are known and differ
lib/oprtperm.gi	Early return fail when subgroup sizes are known and differ
tst/testinstall/ConjNatSym.tst	Test case for non-conjugate subgroups with different orders

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[fingolfin]

I wonder if there are other properties and attributes (isomorphism invariants) that would be worthwhile to check? Like IsAbelian, IsSolvableGroup, IsPerfectGroup, NrConjugacyClasses and a gazillion more. With help of Tester and Getter one could easily handle them uniformly with a loop. But is it worth it? Hm.

I do think Size is worth it for sure

[limakzi]

I'm not sure. As you noted there are gazillion isomorphism invariants.
Some are difficult to compute and the difficulty differs between groups.
Proposal:

InstallGlobalFunction( KnownIsomorphismInvariantsMatch,
function( G, H )
    local inv;
    for inv in [ Size, AbelianInvariants, DerivedLength,
                 NilpotencyClassOfGroup, Exponent, HirschLength,
                 IsAbelian, IsNilpotentGroup, IsSolvableGroup,
                 IsPerfectGroup, IsSimpleGroup, IsSporadicSimpleGroup ] do
        if Tester(inv)(G) and Tester(inv)(H) and inv(G) <> inv(H) then
            return false;
        fi;
    od;
    return true;
end );

Then

if not KnownIsomorphismInvariantsMatch(d, e) then
  return fail;
fi;

[ThomasBreuer]

This IsConjugate method calls RepresentativeAction if the shortcut for normal subgroups does not strike. Thus I think it is sufficient to add new tests only in RepresentativeActionOp methods in question.

[limakzi]

Good! The only thing we miss is catalog of checks.
How do you like the proposal I put above?

[fingolfin]

I discussed this with @ThomasBreuer yesterday, and I think our rough agreement was that we should merge this here as-is, and only implement further invariants if either someone has a concrete need (= has use cases were they matter), or someone spends time to carefully test the code with and without the invariants, in various situations -- because experience tells that "let's just check this catalogue of invariants, they are all cheap so this is fine" is a dangerous trap, because testing a dozen "cheap" invariants with non-trivial orchestration code (= more overhead) quickly turns into "not so cheap after all".

@ThomasBreuer if you agree, please merge; otherwise, add another comment