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 InvariantQuadraticForm for Omega(-1, 2*d, 2^n) #4327

Merged
merged 1 commit into from
Mar 23, 2021

Conversation

fingolfin
Copy link
Member

Also add proper tests to check that the quadratic forms are indeed correct
(the existing tests were too weak).

Fixes #4323

Some background: We didn't even store the InvariantQuadraticForm for these groups up to GAP 4.9; it was only added in 4.10, via my PR #2577. In there, I also added tests to verify the quadratic form, but those tests were insufficient (rather obviously so, in retrospect sigh). This adds a test similar to what Thomas used to highlight the issue in his bug report.

@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 release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Mar 21, 2021
Copy link
Contributor

@ThomasBreuer ThomasBreuer left a comment

Choose a reason for hiding this comment

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

Thanks.
Is there anything one can say why the matrix of the quadratic form in OmegaMinus can look like this? (The structure of the generators yields that one has to work only for d = 4.)
In CheckQuadraticForm, checking the relation between the matrices of the bilinear and the quadratic form still makes sense, it is just not sufficient in even characteristic.

@hulpke
Copy link
Contributor

hulpke commented Mar 22, 2021

Thanks.
Is there anything one can say why the matrix of the quadratic form in OmegaMinus can look like this? (The structure of the generators yields that one has to work only for d = 4.)

I believe the generators are taken from `Rylands, Taylor: Matrix generators for the Orthogonal Group, JSC 25 (1998), 351--360, and p. 359 (about the middle) gives the form for Omega-

@fingolfin
Copy link
Member Author

@ThomasBreuer I restored the check for the relation between the matrices of the bilinear and the quadratic form -- I did not mean to disable it in the first place, thanks for spotting this!

@ThomasBreuer
Copy link
Contributor

@hulpke Thanks for the hint. Yes, the Rylands/Taylor paper is mentioned in the GAP manual, and it describes the form.
Note that the paper does not distinguish odd and even characteristic. Following the paper, one could simplify the proposed code by setting x[m+1,d-m+1] := -xi;.

Also add proper tests to check that the quadratic forms are indeed correct
(the existing tests were too weak).
@fingolfin
Copy link
Member Author

@ThomasBreuer ah that's of course much nicer. Pushed it.

@ThomasBreuer ThomasBreuer merged commit 379de8b into gap-system:master Mar 23, 2021
@fingolfin fingolfin deleted the mh/fix-omega-forms branch March 24, 2021 15:01
@fingolfin fingolfin changed the title Fix InvariantQuadraticForm for Omega(-1, 2*d, 2^n) Fix InvariantQuadraticForm for Omega(-1, 2*d, 2^n) Aug 17, 2022
@fingolfin 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
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.

More problems with orthogonal groups ...
3 participants