Opened 2 years ago

Closed 2 years ago

Last modified 18 months ago

#26028 closed defect (fixed)

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

Reported by: soehms Owned by:
Priority: major Milestone: sage-8.4
Component: group theory Keywords: matrix groups, unitary, symplectic, orthogonla, classical, invariant bilinear form
Cc: tscrim Merged in:
Authors: Sebastian Oehms Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 331e5cb (Commits) Commit:
Dependencies: #25761 Stopgaps:


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]
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,['matrix'].matrix())
sage: invariant_form ==

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()
sage: GO3_5 = GO(3,5)
sage: GO3_5.order()
sage: GO3_5.is_isomorphic(GO3_25)

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

Change History (15)

comment:1 Changed 2 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)

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 2 years ago by tscrim

Actually, it may not be a problem with gap:

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

comment:3 Changed 2 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()
sage: GO(3,8).cardinality()

The same change needs to be made for SO.

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

comment:4 Changed 2 years ago by soehms

  • Branch set to u/soehms/classical_grps_invariant_form-26028

comment:5 Changed 2 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

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:

591cc0bMerge branch 'develop' into is_unitary_for_finite_fields-25761
2a8dd57initial implementation
d97fc1eMerge branch 'classical_grps_invariant_form-26028' of /home/sebastian/speedport/sagedev into classical_grps_invariant_form-26028

comment:6 Changed 2 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:

588b4d3Did some formatting fixes and other small reviewer tweaks.

comment:7 Changed 2 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 2 years ago by git

  • Commit changed from 588b4d37dc33a628839bc1aff55e73567a1e93f0 to 331e5cbc6327a37bb9b29af7aba236999e8d72b5

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

331e5cbFixing doctest issues.

comment:9 Changed 2 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 2 years ago by soehms

  • Status changed from needs_review to positive_review

Sorry, that was stupid!

comment:11 Changed 2 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 2 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: Changed 18 months ago by jdemeyer

  • Commit 331e5cbc6327a37bb9b29af7aba236999e8d72b5 deleted

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

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

See #27427

comment:14 in reply to: ↑ 13 ; follow-up: Changed 18 months ago by tscrim

Replying to jdemeyer:

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

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

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))

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 18 months ago by soehms

Replying to tscrim:

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

And I am sorry for doing that! It was before I learned about that convention, as well. That will not occur any more!

Note: See TracTickets for help on using tickets.