The functions `GO`, `GU`, `Omega`, `SO`, `SU`, and `Sp` now allow to specify an invariant form for the group in question; the methods that do the work are provided by the Forms package #4489

ThomasBreuer · 2021-05-12T22:21:57Z

Text for release notes

The documentation and the code of the functions GO, GU, Omega, SO, SU, and Sp have been extended such that one can specify an invariant form for the group in question. The methods that do the work will be provided via the Forms package.

(End of text for release notes)

See gap-packages/forms/pull/4 for the proposed corresponding changes in the Forms package.
The pull request is intended to resolve issue #500.

The documentation and the code of the functions `GO`, `GU`, `Omega`, `SO`, `SU`, `Sp` have been extended such that the Forms package can provide the additional methods for the new feature.

fingolfin

Thanks for working on this, I like the direction, but have some suggestions / nitpicks

fingolfin · 2021-05-14T09:32:18Z

grp/classic.gd

+  if   Length( arg ) = 1 then
+    # form (matrix, form object, or group with stored form)
+    return GeneralOrthogonalGroupCons( IsMatrixGroup, arg[1] );
+  elif Length( arg ) = 2 and IsInt( arg[1] ) then


Any reason why "final argument is ring or integer" is not checked here, but is checked in the three argument case below?

My aim is to add new variants of arguments. For that, I have to perform enough tests to distinguish the possible cases, and I have tried not to introduce more tests. (In the original code for the two-argument case, none of the argument types is checked.)
It was not my aim to clean the available code.

grp/classic.gd

fingolfin · 2021-05-14T10:00:57Z

grp/classic.gd

    return GeneralOrthogonalGroupCons( IsMatrixGroup,arg[1],arg[2],arg[3] );
+  elif Length( arg ) = 3 and IsInt(arg[1]) and
+    (IsInt(arg[2]) or IsRing(arg[2])) and


Actually, it seems the possibility to pass in a ring is undocumented for GO, SO, etc. (however, it is documented for GL and SL). That seems like an oversight to me, perhaps we should also fix that (not necessarily in this PR, of course).

I am not sure whether this was an oversight.
For GL, SL, and Sp, we have also methods for constructing the group in question over a residue class ring of the integers, but finite fields are the only supported rings for the orthogonal groups.

It would not be a problem to admit also GO( 3, GF(5) ), etc.; the documentation should then state that currently only finite fields are supported, analogous to GL, SL, and Sp.

We already admit GO(3,GF(5)), it just is undocumented

Perhaps the word "admit" was not a good choice.
I wrote in a commit message:

Concerning the possibility to enter GF(q) instead of q (or more generally a ring) as an argument:
This was already documented and supported for Sp, and supported but not documented for GO and SO.
Now it will be supported but not documented also for Omega.
(For GU and SU, supporting GF(q) (or GF(q^2)?) would be asking for trouble:
The GAP library uses q where the Forms package expects GF(q^2).)

fingolfin · 2021-05-14T10:03:16Z

grp/classic.gd

