Skip to content

Conversation

@ThomasBreuer
Copy link
Contributor

@ThomasBreuer ThomasBreuer commented Jul 4, 2018

For odd d and even q,
the generators for the matrix groups Omega(0,d,q)
from the Rylands/Taylor paper were not correct;
one would have to transpose the matrices in order to get
a group that respects a form as required.

Instead of transposing the generators, we delegate to SO,
which has the advantage to reduce the possible confusion
about the choice of different forms for related orthogonal groups.

Resolves #2576

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

Looks good to me, just have two minor (and optional) remarks.

#
gap> Omega(3,2);
Omega(0,3,2)
GO(0,3,2)

This comment was marked as resolved.

x2:= IdentityMat( d, f );
x2[ m+1 ][m]:= o;
x2[ m+2 ][m]:= o;
x:= x1 * x2;

This comment was marked as resolved.

@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements labels Jul 4, 2018
For odd d and even q,
the generators for the matrix groups Omega(0,d,q)
from the Rylands/Taylor paper were not correct;
one would have to transpose the matrices in order to get
a group that respects a form as required.

Instead of transposing the generators, we delegate to `SO`,
which has the advantage to reduce the possible confusion
about the choice of different forms for related orthogonal groups.
@fingolfin
Copy link
Member

LGTM!

@codecov
Copy link

codecov bot commented Jul 4, 2018

Codecov Report

Merging #2613 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2613      +/-   ##
==========================================
+ Coverage    74.8%    74.8%   +<.01%     
==========================================
  Files         479      479              
  Lines      242251   242238      -13     
==========================================
- Hits       181206   181198       -8     
+ Misses      61045    61040       -5
Impacted Files Coverage Δ
grp/classic.gd 100% <ø> (ø) ⬆️
grp/classic.gi 95.57% <100%> (-0.06%) ⬇️
lib/stbcrand.gi 90.56% <0%> (+0.09%) ⬆️
hpcgap/lib/hpc/stdtasks.g 64.45% <0%> (+1.02%) ⬆️

@fingolfin fingolfin merged commit 2c27b99 into gap-system:master Jul 4, 2018
@fingolfin fingolfin added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Sep 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: bug Issues describing general bugs, and PRs fixing them kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: added PRs introducing changes that have since been mentioned in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants