Stabilisers of timelike vectors, some refactoring for K3Auto.jl

[simonbrandhorst]

No description provided.

[simonbrandhorst]

If the tests pass, this should be ready to merge. (I am aware that the stabilizer computation is probably slow because of the slow action function.)

[fingolfin]

Just wondering: If this function returns a stabilizer, then why is it called automorphism_group and not e.g. stabilizer ?

[simonbrandhorst]

Because it is a finite stabilizer in an infinite group I do not have direct access to, I cannot simply call stabilizer(group, vector).
Another reason is that it is called the same in magma:
https://magma.maths.usyd.edu.au/magma/handbook/text/349#3451

[codecov]

Codecov Report

Attention: Patch coverage is 88.88889% with 6 lines in your changes missing coverage. Please review.

Project coverage is 84.41%. Comparing base (4bcd2d8) to head (54cc06a).
Report is 33 commits behind head on master.

Files with missing lines	Patch %	Lines
src/AlgebraicGeometry/Surfaces/K3Auto.jl	87.50%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4534      +/-   ##
==========================================
- Coverage   84.55%   84.41%   -0.14%     
==========================================
  Files         672      672              
  Lines       88873    89193     +320     
==========================================
+ Hits        75145    75294     +149     
- Misses      13728    13899     +171     

Files with missing lines	Coverage Δ
src/Groups/matrices/form_group.jl	97.56% <100.00%> (+0.01%)	⬆️
src/AlgebraicGeometry/Surfaces/K3Auto.jl	93.30% <87.50%> (+0.29%)	⬆️

... and 66 files with indirect coverage changes

[fieker]

On Mon, Feb 03, 2025 at 01:36:46PM -0800, Simon Brandhorst wrote: @simonbrandhorst commented on this pull request. > @@ -729,6 +729,30 @@ function isometry_group(L::ZZLat; algorithm = :direct, depth::Int = -1, bacher_d end::MatrixGroup{QQFieldElem, QQMatrix} end +automorphism_group(L::ZZLat, v::Vector{QQFieldElem}) = isometry_group(L, matrix(QQ,1,degree(L),v)) + +""" + automorphism_group(L::ZZLat, v::QQMatrix; kwargs...) -> MatrixGroup + +Return the stabilizer of the matrix ``v`` in the orthogonal group of ``L``. Because it is a finite stabilizer in an infinite group I do not have direct access to, I cannot simply call `stabilizer(group, vector)`. Another reason is that it is called the same in magma: https://magma.maths.usyd.edu.au/magma/handbook/text/349#3451

Not quite sure I like this argument - furthermore, the name (in Magma) is not related to ZZLat, but to some rather special, user defined, lattices over number fields....

…

-- Reply to this email directly or view it on GitHub: #4534 (comment) You are receiving this because you are subscribed to this thread. Message ID: ***@***.***>

[simonbrandhorst]

I don't have a strong opinion here, I can also live with a private name _automorphism_group
or any other one that you come up with. But stabilizer(L::ZZLat, v::QQMatrix) does not seem right.
By the way, a notation sometimes used for this stabilizer is O(L,v), hence automorphism_group(L,v) is not that far fetched.

[simonbrandhorst]

@fieker Please note that our ZZLat are different from the lattices in magma, because the ones in magma are positive definite and ours have no restriction on the signature.

So how to proceed here?

[fingolfin]

To be clear, I don't care too much how this function is named -- what I object to is describing a function called automorphism_group by "Returns the stabilizer ...".

If you want to call this automorphism_group then IMHO the description should make it clear in how far what it returns is an automorphism group. That it internally does the computation via a stabilizer computation is an implementation detail.

But generally I'd expect an automorphism_group method to be unary -- it isn't here. So to me this suggests that either a type / abstraction is missing for something; or the name is not appropriate. Of course this is just a heuristic argument. I am happy to be convinced otherwise.

[simonbrandhorst]

I concur. In an ideal world the user could type
stabilizer(orthogonal_group(L), v) .
The challenge is that orthogonal_group(L) is an infinite, finitely generated group and generators are too expensive to compute in general.
In special cases where the group is finite, we return a MatrixGroup ortherwise a
NotImplementedError.

What could help here is an abstract type for matrix groups, or an extra type parameter. Then I could dispatch on that.

Shall we settle in the meantime on stabilizer_in_orthogonal_group(L, v)?

[fingolfin]

Yeah, stabilizer(orthogonal_group(L), v) seems much better to me.

We should have orthogonal_group(L) return a "lazy" group, that has no generators defined initially (so gens called on it would either trigger a (possibly expensive) computation, or even thrown an error (if no method is available).

We already do this for e.g. GL:

julia> GL(3, QQ)
GL(3,QQ)

julia> gens(ans)
ERROR: no generators are known for the matrix group of type GL over Rational field

(Here the error could even be made to say "error, this groups is not finitely generated")

Calling it stabilizer_in_orthogonal_group for now so we can proceed and resolving it properly later sounds good to me.

[simonbrandhorst]

O.K., setting G.descr = :IntegalOrthogonalGroup could do the trick.
It will make dispatch rather clumsy and maybe type unstable though.
But maybe this dispatch is not needed too often.

[fingolfin]

The whole system with descr needs to be eventually revised for various reasons, but I am not aware of type stability issues (at least not more than with anything else in Oscar; if you know something I am happy to look into a fix).

Of course this could also be handled by a bespoke type for these orthogonal groups; they don't need to be matrix group instances.