Matrix groups over ZZ (and possibly other rings) weirdness

[lgoettgens]

Describe the bug
The trivial matrix group of dim 1x1 behaves differently depending on the base ring.

To Reproduce

julia> GQQ = matrix_group([matrix(QQ, 1, 1, [1])])
Matrix group of degree 1
  over rational field

julia> GZZ = matrix_group([matrix(ZZ, 1, 1, [1])])
Matrix group of degree 1
  over integer ring

julia> describe(GQQ)
"1"

julia> describe(GZZ)
ERROR: MethodError: no method matching isfinite(::ZZRing)

Closest candidates are:
  isfinite(::DecFP.Dec128)
   @ DecFP ~/.julia/packages/DecFP/Cud5F/src/DecFP.jl:540
  isfinite(::RealFieldElem)
   @ Nemo ~/.julia/packages/Nemo/tzyHK/src/arb/Real.jl:476
  isfinite(::Missing)
   @ Base missing.jl:101
  ...

Stacktrace:
 [1] is_infinite(x::ZZRing)
   @ Nemo ~/.julia/packages/Nemo/tzyHK/src/Infinity.jl:105
 [2] describe(G::MatrixGroup{ZZRingElem, ZZMatrix})
   @ Oscar ~/code/julia/Oscar.jl/src/Groups/GAPGroups.jl:2443
 [3] top-level scope
   @ REPL[45]:1

julia> small_generating_set(GQQ)
1-element Vector{MatrixGroupElem{QQFieldElem, QQMatrix}}:
 [1]

julia> small_generating_set(GZZ)
ERROR: MethodError: no method matching isfinite(::ZZRing)

Closest candidates are:
  isfinite(::DecFP.Dec128)
   @ DecFP ~/.julia/packages/DecFP/Cud5F/src/DecFP.jl:540
  isfinite(::RealFieldElem)
   @ Nemo ~/.julia/packages/Nemo/tzyHK/src/arb/Real.jl:476
  isfinite(::Missing)
   @ Base missing.jl:101
  ...

Stacktrace:
 [1] is_infinite(x::ZZRing)
   @ Nemo ~/.julia/packages/Nemo/tzyHK/src/Infinity.jl:105
 [2] small_generating_set(G::MatrixGroup{ZZRingElem, ZZMatrix})
   @ Oscar ~/code/julia/Oscar.jl/src/Groups/GAPGroups.jl:641
 [3] top-level scope
   @ REPL[47]:1

This boils down to is_finite(::ZZRing) not being implemented. But I think adding this just fixes the symptom here, not the underlying issue. Instead,

Oscar.jl/src/Groups/GAPGroups.jl

Line 2443 in 615af44

if G isa MatrixGroup && is_infinite(base_ring(G))

and

Oscar.jl/src/Groups/GAPGroups.jl

Line 641 in 615af44

if G isa MatrixGroup && is_infinite(base_ring(G))

and other similar places should have a workaround for is_finite not being available for base_ring(G).

Thanks to @flyingapfopenguin for making me aware of this issue.

[fingolfin]

First off, I really think is_(in)finite should be made to work for ZZ. The reason it works for QQ is this method in AA:

is_finite(F::Field) = characteristic(F) != 0 && throw(NotImplementedError(:is_finite, F))

I think we could just extend this to NCRing and it would work for ZZ.

Overall, I think this is more than just fixing symptoms; it is about what APIs we expect for rings to be implemented, and I think it would be fair to expect is_finite to be implemented for "most" rings. Of course it is possible to make rings where it may be hard or basically impossible to decide finiteness, but these are not usually the ones we actually compute in, I think? (Happy to learn about counter examples, though).

As such I really wish we'd implement this for more rings. E.g. for a polynomial rings.

That said, we could indeed wrap the is_(in)finite checks into a try/catch, and then if there is an exception just treat the ring as if it was infinite.

[fingolfin]

(But to be clear: this is definitely a bug and we definitely should fix it, and I am glad it got reported, thank you !!!)