From 63b4a0594f907a03fa9074c858d8ccd09c9e6b64 Mon Sep 17 00:00:00 2001 From: kalmar Date: Mon, 10 Jul 2017 19:29:53 +0200 Subject: [PATCH 1/7] Revert "add mul! and addeq! for MatrixSpace construction" This reverts commit 5fb131c39009003acdccc755cb62fa968cb5a93f. --- src/GroupRings.jl | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/src/GroupRings.jl b/src/GroupRings.jl index 20703e7..c5573f2 100644 --- a/src/GroupRings.jl +++ b/src/GroupRings.jl @@ -1,7 +1,7 @@ module GroupRings using Nemo -import Nemo: Group, GroupElem, Ring, RingElem, parent, elem_type, parent_type, mul!, addeq! +import Nemo: Group, GroupElem, Ring, RingElem, parent, elem_type, parent_type import Base: convert, show, hash, ==, +, -, *, //, /, length, norm, rationalize, deepcopy_internal, getindex, setindex!, eltype, one, zero @@ -285,13 +285,6 @@ end # ############################################################################### -function addeq!{T}(X::GroupRingElem{T}, Y::GroupRingElem{T}) - parent(X) == parent(Y) || throw(ArgumentError( - "Elements don't seem to belong to the same Group Ring!")) - X.coeffs .+= Y.coeffs - return X -end - function add{T<:Number}(X::GroupRingElem{T}, Y::GroupRingElem{T}) parent(X) == parent(Y) || throw(ArgumentError( "Elements don't seem to belong to the same Group Ring!")) @@ -309,7 +302,7 @@ end (+)(X::GroupRingElem, Y::GroupRingElem) = add(X,Y) (-)(X::GroupRingElem, Y::GroupRingElem) = add(X,-Y) -function mul!{T}(X::AbstractVector{T}, Y::AbstractVector{T}, +function mul!{T<:Number}(X::AbstractVector{T}, Y::AbstractVector{T}, pm::Array{Int,2}, result::AbstractVector{T}) for (j,y) in enumerate(Y) if y != zero(eltype(Y)) @@ -323,11 +316,6 @@ function mul!{T}(X::AbstractVector{T}, Y::AbstractVector{T}, end end -function mul!(result::GroupRingElem, X::GroupRingElem, Y::GroupRingElem) - mul!(X.coeffs, Y.coeffs, parent(X).pm, result.coeffs) - return result -end - function mul{T<:Number}(X::AbstractVector{T}, Y::AbstractVector{T}, pm::Array{Int,2}) result = zeros(X) @@ -337,8 +325,8 @@ end function mul(X::AbstractVector, Y::AbstractVector, pm::Array{Int,2}) T = promote_type(eltype(X), eltype(Y)) - result = zeros(T, X) - mul!(Vector{T}(X), Vector{T}(Y), pm, result) + result = zeros(T, deepcopy(X)) + mul!(X, Y, pm, result) return result end From 9f6287382d3bb56cd13894bbdc7b83b2f24b7598 Mon Sep 17 00:00:00 2001 From: kalmar Date: Tue, 11 Jul 2017 18:44:08 +0200 Subject: [PATCH 2/7] small tweaks to printing --- src/GroupRings.jl | 4 ++-- test/runtests.jl | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/GroupRings.jl b/src/GroupRings.jl index c5573f2..b863695 100644 --- a/src/GroupRings.jl +++ b/src/GroupRings.jl @@ -208,9 +208,9 @@ function show(io::IO, X::GroupRingElem) elseif isdefined(RG, :basis) non_zeros = ((X.coeffs[i], RG.basis[i]) for i in findn(X.coeffs)) elts = ("$(sign(c)> 0? " + ": " - ")$(abs(c))*$g" for (c,g) in non_zeros) - str = join(elts, "") + str = join(elts, "")[2:end] if sign(first(non_zeros)[1]) > 0 - str = str[4:end] + str = str[3:end] end print(io, str) else diff --git a/test/runtests.jl b/test/runtests.jl index b40cf0b..924efac 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -77,7 +77,8 @@ using Nemo @test a[5] == 1 @test a[p] == 1 - @test string(a) == " + 1*[2, 3, 1]" + @test string(a) == "1*[2, 3, 1]" + @test string(-a) == "- 1*[2, 3, 1]" @test RG([0,0,0,0,1,0]) == a @@ -89,7 +90,8 @@ using Nemo @test a[1] == 2 @test a[s] == 2 - @test string(a) == " + 2*[1, 2, 3] + 1*[2, 3, 1]" + @test string(a) == "2*[1, 2, 3] + 1*[2, 3, 1]" + @test string(-a) == "- 2*[1, 2, 3] - 1*[2, 3, 1]" @test length(a) == 2 end From 8a3a4c70d62f0dcd7a752300e0b4d20201481d63 Mon Sep 17 00:00:00 2001 From: kalmar Date: Tue, 11 Jul 2017 18:28:30 +0200 Subject: [PATCH 3/7] critical fix: if g was not in basis setindex! failed silently --- src/GroupRings.jl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/GroupRings.jl b/src/GroupRings.jl index b863695..2487e3c 100644 --- a/src/GroupRings.jl +++ b/src/GroupRings.jl @@ -180,9 +180,8 @@ function setindex!(X::GroupRingElem, value, g::GroupElem) typeof(g) == elem_type(RG.group) || throw("$g is not an element of $(RG.group)") if !(g in keys(RG.basis_dict)) g = (RG.group)(g) - else - X.coeffs[RG.basis_dict[g]] = value end + X.coeffs[RG.basis_dict[g]] = value end eltype(X::GroupRingElem) = eltype(X.coeffs) From 90106359e4535b3ccede768436a9419d0eec9161 Mon Sep 17 00:00:00 2001 From: kalmar Date: Tue, 11 Jul 2017 18:43:37 +0200 Subject: [PATCH 4/7] add tests for the silent failure --- test/runtests.jl | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/runtests.jl b/test/runtests.jl index 924efac..cf2da17 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -53,6 +53,12 @@ using Nemo B = GroupRing(F, basis, d, pm) @test A == B + g = B() + s = S[2] + g[s] = 1 + @test g == B(s) + @test g[s^2] == 0 + @test_throws KeyError g[s^10] end @testset "GroupRingElems constructors/basic manipulation" begin From 6ac5800379c1084f5f0fa6783010a55b972ca751 Mon Sep 17 00:00:00 2001 From: kalmar Date: Wed, 12 Jul 2017 20:53:17 +0200 Subject: [PATCH 5/7] allocation-free multiplication! # Conflicts: # src/GroupRings.jl --- src/GroupRings.jl | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/GroupRings.jl b/src/GroupRings.jl index 2487e3c..92bdc19 100644 --- a/src/GroupRings.jl +++ b/src/GroupRings.jl @@ -301,14 +301,15 @@ end (+)(X::GroupRingElem, Y::GroupRingElem) = add(X,Y) (-)(X::GroupRingElem, Y::GroupRingElem) = add(X,-Y) -function mul!{T<:Number}(X::AbstractVector{T}, Y::AbstractVector{T}, - pm::Array{Int,2}, result::AbstractVector{T}) - for (j,y) in enumerate(Y) - if y != zero(eltype(Y)) - for (i, index) in enumerate(pm[:,j]) - if X[i] != zero(eltype(X)) - index == 0 && throw(ArgumentError("The product don't seem to belong to the span of basis!")) - result[index] += X[i]*y +function mul!{T}(result::AbstractVector{T}, X::AbstractVector, Y::AbstractVector, pm::Array{Int,2}) + z = zero(T) + result .= z + for j in eachindex(Y) + if Y[j] != z + for i in 1:size(pm,1) + if X[i] != z + pm[i,j] == 0 && throw(ArgumentError("The product don't seem to be supported on basis!")) + result[pm[i,j]] += X[i]*Y[j] end end end From 1862868a357950e86fe93d776aeb35494441e99b Mon Sep 17 00:00:00 2001 From: kalmar Date: Mon, 24 Jul 2017 22:50:52 +0200 Subject: [PATCH 6/7] more verbose throws in divexact --- src/GroupRings.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/GroupRings.jl b/src/GroupRings.jl index bf3e173..45bae20 100644 --- a/src/GroupRings.jl +++ b/src/GroupRings.jl @@ -432,11 +432,11 @@ end function divexact{T}(X::GroupRingElem{T}, Y::GroupRingElem{T}) if length(Y) != 1 - throw("Can not divide by a non-primitive element $(Y)!") + throw("Can not divide by a non-primitive element: $(Y)!") else idx = findfirst(Y) c = Y[idx] - c == 0 || throw("Can not invert") + c != 0 || throw("Can not invert: $c not found in $Y") g = parent(Y).basis[idx] return X*1//c*parent(Y)(inv(g)) end From 7188433472292f32034f33ded9532605b2801586 Mon Sep 17 00:00:00 2001 From: kalmar Date: Tue, 25 Jul 2017 00:01:23 +0200 Subject: [PATCH 7/7] fix: remove remains of the old version --- src/GroupRings.jl | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/GroupRings.jl b/src/GroupRings.jl index 45bae20..f2246df 100644 --- a/src/GroupRings.jl +++ b/src/GroupRings.jl @@ -413,9 +413,6 @@ function *{T<:Number, S<:Number}(X::GroupRingElem{T}, Y::GroupRingElem{S}, check TT = typeof(first(X.coeffs)*first(Y.coeffs)) warn("Multiplying elements with different base rings! Promoting the result to $TT.") - result = mul!(result, X, Y) - return result - if isdefined(parent(X), :basis) result = parent(X)(similar(X.coeffs)) result = convert(TT, result)