Opened 4 years ago

Closed 4 years ago

# Bug-Fixes and improvements with respect to the bilinear invariant form of classical matrix groups

Reported by: Owned by: soehms major sage-8.4 group theory matrix groups, unitary, symplectic, orthogonla, classical, invariant bilinear form tscrim Sebastian Oehms Travis Scrimshaw N/A 331e5cb #25761

### Description

1) Bug with respect to the symplectic groups:

```sage: Sp(4, QQ).invariant_form()
[0 0 0 1]
[0 0 1 0]
[0 1 0 0]
[1 0 0 0]
```

This bilinear form isn't alternating. Thus Sp(4, QQ) in fact is an orthogonal group!

2) Bug with respect to the unitary groups:

```sage: G = GU(2,2); G
General Unitary Group of degree 2 over Finite Field in a of size 2^2
sage: m = matrix(G.base_ring(), 2, 2, (1,1,1,0)); m
[1 1]
[1 0]
sage: m in [g.matrix() for g in G]
True
sage: G(m)
Traceback (most recent call last):
...
TypeError: matrix must be unitary
```

Note, that this bug is not fixed by ticket #25761. The reason for this bug is this:

```sage: invariant_form = matrix(G.base_ring(), 2,2, G.gap().InvariantSesquilinearForm()['matrix'].matrix())
sage: invariant_form == G.one().matrix()
False
```

Furthermore, There is no method to obtain the invariant from of a unitary group analogues as for symplectic and orthogonal groups.

```sage: GU(3,2).invariant_form()
Traceback (most recent call last):
...
AttributeError: 'UnitaryMatrixGroup_gap_with_category' object has no attribute 'invariant_form'
```

3) Bug with respect to the orthogonal group (but not with respect to the invariant form):

```sage: GO3_25 = GO(3,25)
sage: GO3_25.order()
240
sage: GO3_5 = GO(3,5)
sage: GO3_5.order()
240
sage: GO3_5.is_isomorphic(GO3_25)
True
```

Furthermore, it should be possible to define generic classical matrix groups with respect to a user given invariant bilinear form.

### comment:1 Changed 4 years ago by tscrim

Well, apparently -- according to the doc -- `Sp` is an orthogonal group:

Return the quadratic form preserved by the orthogonal group.

However, to note that for finite fields, it does return the correct result (in part, but GAP does the computation). The generic implementation seems to have forgotten that it should be a skew-symmetric bilinear form. That should be simple to fix.

You seem to have come across a bigger issue for the last one

```sage: [GO(3,2^k).cardinality() for k in range(1,5)]
[6, 6, 6, 6]
```

Contrast with

```sage: ret = []
sage: X = GL(3,4)
sage: G = GO(3,4)
sage: it = iter(X)
sage: for _ in range(1000):
....:     x = next(it)
....:     try:
....:         ret.append(G(x)) # raises an error if not in G
....:     except TypeError:
....:         pass
....:
sage: len(ret)
20
```

So the matrix group `GO(3,4)` is quite a bit bigger than `6`. I am pretty certain this comes from the groups having the same generators:

```sage: GO(3,4).gens()
(
[1 0 0]  [1 0 0]
[1 1 1]  [0 0 1]
[0 0 1], [0 1 0]
)
sage: GO(3,2).gens()
(
[1 0 0]  [1 0 0]
[1 1 1]  [0 0 1]
[0 0 1], [0 1 0]
)
```

This is a bug with upstream GAP not creating the generators correctly.

IMO each of these issues should be a separate ticket.

### comment:2 Changed 4 years ago by tscrim

Actually, it may not be a problem with gap:

```sage: GO(3,4).gap()
GO(0,3,2)
sage: GO(3,2).gap()
GO(0,3,2)
```

### comment:3 Changed 4 years ago by tscrim

Yep, I am now sure the issue is with the initializer. In `GO`:

```    if is_FiniteField(ring):
cmd  = 'GO({0}, {1}, {2})'.format(e, degree, ring.characteristic())
return OrthogonalMatrixGroup_gap(degree, ring, False, name, ltx, cmd)
```

versus, say `Sp`:

```        cmd  = 'Sp({0}, {1})'.format(degree, ring._gap_init_())
return SymplecticMatrixGroup_gap(degree, ring, True, name, ltx, cmd)
```

I think the former should be in the same as the latter or `cardinality()`. With changing `characteristic` -> `cardinality`, I at least get something reasonable:

```sage: GO(3,4).gens()
(
[    1     0     0]  [1 0 0]
[    0     a     0]  [1 1 1]
[    0     0 a + 1], [0 1 0]
)
sage: GO(3,4).cardinality()
60
sage: GO(3,8).cardinality()
504
```

The same change needs to be made for `SO`.

Last edited 4 years ago by tscrim (previous) (diff)

### comment:4 Changed 4 years ago by soehms

• Branch set to u/soehms/classical_grps_invariant_form-26028

### comment:5 Changed 4 years ago by soehms

• Commit set to d97fc1e944b707f026f6d0626e5d769a6e2dd066
• Dependencies set to #25761
• Keywords invariant bilinear form added
• Status changed from new to needs_review

Thank your for these suggestions. I've already worked on the ticket but didn't push it up right after creating the ticket since I had build problems after merging with 8.4.beta0. What I do exactly coincides with your suggestions. In detail this is:

The three Bug-fixes I implement in

1) method `invariant_form` of `SymplecticMatrixGroup_generic`.

2) method `_check_matrix` of `UnitaryMatrixGroup_generic`.

3) new function `_OG` in `orthogonal.py`.

I add a new method `invariant_form` to the classes `UnitaryMatrixGroup_generic` and `UnitaryMatrixGroup_gap`.

