Skip to content

Commit

Permalink
No type piracy: remove matrix*vector methods for Nemo types (#760)
Browse files Browse the repository at this point in the history
* No type piracy: remove matrix*vector methods for Nemo types

* Adjut tests

* Remove more problematic * methods

Multiplying FreeModuleElem by a matrix group element returning a
Vector{} is weird and shouldn't be done. While this could be fixed
by adding a second transpose call, I don't think this kind of
inefficient operation should be encouraged, that vector space
simply is a right-module, end of story.

In a similar vein, remove the three argument * method.
  • Loading branch information
fingolfin authored Oct 21, 2024
1 parent 9ef11df commit 3deb88b
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 53 deletions.
7 changes: 4 additions & 3 deletions src/Groups/matrices/forms.jl
Original file line number Diff line number Diff line change
Expand Up @@ -367,15 +367,16 @@ function (f::SesquilinearForm{T})(v::AbstractAlgebra.Generic.FreeModuleElem{T},w
@req f.descr!=:quadratic "Quadratic forms requires only one argument"

if f.descr==:hermitian
return v*gram_matrix(f)*map( y->frobenius(y,div(degree(base_ring(w)),2)),w)
w = conjugate_transpose(w.v)
else
return v*gram_matrix(f)*w
w = transpose(w.v)
end
return v.v*gram_matrix(f)*w
end

function (f::SesquilinearForm{T})(v::AbstractAlgebra.Generic.FreeModuleElem{T}) where T <: RingElem
@req f.descr==:quadratic "Sesquilinear forms requires two arguments"
return v*gram_matrix(f)*v
return v.v*gram_matrix(f)*transpose(v.v)
end


Expand Down
15 changes: 0 additions & 15 deletions src/Groups/matrices/matrix_manipulation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -170,17 +170,7 @@ end
#
########################################################################


Base.:*(v::AbstractAlgebra.Generic.FreeModuleElem{T},x::MatElem{T}) where T <: RingElem = v.parent(v.v*x)
Base.:*(x::MatElem{T},u::AbstractAlgebra.Generic.FreeModuleElem{T}) where T <: RingElem = x*transpose(u.v)

# evaluation of the form x into the vectors v and u
Base.:*(v::AbstractAlgebra.Generic.FreeModuleElem{T},x::MatElem{T},u::AbstractAlgebra.Generic.FreeModuleElem{T}) where T <: RingElem = (v.v*x*transpose(u.v))[1]


Base.:*(v::AbstractAlgebra.Generic.FreeModuleElem{T},x::MatrixGroupElem{T}) where T <: RingElem = v.parent(v.v*matrix(x))
Base.:*(x::MatrixGroupElem{T},u::AbstractAlgebra.Generic.FreeModuleElem{T}) where T <: RingElem = matrix(x)*transpose(u.v)


Base.:*(v::Vector{T}, x::MatrixGroupElem{T}) where T <: RingElem = v*matrix(x)
Base.:*(x::MatrixGroupElem{T}, u::Vector{T}) where T <: RingElem = matrix(x)*u
Expand All @@ -193,8 +183,3 @@ Base.:^(v::AbstractAlgebra.Generic.FreeModuleElem{T},x::MatrixGroupElem{T}) wher
function Base.:^(V::AbstractAlgebra.Generic.Submodule{T}, x::MatrixGroupElem{T}) where T <: RingElem
return sub(V.m, [v^x for v in V.gens])[1]
end

# evaluation of the form x into the vectors v and u
Base.:*(v::AbstractAlgebra.Generic.FreeModuleElem{T},x::MatrixGroupElem{T},u::AbstractAlgebra.Generic.FreeModuleElem{T}) where T <: RingElem = (v.v*matrix(x)*transpose(u.v))[1]

map(f::Function, v::AbstractAlgebra.Generic.FreeModuleElem{T}) where T <: RingElem = v.parent(map(f,v.v))
2 changes: 1 addition & 1 deletion test/Groups/conformance.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import Oscar.AbstractAlgebra: Group

include(joinpath(dirname(pathof(AbstractAlgebra)), "..", "test", "Groups-conformance-tests.jl"))

@testset "GAPGroups_interface_conformance for $(G)" for G in L
@testset "GAPGroups_interface_conformance $G of type $(typeof(G))" for G in L

test_Group_interface(G)
test_GroupElem_interface(rand(G, 2)...)
Expand Down
60 changes: 26 additions & 34 deletions test/Groups/operations.jl
Original file line number Diff line number Diff line change
Expand Up @@ -50,49 +50,50 @@
end
end

@testset "Matrix manipulation" begin
F = GF(5, 1)

I = identity_matrix(F,6)
x = matrix(F,2,2,[2,3,4,0])
I1 = deepcopy(I)
I1[3:4,3:4] = x
@test I==identity_matrix(F,6)
I[3:4,3:4] = x
@test I==I1
@test I[3:4,3:4]==x
@testset "Matrix manipulation" for F in (GF(5), GF(5,1))

V = vector_space(F,6)
@test matrix([V[i] for i in 1:6])==identity_matrix(F,6)
I = identity_matrix(F,6)

# test converting a list of vector space elements into a matrix
@test matrix([V[i] for i in 1:6]) == I

# permutation matrix tests
L = [1,4,6,2,3,5]
P = permutation_matrix(F,L)
@testset for i in 1:6
@test V[i]*P == V[L[i]]
end
p = symmetric_group(6)(L)
@test permutation_matrix(F,p)==permutation_matrix(F,L)
@test upper_triangular_matrix(F.([2,3,1,1,0,1]))==matrix(F,3,3,[2,3,1,0,1,0,0,0,1])
@test lower_triangular_matrix(F.([2,3,1,1,0,1]))==matrix(F,3,3,[2,0,0,3,1,0,1,0,1])
@test permutation_matrix(F,p) == permutation_matrix(F,L)

# upper + lower triangular matrix constructor
@test upper_triangular_matrix(F.([1,2,3,4,5,6])) == matrix(F, [1 2 3; 0 4 5; 0 0 6])
@test lower_triangular_matrix(F.([2,3,1,1,0,1])) == matrix(F, [2 0 0; 3 1 0; 1 0 1])
@test_throws ArgumentError lower_triangular_matrix(F.([2,3,1,1]))

R,t=polynomial_ring(F,:t)
f = t^4+2*t^3+4*t+1
@test f(identity_matrix(F,6))==f(1)*identity_matrix(F,6)
@test_throws ArgumentError conjugate_transpose(x)
@test f(I)==f(1)*I

@test_throws ArgumentError conjugate_transpose(I)

P = permutation_matrix(F,L)
@test is_symmetric(P+transpose(P))
@test is_skew_symmetric(P-transpose(P))
@test is_alternating(P-transpose(P))

F,z = finite_field(2,2)
x=matrix(F,4,4,[1,z,0,0,0,1,z^2,z,z,0,0,1,0,0,z+1,0])
x=matrix(F,[1 z 0 0; 0 1 z^2 z; z 0 0 1; 0 0 z+1 0])
y=x+transpose(x)
@test is_symmetric(y)
@test is_hermitian(x+conjugate_transpose(x))
@test is_skew_symmetric(y)
@test is_alternating(y)
@test conjugate_transpose(conjugate_transpose(x)) == x

y[1,1]=1
@test is_symmetric(y)
@test is_hermitian(x+conjugate_transpose(x))
@test is_skew_symmetric(y)
@test !is_alternating(y)
@test conjugate_transpose(x)==transpose(matrix(F,4,4,[1,z+1,0,0,0,1,z,z+1,z+1,0,0,1,0,0,z,0]))
@test conjugate_transpose(x)==transpose(matrix(F,[1 z+1 0 0; 0 1 z z+1; z+1 0 0 1; 0 0 z 0]))

end

Expand All @@ -109,19 +110,10 @@ end
@test complement(V,W0)[1]==V
@test complement(V,sub(V,gens(V))[1])[1]==W0

v1=V([1,2,3,4,5])
v2=V([1,6,0,5,2])
G = GL(5,F)
B=matrix(F,5,5,[1,2,3,1,0,4,5,2,0,1,3,2,5,4,0,1,6,4,3,5,2,0,4,1,1])
B = rand(G)
v1 = rand(V)
@test v1*B == V([ sum([v1[i]*B[i,j] for i in 1:5]) for j in 1:5 ])
@test V(transpose(B*v2))==V([ sum([v2[i]*B[j,i] for i in 1:5]) for j in 1:5 ])
B = G(B)
@test v1*B == V([ sum([v1[i]*B[i,j] for i in 1:5]) for j in 1:5 ])
@test V(transpose(B*v2))==V([ sum([v2[i]*B[j,i] for i in 1:5]) for j in 1:5 ])
@test map(x->x+1,v1)==V([2,3,4,5,6])

# see the discussion of pull/1368 and issues/872
@test_throws MethodError v1*v2
end

# from file matrices/stuff_field_gen.jl
Expand Down

0 comments on commit 3deb88b

Please sign in to comment.