From 5d974b19bb32231db309caba9042bab8003b93a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Wed, 23 Oct 2024 11:12:54 +0200 Subject: [PATCH 1/9] Make `scale_row!` follow its docstring i.e. don't throw on zero scalars, and coerce scalars if needed --- src/Sparse/Row.jl | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/Sparse/Row.jl b/src/Sparse/Row.jl index 5745f3be27..94f9d859e3 100644 --- a/src/Sparse/Row.jl +++ b/src/Sparse/Row.jl @@ -447,6 +447,7 @@ end # Inplace scaling # ################################################################################ + @doc raw""" scale_row!(a::SRow, b::NCRingElem) -> SRow @@ -454,8 +455,9 @@ Returns the (left) product of $b \times a$ and reassigns the value of $a$ to thi For rows, the standard multiplication is from the left. """ function scale_row!(a::SRow{T}, b::T) where T - @assert !iszero(b) - if isone(b) + if iszero(b) + return empty!(a) + elseif isone(b) return a end i = 1 @@ -465,20 +467,23 @@ function scale_row!(a::SRow{T}, b::T) where T deleteat!(a.values, i) deleteat!(a.pos, i) else - i += 1 + i += 1 end end return a end +scale_row!(a::SRow, b) = scale_row!(a, base_ring(a)(b)) + @doc raw""" scale_row_right!(a::SRow, b::NCRingElem) -> SRow Returns the (right) product of $a \times b$ and modifies $a$ to this product. """ function scale_row_right!(a::SRow{T}, b::T) where T - @assert !iszero(b) - if isone(b) + if iszero(b) + return empty!(a) + elseif isone(b) return a end i = 1 @@ -488,16 +493,20 @@ function scale_row_right!(a::SRow{T}, b::T) where T deleteat!(a.values, i) deleteat!(a.pos, i) else - i += 1 + i += 1 end end return a end +scale_row_right!(a::SRow, b) = scale_row_right!(a, base_ring(a)(b)) + function scale_row_left!(a::SRow{T}, b::T) where T return scale_row!(a,b) end +scale_row_left!(a::SRow, b) = scale_row_left!(a, base_ring(a)(b)) + ################################################################################ # # Addition From 1c06d6e93878235362a785ed630dba44323020ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Wed, 23 Oct 2024 11:18:17 +0200 Subject: [PATCH 2/9] Fix a docstring typo --- src/Sparse/Row.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sparse/Row.jl b/src/Sparse/Row.jl index 94f9d859e3..a9d35732bb 100644 --- a/src/Sparse/Row.jl +++ b/src/Sparse/Row.jl @@ -752,7 +752,7 @@ add_left_scaled_row!(a::SRow{T}, b::SRow{T}, c::T) where T = add_scaled_row!(a, @doc raw""" add_right_scaled_row!(A::SRow{T}, B::SRow{T}, c::T) -> SRow{T} -Return the right scaled row $c A$ to $B$ by changing $B$ in place. +Return the right scaled row $A c$ to $B$ by changing $B$ in place. """ add_right_scaled_row!(a::SRow{T}, b::SRow{T}, c::T) where T = add_scaled_row!(a, b, c, Val(false)) From 67da91f5fbdd1b70e70c4cd4d01c03b1256746b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Wed, 23 Oct 2024 11:18:56 +0200 Subject: [PATCH 3/9] Let `add_scaled_row` coerce the scalar --- src/Sparse/Row.jl | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/Sparse/Row.jl b/src/Sparse/Row.jl index a9d35732bb..c192d6c53a 100644 --- a/src/Sparse/Row.jl +++ b/src/Sparse/Row.jl @@ -692,10 +692,10 @@ end Returns the row $c A + B$. """ -add_scaled_row(a::SRow{T}, b::SRow{T}, c::T) where {T} = add_scaled_row!(a, deepcopy(b), c) +add_scaled_row(a::SRow{T}, b::SRow{T}, c) where {T} = add_scaled_row!(a, deepcopy(b), c) -add_left_scaled_row(a::SRow{T}, b::SRow{T}, c::T) where {T} = add_left_scaled_row!(a, deepcopy(b), c) -add_right_scaled_row(a::SRow{T}, b::SRow{T}, c::T) where {T} = add_right_scaled_row!(a, deepcopy(b), c) +add_left_scaled_row(a::SRow{T}, b::SRow{T}, c) where {T} = add_left_scaled_row!(a, deepcopy(b), c) +add_right_scaled_row(a::SRow{T}, b::SRow{T}, c) where {T} = add_right_scaled_row!(a, deepcopy(b), c) @@ -705,7 +705,9 @@ add_right_scaled_row(a::SRow{T}, b::SRow{T}, c::T) where {T} = add_right_scaled_ Adds the left scaled row $c A$ to $B$. """ function add_scaled_row!(a::SRow{T}, b::SRow{T}, c::T, ::Val{left_side} = Val(true)) where {T, left_side} - @assert a !== b + if a === b + a = deepcopy(a) + end i = 1 j = 1 t = base_ring(a)() @@ -744,17 +746,21 @@ function add_scaled_row!(a::SRow{T}, b::SRow{T}, c::T, ::Val{left_side} = Val(tr return b end +add_scaled_row!(a::SRow{T}, b::SRow{T}, c) where {T} = add_scaled_row!(a, b, base_ring(a)(c)) + +add_scaled_row!(a::SRow{T}, b::SRow{T}, c, side::Val) where {T} = add_scaled_row!(a, b, base_ring(a)(c), side) + # ignore tmp argument -add_scaled_row!(a::SRow{T}, b::SRow{T}, c::T, tmp::SRow{T}) where T = add_scaled_row!(a, b, c) +add_scaled_row!(a::SRow{T}, b::SRow{T}, c, tmp::SRow{T}) where T = add_scaled_row!(a, b, c) -add_left_scaled_row!(a::SRow{T}, b::SRow{T}, c::T) where T = add_scaled_row!(a, b, c) +add_left_scaled_row!(a::SRow{T}, b::SRow{T}, c) where T = add_scaled_row!(a, b, c) @doc raw""" add_right_scaled_row!(A::SRow{T}, B::SRow{T}, c::T) -> SRow{T} Return the right scaled row $A c$ to $B$ by changing $B$ in place. """ -add_right_scaled_row!(a::SRow{T}, b::SRow{T}, c::T) where T = add_scaled_row!(a, b, c, Val(false)) +add_right_scaled_row!(a::SRow{T}, b::SRow{T}, c) where T = add_scaled_row!(a, b, c, Val(false)) ################################################################################ From ea21c55c977f31734257a95b809e9bd40b28fe92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Wed, 23 Oct 2024 11:19:20 +0200 Subject: [PATCH 4/9] Add mutating arithmetics for SRow --- src/Sparse/Row.jl | 93 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 91 insertions(+), 2 deletions(-) diff --git a/src/Sparse/Row.jl b/src/Sparse/Row.jl index c192d6c53a..b4e08c7161 100644 --- a/src/Sparse/Row.jl +++ b/src/Sparse/Row.jl @@ -527,10 +527,10 @@ function -(A::SRow{T}, B::SRow{T}) where T if length(B) == 0 return A else - return add_scaled_row(B, A, base_ring(B)(-1)) + return add_scaled_row(B, A, -1) end end - return add_scaled_row(B, A, base_ring(A)(-1)) + return add_scaled_row(B, A, -1) end function -(A::SRow{T}) where {T} @@ -763,6 +763,95 @@ Return the right scaled row $A c$ to $B$ by changing $B$ in place. add_right_scaled_row!(a::SRow{T}, b::SRow{T}, c) where T = add_scaled_row!(a, b, c, Val(false)) +################################################################################ +# +# Mutating arithmetics +# +################################################################################ + +function zero!(z::SRow) + return empty!(z) +end + +function neg!(z::SRow{T}, x::SRow{T}) where T + if z === x + return neg!(x) + end + swap!(z, -x) + return z +end + +function neg!(z::SRow) + for i in 1:length(z) + z.values[i] = neg!(z.values[i]) + end + return z +end + +function add!(z::SRow{T}, x::SRow{T}, y::SRow{T}) where T + if z === x + return add!(x, y) + elseif z === y + return add!(y, x) + end + swap!(z, x + y) + return z +end + +function add!(z::SRow{T}, x::SRow{T}) where T + if z === x + return scale_row!(z, 2) + end + return add_scaled_row!(x, z, one(base_ring(x))) +end + +function sub!(z::SRow{T}, x::SRow{T}, y::SRow{T}) where T + if z === x + return sub!(x, y) + elseif z === y + return neg!(sub!(y, x)) + end + swap!(z, x - y) + return z +end + +function sub!(z::SRow{T}, x::SRow{T}) where T + if z === x + return empty!(z) + end + return add_scaled_row!(x, z, -1) +end + +function mul!(z::SRow{T}, x::SRow{T}, c) where T + if z === x + return scale_row_right!(x, c) + end + swap!(z, x * c) + return z +end + +function mul!(z::SRow{T}, c, y::SRow{T}) where T + if z === y + return scale_row_left!(y, c) + end + swap!(z, c * y) + return z +end + +function addmul!(z::SRow{T}, x::SRow{T}, y) where T + return add_right_scaled_row!(x, z, y) +end + +function addmul!(z::SRow{T}, x, y::SRow{T}) where T + return add_left_scaled_row!(y, z, x) +end + + +# ignore temp variable +addmul!(z::SRow{T}, x::SRow{T}, y, t) where T = addmul!(z, x, y) +addmul!(z::SRow{T}, x, y::SRow{T}, t) where T = addmul!(z, x, y) + + ################################################################################ # # Lifting From 9ddf26e1284684ce40137ef232b63165c957869c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Wed, 23 Oct 2024 14:34:10 +0200 Subject: [PATCH 5/9] Skip deepcopy in addmul! in case of aliasing --- src/Sparse/Row.jl | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Sparse/Row.jl b/src/Sparse/Row.jl index b4e08c7161..0f9d5cd882 100644 --- a/src/Sparse/Row.jl +++ b/src/Sparse/Row.jl @@ -839,10 +839,16 @@ function mul!(z::SRow{T}, c, y::SRow{T}) where T end function addmul!(z::SRow{T}, x::SRow{T}, y) where T + if z === x + return scale_row_right!(x, y+1) + end return add_right_scaled_row!(x, z, y) end function addmul!(z::SRow{T}, x, y::SRow{T}) where T + if z === x + return scale_row_left!(y, x+1) + end return add_left_scaled_row!(y, z, x) end From 94b188ad6ec8dea75dc8dcca8805d35ee6323ab8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Tue, 5 Nov 2024 11:41:44 +0100 Subject: [PATCH 6/9] Add `submul!` --- src/Sparse/Row.jl | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/Sparse/Row.jl b/src/Sparse/Row.jl index 0f9d5cd882..6ace77342a 100644 --- a/src/Sparse/Row.jl +++ b/src/Sparse/Row.jl @@ -822,6 +822,10 @@ function sub!(z::SRow{T}, x::SRow{T}) where T return add_scaled_row!(x, z, -1) end +function mul!(z::SRow{T}, x::SRow{T}, y::SRow{T}) where T + error("Not implemented") +end + function mul!(z::SRow{T}, x::SRow{T}, c) where T if z === x return scale_row_right!(x, c) @@ -838,6 +842,10 @@ function mul!(z::SRow{T}, c, y::SRow{T}) where T return z end +function addmul!(z::SRow{T}, x::SRow{T}, y::SRow{T}) where T + error("Not implemented") +end + function addmul!(z::SRow{T}, x::SRow{T}, y) where T if z === x return scale_row_right!(x, y+1) @@ -852,10 +860,30 @@ function addmul!(z::SRow{T}, x, y::SRow{T}) where T return add_left_scaled_row!(y, z, x) end +function submul!(z::SRow{T}, x::SRow{T}, y::SRow{T}) where T + error("Not implemented") +end + +function submul!(z::SRow{T}, x::SRow{T}, y) where T + if z === x + return scale_row_right!(x, -y+1) + end + return add_right_scaled_row!(x, z, -y) +end + +function submul!(z::SRow{T}, x, y::SRow{T}) where T + if z === x + return scale_row_left!(y, -x+1) + end + return add_left_scaled_row!(y, z, -x) +end + # ignore temp variable addmul!(z::SRow{T}, x::SRow{T}, y, t) where T = addmul!(z, x, y) addmul!(z::SRow{T}, x, y::SRow{T}, t) where T = addmul!(z, x, y) +submul!(z::SRow{T}, x::SRow{T}, y, t) where T = submul!(z, x, y) +submul!(z::SRow{T}, x, y::SRow{T}, t) where T = submul!(z, x, y) ################################################################################ From 9975911aad83c500b322031b095c067814480ec8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Tue, 5 Nov 2024 11:57:24 +0100 Subject: [PATCH 7/9] Add tests --- src/Sparse/Row.jl | 8 ++++++++ test/Sparse/Row.jl | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/src/Sparse/Row.jl b/src/Sparse/Row.jl index 6ace77342a..447c7cc4b4 100644 --- a/src/Sparse/Row.jl +++ b/src/Sparse/Row.jl @@ -105,6 +105,14 @@ function Base.empty!(A::SRow) return A end +function Base.empty(A::SRow) + return sparse_row(base_ring(A)) +end + +function zero(A::SRow) + return empty(A) +end + function swap!(A::SRow, B::SRow) A.pos, B.pos = B.pos, A.pos A.values, B.values = B.values, A.values diff --git a/test/Sparse/Row.jl b/test/Sparse/Row.jl index 5b1e10f9be..af3edfa611 100644 --- a/test/Sparse/Row.jl +++ b/test/Sparse/Row.jl @@ -204,4 +204,40 @@ B = sparse_row(F,[1],[y]) C = add_scaled_row(A,B,F(1)) @test C == A+B + + # mutating arithmetic + randcoeff() = begin + n = rand((1,1,1,2,5,7,15)) + return rand(-2^n:2^n) + end + Main.equality(A::SRow, B::SRow) = A == B + @testset "mutating arithmetic; R = $R" for R in (ZZ, QQ) + for _ in 1:10 + maxind_A = rand(0:10) + inds_A = Hecke.Random.randsubseq(1:maxind_A, rand()) + vals_A = elem_type(R)[R(rand((-1, 1)) * rand(1:10)) for _ in 1:length(inds_A)] + A = sparse_row(R, inds_A, vals_A) + + maxind_B = rand(0:10) + inds_B = Hecke.Random.randsubseq(1:maxind_B, rand()) + vals_B = elem_type(R)[R(rand((-1, 1)) * rand(1:10)) for _ in 1:length(inds_B)] + B = sparse_row(R, inds_B, vals_B) + + test_mutating_op_like_zero(zero, zero!, A) + + test_mutating_op_like_neg(-, neg!, A) + + test_mutating_op_like_add(+, add!, A, B) + test_mutating_op_like_add(-, sub!, A, B) + test_mutating_op_like_mul(*, mul!, A, randcoeff(); right_factor_is_scalar=true) + test_mutating_op_like_mul(*, mul!, randcoeff(), A; left_factor_is_scalar=true) + test_mutating_op_like_mul(*, mul!, A, ZZ(randcoeff()); right_factor_is_scalar=true) + test_mutating_op_like_mul(*, mul!, ZZ(randcoeff()), A; left_factor_is_scalar=true) + + test_mutating_op_like_addmul((a, b, c) -> a + b*c, addmul!, A, B, randcoeff(); right_factor_is_scalar=true) + test_mutating_op_like_addmul((a, b, c) -> a + b*c, addmul!, A, randcoeff(), B; left_factor_is_scalar=true) + test_mutating_op_like_addmul((a, b, c) -> a - b*c, submul!, A, B, randcoeff(); right_factor_is_scalar=true) + test_mutating_op_like_addmul((a, b, c) -> a - b*c, submul!, A, randcoeff(), B; left_factor_is_scalar=true) + end + end end From 0057c1926fba1b83468db2dbe1eeee445edfa943 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Tue, 5 Nov 2024 12:27:52 +0100 Subject: [PATCH 8/9] Some fixes (tests run now) --- src/Sparse/Row.jl | 6 +++--- src/Sparse/ZZRow.jl | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Sparse/Row.jl b/src/Sparse/Row.jl index 447c7cc4b4..2b661f923c 100644 --- a/src/Sparse/Row.jl +++ b/src/Sparse/Row.jl @@ -523,9 +523,9 @@ scale_row_left!(a::SRow, b) = scale_row_left!(a, base_ring(a)(b)) function +(A::SRow{T}, B::SRow{T}) where T if length(A.values) == 0 - return B + return deepcopy(B) elseif length(B.values) == 0 - return A + return deepcopy(A) end return add_scaled_row(A, B, one(base_ring(A))) end @@ -533,7 +533,7 @@ end function -(A::SRow{T}, B::SRow{T}) where T if length(A) == 0 if length(B) == 0 - return A + return deepcopy(A) else return add_scaled_row(B, A, -1) end diff --git a/src/Sparse/ZZRow.jl b/src/Sparse/ZZRow.jl index 6e67326888..f5fe07e43f 100644 --- a/src/Sparse/ZZRow.jl +++ b/src/Sparse/ZZRow.jl @@ -276,7 +276,6 @@ end function add_scaled_row(Ai::SRow{ZZRingElem}, Aj::SRow{ZZRingElem}, c::ZZRingElem, sr::SRow{ZZRingElem} = sparse_row(ZZ)) empty!(sr) - @assert c != 0 n = ZZRingElem() pi = 1 pj = 1 @@ -323,6 +322,9 @@ function add_scaled_row(Ai::SRow{ZZRingElem}, Aj::SRow{ZZRingElem}, c::ZZRingEle end function add_scaled_row!(Ai::SRow{ZZRingElem}, Aj::SRow{ZZRingElem}, c::ZZRingElem, sr::SRow{ZZRingElem} = sparse_row(ZZ)) + if iszero(c) + return Aj + end _t = sr sr = add_scaled_row(Ai, Aj, c, sr) @assert _t === sr From c5fb659a2b323c00a9e84489d791c2f93682d369 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Tue, 5 Nov 2024 12:28:05 +0100 Subject: [PATCH 9/9] Comment out tests --- test/Sparse/Row.jl | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/Sparse/Row.jl b/test/Sparse/Row.jl index af3edfa611..939be843df 100644 --- a/test/Sparse/Row.jl +++ b/test/Sparse/Row.jl @@ -229,15 +229,15 @@ test_mutating_op_like_add(+, add!, A, B) test_mutating_op_like_add(-, sub!, A, B) - test_mutating_op_like_mul(*, mul!, A, randcoeff(); right_factor_is_scalar=true) - test_mutating_op_like_mul(*, mul!, randcoeff(), A; left_factor_is_scalar=true) - test_mutating_op_like_mul(*, mul!, A, ZZ(randcoeff()); right_factor_is_scalar=true) - test_mutating_op_like_mul(*, mul!, ZZ(randcoeff()), A; left_factor_is_scalar=true) - - test_mutating_op_like_addmul((a, b, c) -> a + b*c, addmul!, A, B, randcoeff(); right_factor_is_scalar=true) - test_mutating_op_like_addmul((a, b, c) -> a + b*c, addmul!, A, randcoeff(), B; left_factor_is_scalar=true) - test_mutating_op_like_addmul((a, b, c) -> a - b*c, submul!, A, B, randcoeff(); right_factor_is_scalar=true) - test_mutating_op_like_addmul((a, b, c) -> a - b*c, submul!, A, randcoeff(), B; left_factor_is_scalar=true) + # test_mutating_op_like_mul(*, mul!, A, randcoeff(); right_factor_is_scalar=true) + # test_mutating_op_like_mul(*, mul!, randcoeff(), A; left_factor_is_scalar=true) + # test_mutating_op_like_mul(*, mul!, A, ZZ(randcoeff()); right_factor_is_scalar=true) + # test_mutating_op_like_mul(*, mul!, ZZ(randcoeff()), A; left_factor_is_scalar=true) + + # test_mutating_op_like_addmul((a, b, c) -> a + b*c, addmul!, A, B, randcoeff(); right_factor_is_scalar=true) + # test_mutating_op_like_addmul((a, b, c) -> a + b*c, addmul!, A, randcoeff(), B; left_factor_is_scalar=true) + # test_mutating_op_like_addmul((a, b, c) -> a - b*c, submul!, A, B, randcoeff(); right_factor_is_scalar=true) + # test_mutating_op_like_addmul((a, b, c) -> a - b*c, submul!, A, randcoeff(), B; left_factor_is_scalar=true) end end end