Add SpinorNorm

[danielrademacher]

For the maximal subgroup project we need the SpinorNorm. In the package "matrix" of GAP3 I found the WallForm and SpinorNorm functions of this PR. I modified the SpinorNorm output slightly in order to have a more intuitive feedback. For this, the function "IsSquare" has been added.

Text for release notes

see title

[fingolfin]

Some quick comments from my phone

[fingolfin]

(Also: thanks for submitting this, I appreciate it despite my nitpicks :-)

[fingolfin]

How about IsNonZeroSquareFFE ?

But why do you need this anyway? Your determinants are always non-zero, aren't they?

[danielrademacher]

As you noted above, a function "IsSquare" should also cover 0 in finite fields. That's why the IsSquare function checks whether the input element is 0. If not, then the IsNonZeroSquareFFE function is called. Since we know that the Wall form of elements from the orthogonal group are always non-degenerate, we can call the function IsNonZeroSquareFFE directly.

[fingolfin]

So is your argument then that this is a micro optimization? You want to avoid checking for zero in a case where you know the input is zero?

But the price is that you de-optimize the general case; now any general call to the IsSquareFFE function involves the overhead of calling to another function; so you optimized one function by deoptimizing another. Although in both cases the (de)optimizations are minuscule and irrelevant unless you expect to be calling this millions of times in tight code?

I am always doubtful when it comes to adding such highly specialized functions., I mean, IsSquareFFE is already kinda niche, but adding a non-zero variant seems excessive so -- all for an optimization that hardly will ever matter?

Next one then has to wonder: Why is there IsSquareFFE and IsSquareInt and IsNonZeroSquareFFE -- but not IsNonZeroSquareInt?

Adding a new function does not just bring a benefit, it also incurs cost: in longterm maintenance, in documentation, in the evolution of the overall API, in consistency, in mental burden for users, in mental burden for developers who need to understand code. All of these are minuscule in this particular case, but it's a slippery slope.

Anyway: I certainly won't block this PR over it! But the reason I am riding this is because I think there is an important lesson here. I dislike this for the same reason (outlined above) I dislike the GroupWithSize function @ssiccha proposed in the ClassicalMatrixGroup` project

[ssiccha]

Thanks for working on this @danielrademacher! I had a cursory look and it looks really nice. Two comments:

• the square helper functions could do with some unit tests. E. G. The characteristic two case is not tested.
• While the source comments are really nice, I prefer describing what the function, its inputs and its return values do in one or a few sentences instead of explicitly specifying them in lists. Having extra lists adds redundancy and feels more clunky, unless the function is really complex. I like the tips on how to write documentation given at the beginning of the Julia manual's chapter of documentation. Here, point 4 of the list there is relevant.

[ssiccha]

I say let's also put this into the reference manual. It's a useful function and I bet once people find it, they will use it, whether it is documented or not.

[fingolfin]

So if we are talking about making these interfaces "official", then I'll review them more carefully, too...

[danielrademacher]

Does anything need to be changed in the functions or is it enough to incorporate the remaining remarks?

[fingolfin]

Actually it should be made clear (and perhaps also checked) that this function only applies to odd characteristic. Right now it always returns +1 in even characteristic; which is of course consistent with the notion of Spinor norm; but if one is interested in even characteristic, then one uses a different definition (but I need to look up the details, it's been too long). Anyway, if you don't need even char, it's fine to not cover it; but it simply should be made clear what his here and what not.

[danielrademacher]

In Taylors book "The geometry of the classical groups" there is no difference in the characteristic. But if there is an other definition, I change this.

[fingolfin]

You are right, in Taylor's book there is no special case here. Instead, Taylor gives a definition of SO(V) that differs from that used in GAP: in even characteristic, in GAP we have that SO and GO return the same group. But for Taylor, SO is always an index 2 subgroup of GO, namely the kernel of the Dickson invariant D:O(V) -> C_2. This then renders Theorem 11.51 true, where he shows that \Omega(V) is the intersection of SO(V) with the kernel of the Spinor norm. Unfortunately, this is false in GAP.

However, other authors handle this differently from Taylor, by using the "usual" definition for SO(V) (kernel of the determinant map on O(V)) and then defining the Spinor norm differently (sorry got no good reference right now, but see e.g. https://groupprops.subwiki.org/wiki/Spinor_norm). And Magma simply returns the Dickson invariant in char 2.

I am not saying this is better, just that it is a potential point of confusion. So it'd be good to document explicitly what happens in char 2.

So I see the following options:

1. document that this function always returns 1 in char 2

• correct and consistent with Taylor
• useless for testing whether a matrix is contained in Omega in char 2 (but we can add a function DicksonInvariant for that)
• changing this behavior in the future could be problematic

2. change the function to produce an error in char 2, and document that

• useless in the same sense as option 1
• defers the decision on what to do in char 2, allowing us to go either way in the future

3. change the function to return the Dickson invariant in char 2

• this way the "theorem" Omega(V) = SO(V) \cap ker(SpinorNorm) becomes true within GAP
• useful for testing membership in Omega in char 2
• not consistent with Definition used by Taylor
• consistent with that used elsewhere, e.g. in Magma

Right now you went with option 2, which is fine by me. It means we can decide on whether to go to option 1 or 3 at a later point in the future.

[fingolfin]

@danielrademacher thank you for the great effort and keeping at it. I am hopeful that future PRs will go through easier