Skip to content

Set InfoText when computing Irr#6275

Merged
fingolfin merged 3 commits intogap-system:masterfrom
ThomasBreuer:TB_InfoText
Mar 23, 2026
Merged

Set InfoText when computing Irr#6275
fingolfin merged 3 commits intogap-system:masterfrom
ThomasBreuer:TB_InfoText

Conversation

@ThomasBreuer
Copy link
Copy Markdown
Contributor

Whenever the Irr value gets computed for a group or a character table, store in InfoText of the table which method was used.

And add an Irr method for the case that IrrDixonSchneider is known. (We have at least one package with a method of higher rank than the IrrDixonSchneider based method, so this makes sense.)

Whenever the `Irr` value gets computed for a group or a character table,
store in `InfoText` of the table which method was used.

And add an `Irr` method for the case that `IrrDixonSchneider` is known.
(We have at least one package with a method of higher rank than
the `IrrDixonSchneider` based method, so this makes sense.)
@ThomasBreuer ThomasBreuer added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: library release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Mar 18, 2026
@fingolfin
Copy link
Copy Markdown
Member

Hmm:

  testing: /home/runner/work/gap/gap/tst/testinstall/ctbl.tst
  ########> Diff in /home/runner/work/gap/gap/tst/testinstall/ctbl.tst:446
  # Input is:
  IrrConlon( G );;  Irr( G );;
  # Expected output:
  # But found:
  Error, Assertion failure: in CoveringTriplesCharacters
  ########
  ########> Diff in /home/runner/work/gap/gap/tst/testinstall/ctbl.tst:447
  # Input is:
  InfoText( OrdinaryCharacterTable( G ) );
  # Expected output:
  "origin: Conlon's Algorithm"
  # But found:
  "origin: Baum-Clausen Algorithm"
  ########
      5372 ms (291 ms GC) and 367MB allocated for ctbl.tst

Though I think this is after a LoadAllPackages ?

@ThomasBreuer
Copy link
Copy Markdown
Contributor Author

Though I think this is after a LoadAllPackages?

Yes. Currently I cannot reproduce the problem locally, that's why I had added messages to some Assert calls (which is anyhow not bad). I am going to find out which package causes the difference, and what is the problem.

(see the comment for the pull request)
@ThomasBreuer
Copy link
Copy Markdown
Contributor Author

The situation is as follows:

  • We have an assertion in the code of CoveringTriplesCharacters which fails after LoadAllPackages().
    Up to now, there were no CI tests for this code, the current pull requests has the side-effect that the code runs in tests.
  • The reason for the different behaviour after LoadAllPackages() is that the packages change the method rankings, in this case the two 6-argument methods for OrbitsStabilizerAlgorithm get swapped:
    • The generic method is installed more or less with requirements IsGroup and IsFinite and IsList, and a special method for pc groups is installed for IsGroup and IsPcgs. Both methods belong to the GAP library.
    • Before loading packages, the two methods have the same rank, and the special method gets priority because it gets installed later than the generic method.
    • Loading all packages changes the ranks of the involved filters, where the rank of IsGroup and IsFinite grows more than that of IsPcgs, such that the generic method gets a much higher rank than the special method.
  • For the moment, I have ranked up the special method by the rank of IsFinite. Alternatively, we could also require IsGroup and IsFinite in the special method.
  • The assertion that caused the failure was good for eventually finding the ranking problem, but it is problematic because it checks whether a group is identical with the parent of another group. The stabilizers in the results of the two OrbitsStabilizerAlgorithm methods have different Parent values. (This should be addressed in another discussion.) As long as we have no rules about parents, such checks make no sense. Hence I think this assertion should be changed or removed.

@ThomasBreuer
Copy link
Copy Markdown
Contributor Author

Concerning Parent inconsistencies, consider the following.

gap> H:= SymmetricGroup( 4 );;
gap> G:= DerivedSubgroup( H );;
gap> HasParent( G ); Parent( G ) = H;
true
true
gap> T:= TrivialSubgroup( G );; HasParent( T ); Parent( T ) = H;
true
true
gap> S:= Subgroup( G, [ G.1 ] );; HasParent( S ); Parent( S ) = G;
true
true
gap> S:= Stabilizer( G, G.1 );; HasParent( S ); Parent( S ) = H;
true
true
gap> S:= Stabilizer( G, 1 );; HasParent( S ); Parent( S );
true
Sym( [ 2 .. 4 ] )

and

gap> H:= SmallGroup( 24, 12 );;
gap> G:= DerivedSubgroup( H );;
gap> HasParent( G ); Parent( G ) = H;
true
true
gap> T:= TrivialSubgroup( G );; HasParent( T ); Parent( T ) = H;
true
true
gap> S:= Subgroup( G, [ G.1 ] );; HasParent( S ); Parent( S ) = G;
true
true
gap> S:= Stabilizer( G, G.1 );; HasParent( S ); Parent( S ) = G;
true
true

@fingolfin fingolfin merged commit 0e575e7 into gap-system:master Mar 23, 2026
35 of 36 checks passed
@ThomasBreuer ThomasBreuer deleted the TB_InfoText branch March 24, 2026 16:03
cdwensley pushed a commit to cdwensley/gap that referenced this pull request Apr 1, 2026
Whenever the `Irr` value gets computed for a group or a character table,
store in `InfoText` of the table which method was used.

And add an `Irr` method for the case that `IrrDixonSchneider` is known.
(We have at least one package with a method of higher rank than
the `IrrDixonSchneider` based method, so this makes sense.)
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: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants