Enable matrix algebras over noncommutative rings

[wbhart]

Fixes #1035

[thofma]

Love it!

[codecov]

Codecov Report

Merging #1127 (03e17e7) into master (c0bb612) will increase coverage by 0.51%.
The diff coverage is 92.17%.

@@            Coverage Diff             @@
##           master    #1127      +/-   ##
==========================================
+ Coverage   84.95%   85.47%   +0.51%     
==========================================
  Files          95       97       +2     
  Lines       26266    27702    +1436     
==========================================
+ Hits        22315    23677    +1362     
- Misses       3951     4025      +74     

Impacted Files	Coverage Δ
src/NCPoly.jl	89.14% <0.00%> (-3.26%)	⬇️
src/Rings.jl	46.29% <ø> (-19.33%)	⬇️
src/generic/GenericTypes.jl	94.05% <77.77%> (+1.02%)	⬆️
src/generic/Matrix.jl	97.08% <90.00%> (ø)
src/Matrix.jl	91.74% <97.07%> (-0.49%)	⬇️
src/NCRings.jl	88.65% <97.22%> (+11.46%)	⬆️
src/MatrixAlgebra.jl	93.20% <100.00%> (+0.24%)	⬆️
src/generic/MatrixAlgebra.jl	100.00% <100.00%> (ø)
src/generic/LaurentPoly.jl	96.17% <0.00%> (-2.90%)	⬇️
... and 20 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c0bb612...03e17e7. Read the comment docs.

[wbhart]

It looks like this is introducing ambiguities for map_entries involving GAP objects. I wonder if @fingolfin could comment on this when he has a chance.

https://github.com/Nemocas/AbstractAlgebra.jl/runs/5205706833?check_suite_focus=true

Presumably the relevant method in AbstractAlgebra is:

map_entries(f, a::MatrixElem{T}) where T <: RingElement

which was formerly not restricted to T <: RingElement. As this has introduced an ambiguity rather than a method error, I may need some guidance on whether this is something that can easily be worked around in Oscar.jl or whether we need to come up with something new here.

[thofma]

I think Oscar would need to do MatrixElem instead of MatElem (in the signature).

[wbhart]

Thanks @thofma

If that seems feasible then I'll assume that for now.

[wbhart]

Do we also want MatrixSpace over noncommutative ring or just MatrixAlgebra over noncommutative ring?

I don't foresee any complication with having both (other than writing a hell of a lot of test code), but maybe you guys see something?

(Unrelated to discussion above, I think.)

[thofma]

At the moment my only application is MatrixAlgebra, but if it is not too much work for MatrixSpaces, I would of course want them too!

[wbhart]

It's just test code, as the tests are separated. It's easier to actually have matrix spaces as well from a code point of view.

I will do both.

[fingolfin]

Just to clarify: GAP.jl knows nothing about AA, MatrixElem etc., and it should stay that way. Any necessary changes will have to be done in Oscar.

[wbhart]

@fingolfin I don't see any issue with that. @thofma appears to have confirmed above that this is in fact something that can be changed in Oscar. I was guessing on the basis of the types alone.

[wbhart]

I've already encountered an issue which is going to be a major problem:

julia> [[1 2; 3 4] [2 3; 4 5]; [3 4; 5 6] [4 5; 6 7]]

4×4 Matrix{Int64}:
 1  2  2  3
 3  4  4  5
 3  4  4  5
 5  6  6  7

It seems Julia itself doesn't support matrices of matrices the way we would expect.

[wbhart]

Even worse, this carries over to our types:

julia> R = MatrixAlgebra(ZZ, 2)
Matrix Algebra of degree 2 over Integers

julia> nrows([R([1 2; 3 4]) R([2 3; 3 4]); R([3 4; 5 6]) R([4 5; 6 7])])
4

Which means that it is currently impossible to construct matrices over matrix algebras.

[thofma]

I don't expect too much from julia matrices, so this is fine.

The other issue you mention is a consequence of us allowing the syntax

[A B;
 C D]

to construct block matrices. I don't know what the best way would be to fix it. I think we should keep the block matrix syntax for convenience. For the tests you could just do matrix(S, 2, 2, [A, B, C, D]).

[wbhart]

Yes, I can do that. It works at least.

[thofma]

The Oscar problem is fixed.

Somehow the documentation needs to be adjusted.

[wbhart]

Great! What changes do you want to the docs?

[thofma]

If think it would be enough if they would build :)

[wbhart]

Oh I see. This PR has a long way to go. Docs will be fixed in the end. I haven't touched them yet.

[wbhart]

There will be some interesting holes in the implementation. For example isunit for a matrix algebra currently relies on det. None of the det methods we currently have work for noncommutative rings and I don't know if it is a theorem that a matrix is invertible if its determinant is a unit in the case of a noncommutative ring.

[wbhart]

Also, I don't see how to test this function:

block_diagonal_matrix(R::NCRing, V::Vector{<:Matrix{T}}) where T <: NCRingElement

It requires a Julia Vector or Julia matrices over a noncommutative ring to test, and as we know, Julia flattens these.

[thofma]

Yes, there are multiple definitions of determinant, so there should be no det function in the general case.

[thofma]

Also, I don't see how to test this function:

block_diagonal_matrix(R::NCRing, V::Vector{<:Matrix{T}}) where T <: NCRingElement

It requires a Julia Vector or Julia matrices over a noncommutative ring to test, and as we know, Julia flattens these.

No, I don't think julia flattens them:

julia> a = ZZ[1 2; 3 4]
[1   2]
[3   4]

julia> [a, a, a]
3-element Vector{fmpz_mat}:
 [1 2; 3 4]
 [1 2; 3 4]
 [1 2; 3 4]

[wbhart]

I was hoping to test it when T is a noncommutative type, i.e. type of a matrix algebra element.

[wbhart]

julia>    R = MatrixAlgebra(ZZ, 2)
Matrix Algebra of degree 2 over Integers

julia>    S = MatrixAlgebra(R, 2)
Matrix Algebra of degree 2 over Matrix Algebra of degree 2 over Integers

julia>    A = [1 2; 3 4]
2×2 Matrix{Int64}:
 1  2
 3  4

julia>    B = [2 3; 4 5]
2×2 Matrix{Int64}:
 2  3
 4  5

julia>    C = [3 4; 5 6]
2×2 Matrix{Int64}:
 3  4
 5  6

julia>    D = [4 5; 6 7]
2×2 Matrix{Int64}:
 4  5
 6  7

julia>    RA = R(A)
[1   2]
[3   4]

julia>    RB = R(B)
[2   3]
[4   5]

julia>    RC = R(C)
[3   4]
[5   6]

julia>    RD = R(D)
[4   5]
[6   7]

julia> Generic.block_diagonal_matrix(S, [[RA RB; RC RD], [RB RC; RA RB]])
ERROR: MethodError: no method matching block_diagonal_matrix(::AbstractAlgebra.Generic.MatAlgebra{AbstractAlgebra.Generic.MatAlgElem{BigInt}}, ::Vector{AbstractAlgebra.Generic.MatAlgElem{BigInt}})
Closest candidates are:
  block_diagonal_matrix(::AbstractAlgebra.NCRing, ::Vector{var"#s159"} where var"#s159"<:Matrix{T}) where T<:Union{NCRingElem, RingElement} at /home/wbhart/.julia/dev/AbstractAlgebra/src/Matrix.jl:309
Stacktrace:
 [1] top-level scope
   @ REPL[64]:1

[thofma]

Ah, my mistake. I misread the signature. Don't think there is an easier way than doing M = Matrix(S, undef, 2, 2); M[1, :] = [RA, RB]; M[2, :] = [RC, RD] to produce some Matrix with these matrices as entries.

[wbhart]

@thofma I've added all the basic methods and test for them. We are up to the more complicated algorithms now.

However, I don't know how we are going to fix the promotion system. Consider multiplying a ResF{BigInt} by a MatAlgElem{AbstractAlgebra.Generic.Poly{AbstractAlgebra.Generic.ResF{BigInt}}}. We currently have a test which does this, but it fails with an ambiguity error.

Now that I've added promotion rules for promoting MatAlgElems as noncommutative ring elements it doesn't know which way to do the promotion. It sees an NCRingElement on the left and a MatAlgElem on the right. Which is it promoting to which?

I don't think this is an easy problem to solve.

[wbhart]

Actually, I managed to fix that and another promotion issue just by adding more promotion rules. But here is one I don't know how to deal with:

promote(::AbstractAlgebra.Generic.MatAlgElem{BigInt}, ::AbstractAlgebra.Generic.NCPoly{AbstractAlgebra.Generic.NCPoly{AbstractAlgebra.Generic.MatAlgElem{BigInt}}})

[wbhart]

I've removed the WIP and rebased this. If it is still desirable to have this branch, it would need the ambiguity issue to be resolved. Otherwise it's ready from my point of view.

[tthsqe12]

Some strange method errir in the oscar ci. Will investigate

[thofma]

Fix should be similar to oscar-system/Oscar.jl#1102

Edit: Should be fixed by thofma/Hecke.jl#628

[tthsqe12]

My 2 cents: I think the transpose function should transpose the entries as well. no? This also complicates matters because base_ring(x) may not be the same as base_ring(transpose(x)). or is it?

function transpose(x::Mat{T}) where T <: NCRingElement
   y = MatSpaceElem{eltype(x)}(permutedims(x.entries))
   y.base_ring = x.base_ring
   y
end

[thofma]

Can you elaborate why the transpose should be 'recursive'? It would be a bit awkward if there were no way to just transpose the matrix generically.

[tthsqe12]

I don't think (AB)^T = B^TA^T works without recursion

[thofma]

Yes, transposition on M_n(S) will not be an anti-morphism unless one applies an anti-morphism S -> S elementwise. But this anti-morphism need not exist for S and even in the case that S is a matrix ring, why do we single out transposition? This makes it impossible to write generic code since transpose is now some awkward function doing funny/useless things depending on the base ring.

[tthsqe12]

De we want this identity? this is also interesting: oscar-system/Singular.jl#233

[thofma]

If you want that identity, just supply the anti-morphism/involution and use a different method. But guessing an involution should not be part of the transpose(M) method.

[tthsqe12]

ok, then I am fine.