@@ -193,13 +192,17 @@ DeclareConstructor( "GeneralOrthogonalGroupCons",

 #############################################################################
 ##
-#F  GeneralOrthogonalGroup( [<filt>, ][<e>, ]<d>, <q> ) .  gen. orthog. group
-#F  GO( [<filt>, ][<e>, ]<d>, <q> )
+#F  GeneralOrthogonalGroup( [<filt>, ][<e>, ]<d>, <q>[, <form>] )


But doesn't the form already imply the value for d and e? If we accept all three, how are they supposed to interact / be sanity checked?

I guess only the base field would perhaps we in need of overriding (i.e., it seems reasonably to say: "use this form given over GF(2) but create a group over GF(8)").

My idea was the following:
One can write GO( 3, 5, form ) in order to express that one wants the group in question, with the given form as an additional requirement. One can also write GO( form ) (which is also a documented variant), if one wants to express that the given form determines the other parameters.
In code examples, I would probably prefer the former variant.

But if the former is done, shouldn't we then also check that the form matches the given values for d and q? Or do we expect all installed methods to re-implement that check?

The GAP library does not know about form objects, and the code dealing with this argument belongs to the Forms package.
We could check some conditions about a matrix or a group with stored form in the global functions in the GAP library, but only the code in the Forms package can decide whether the given argument makes sense.

grp/classic.gd

- rewrote/simplified the global functions `GO`, `GU`, `SO`, `SU` as suggested (introduced auxiliary functions `DescribesInvariantQuadraticForm`, etc.); now the argument checks are more uniform - concerning the possibility to enter `GF(q)` instead of `q` (or more generally a ring) as an argument: This was already documented and supported for `Sp`, and supported but not documented for `GO` and `SO`. Now it will be supported but not documented also for `Omega`. (For `GU` and `SU`, supporting `GF(q)` (or `GF(q^2)`?) would be asking for trouble: The GAP library uses `q` where the Forms package expects `GF(q^2)`.) - fixed the `IsPermGroup` constructor for `OmegaCons` (strange, there were even tests for the error messages that were caused by the wrong installation) - added/changed a few manual examples for `GU`, `SU`, etc. - adjusted some error messages in the test files

about the parameters [e, ], d, q in the case that a form is prescribed in GO, GU, Omega, SO, Sp, SU: They can be omitted, and one gets an error message if they are given and contradict the given form.

fingolfin

There was a new release of the forms package today (version 1.2.6). So let's move forward with this PR. As far as I understand, we can merge it now, independent of forms, right?

I'll wait with merging for (a) the CI tests which I just restarted by closing and reopening this PR, and (b) for @ThomasBreuer to comment if this PR is ready from his perspective

ThomasBreuer · 2021-07-29T16:34:59Z

@fingolfin This pull requests depends on the new version of the Forms package in the sense that the documentation of the GAP functions in question promises that certain functionality is available if the Forms package is available.
If it is sure that the next released version of GAP will contain at least Forms 1.2.6 then we can merge the pull request.
Formally, it would be perhaps the safest to state explicitly in the abovementioned documentation that Forms >= 1.2.6 is needed for the promised functionality; shall I adjust the pull request this way?

fingolfin · 2021-07-29T21:20:53Z

Yes, please update the PR accordingly

that is needed in order to get the new feature

ThomasBreuer · 2021-07-30T07:12:51Z

@fingolfin O.k.; I have updated the PR, the tests have passed, I will merge the PR.

admit specifying a form for classical groups

2810280

The documentation and the code of the functions `GO`, `GU`, `Omega`, `SO`, `SU`, `Sp` have been extended such that the Forms package can provide the additional methods for the new feature.

ThomasBreuer added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: library release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels May 12, 2021

ThomasBreuer mentioned this pull request May 12, 2021

extend the GAP library functions GO, GU, etc. gap-packages/forms#4

Merged

fingolfin requested changes May 14, 2021

View reviewed changes

ThomasBreuer added 2 commits May 17, 2021 22:40

added comments

79e5030

about the parameters [e, ], d, q in the case that a form is prescribed in GO, GU, Omega, SO, Sp, SU: They can be omitted, and one gets an error message if they are given and contradict the given form.

ThomasBreuer force-pushed the TB_support_form_as_argument branch from ad28361 to 79e5030 Compare May 20, 2021 20:45

fingolfin closed this Jul 29, 2021

fingolfin reopened this Jul 29, 2021

fingolfin approved these changes Jul 29, 2021

View reviewed changes

specify the version of the Forms package

0c479f6

that is needed in order to get the new feature

ThomasBreuer merged commit 53ee4a3 into gap-system:master Jul 30, 2021

ThomasBreuer deleted the TB_support_form_as_argument branch July 30, 2021 07:13

This was referenced Jul 30, 2021

Fix DescribesInvariantQuadraticForm, etc #4623

Merged

Omega(n,q) is not always a subgroup of SO(n,q) #500

Closed

fingolfin changed the title ~~admit specifying a form for classical groups~~ The functions GO, GU, Omega, SO, SU, and Sp now allow to specify an invariant form for the group in question; the methods that do the work are provided by the Forms package Aug 17, 2022

fingolfin added release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Aug 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

The functions `GO`, `GU`, `Omega`, `SO`, `SU`, and `Sp` now allow to specify an invariant form for the group in question; the methods that do the work are provided by the Forms package #4489

The functions `GO`, `GU`, `Omega`, `SO`, `SU`, and `Sp` now allow to specify an invariant form for the group in question; the methods that do the work are provided by the Forms package #4489

ThomasBreuer commented May 12, 2021 •

edited

Loading

fingolfin left a comment

fingolfin May 14, 2021

ThomasBreuer May 14, 2021

fingolfin May 14, 2021

ThomasBreuer May 14, 2021

fingolfin May 18, 2021

ThomasBreuer May 18, 2021

fingolfin May 14, 2021

ThomasBreuer May 14, 2021

fingolfin May 18, 2021

ThomasBreuer May 18, 2021

fingolfin left a comment

ThomasBreuer commented Jul 29, 2021

fingolfin commented Jul 29, 2021

ThomasBreuer commented Jul 30, 2021

The functions GO, GU, Omega, SO, SU, and Sp now allow to specify an invariant form for the group in question; the methods that do the work are provided by the Forms package #4489

The functions GO, GU, Omega, SO, SU, and Sp now allow to specify an invariant form for the group in question; the methods that do the work are provided by the Forms package #4489

Conversation

ThomasBreuer commented May 12, 2021 • edited Loading

Text for release notes

(End of text for release notes)

fingolfin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fingolfin left a comment

Choose a reason for hiding this comment

ThomasBreuer commented Jul 29, 2021

fingolfin commented Jul 29, 2021

ThomasBreuer commented Jul 30, 2021

The functions `GO`, `GU`, `Omega`, `SO`, `SU`, and `Sp` now allow to specify an invariant form for the group in question; the methods that do the work are provided by the Forms package #4489

The functions `GO`, `GU`, `Omega`, `SO`, `SU`, and `Sp` now allow to specify an invariant form for the group in question; the methods that do the work are provided by the Forms package #4489

ThomasBreuer commented May 12, 2021 •

edited

Loading