Since the according method of `OrthogonalMatrixGroup_generic` was named `invarinat_bilinear_form` I add an alias for `invariant_form` there. Furthermore, I replace the (in the generic case) identical method `invariant_quadratic_form` by an alias, as well.

In order to implement an optional argument to define classical matrix groups with respect to a user given invariant bilinear form, I do the following:

I add the attribute `_user_invariant_form_` to the class `NamedMatrixGroup_generic` together with an optional argument `invariant_form` to set it (taking care of CachedRepresentation?). Furthermore, I add the function `normalize_args_invariant_form` (analogous to `normalize_args_vectorspace`).

Than, I add the optional argument `invariant_form` to the functions `GO, SO, GU, SU` and `Sp` and transfer them to `NamedMatrixGroup_generic` after normalization. Since `GO` and `SO` (resp. `GU` and `SU`) use nearly identical code I capsule these in local functions `_OG` and `_UG` to avoid to much duplicated code.

The behavior with respect to the new argument I realize in the following way:

The argument is checked to define a non singular square-matrix over the given ring according to the given degree. Furthermore, the matrix is checked to by symmetric, hermitian or alternating according to the case of classical group. It is not checked to be positive definite in the case of orthogonal and unitary groups. But it will be printed in the representation string and latex string if it is not. Furthermore, the user given invariant bilinear form will always be printed in these representation strings.

Finally, I add the invariant bilinear form to the error message of the `_checke_matrix` method, but in the cases of orthogonal and unitary groups only if it is different from the identity matrix.

Well, this is a lot of stuff. If you think not all of this is necessary, please correct it!

New commits:

 ​591cc0b `Merge branch 'develop' into is_unitary_for_finite_fields-25761` ​2a8dd57 `initial implementation` ​d97fc1e `Merge branch 'classical_grps_invariant_form-26028' of /home/sebastian/speedport/sagedev into classical_grps_invariant_form-26028`

### comment:6 Changed 4 years ago by tscrim

• Branch changed from u/soehms/classical_grps_invariant_form-26028 to public/groups/invariant_form_classical_gps-26028
• Commit changed from d97fc1e944b707f026f6d0626e5d769a6e2dd066 to 588b4d37dc33a628839bc1aff55e73567a1e93f0
• Reviewers set to Travis Scrimshaw

I made a bunch of small tweaks. Most of them were doc formatting and convention things. If my changes are good, then you can set a positive review.

New commits:

 ​588b4d3 `Did some formatting fixes and other small reviewer tweaks.`

### comment:7 Changed 4 years ago by soehms

thank you very much, Travis! I agree with all your changes. But there is one problem left:

I saw that you forget to change the doctest concerning the `NotImplementedError` to the new text ("... for finite groups is fixed by GAP"). On the other hand all doctests passed!

Why isn't this detected? The doctest after the blank line is completely ignored (you can type any error into it). But if I delete the blank-line I'm getting a doctest failure with respect to the former test.

What can we do to ensure the test is performed?

### comment:8 Changed 4 years ago by git

• Commit changed from 588b4d37dc33a628839bc1aff55e73567a1e93f0 to 331e5cbc6327a37bb9b29af7aba236999e8d72b5

Branch pushed to git repo; I updated commit sha1. New commits:

 ​331e5cb `Fixing doctest issues.`

### comment:9 Changed 4 years ago by tscrim

So the issue was that it was `sage foo` not `sage: foo` for those tests. So Sage's doctest framework did not pick up that they should actually be tests. Fixed.

### comment:10 Changed 4 years ago by soehms

• Status changed from needs_review to positive_review

Sorry, that was stupid!

### comment:11 Changed 4 years ago by tscrim

I was actually a little surprised the doctest framework didn't catch that, but there are only so many things it can do.

### comment:12 Changed 4 years ago by vbraun

• Branch changed from public/groups/invariant_form_classical_gps-26028 to 331e5cbc6327a37bb9b29af7aba236999e8d72b5
• Resolution set to fixed
• Status changed from positive_review to closed

### comment:13 follow-up: ↓ 14 Changed 3 years ago by jdemeyer

• Commit 331e5cbc6327a37bb9b29af7aba236999e8d72b5 deleted

What's the reason for the `try`/`except` here?

```        try:
if not invariant_form.is_positive_definite():
inserted_text = 'with respect to non positive definite hermitian form'
except:
pass
```

See #27427

### comment:14 in reply to: ↑ 13 ; follow-up: ↓ 15 Changed 3 years ago by tscrim

What's the reason for the `try`/`except` here?

```        try:
if not invariant_form.is_positive_definite():
inserted_text = 'with respect to non positive definite hermitian form'
except:
pass
```

See #27427

For this:

```sage: F.<a> = FiniteField(5^3)
sage: M = matrix([[a,1],[a^2,a+2]])
sage: M.is_positive_definite()
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-7-c019faa7a9b7> in <module>()
----> 1 M.is_positive_definite()

/home/uqtscrim/sage/local/lib/python2.7/site-packages/sage/matrix/matrix2.pyx in sage.matrix.matrix2.Matrix.is_positive_definite (build/cythonized/sage/matrix/matrix2.c:85468)()
12710             real = False
12711         else:
> 12712             raise ValueError("Could not see {} as a subring of the "
12713                     "real or complex numbers".format(R))
12714

ValueError: Could not see Finite Field in a of size 5^3 as a subring of the real or complex numbers
```

(Sorry Jeroen for missing the bare `except:` during my review.)

### comment:15 in reply to: ↑ 14 Changed 3 years ago by soehms

(Sorry Jeroen for missing the bare `except:` during my review.)