From ec273aabe7730fa45cfd421310ebcd494d3f43ea Mon Sep 17 00:00:00 2001 From: Alex Arslan Date: Mon, 5 Nov 2018 16:00:24 -0800 Subject: [PATCH 1/2] Fix use of norm and remove vecnorm The behavior of `norm(x, p)` for matrices changed in Julia PR 27401 to be the same as `norm(vec(x), p)` and `vecnorm` was removed. The previous behavior of `norm` for matrices is now captured by `opnorm`. This updates our extensions to these functions to match, and emits deprecation warnings as appropriate. As a somewhat related change, this also deprecates the positional dimension argument to `sum` in favor of keyword arguments, in keeping with Base. --- src/atoms/affine/sum.jl | 10 ++++++++-- src/atoms/sdp_cone/operatornorm.jl | 16 ++++++++++++++++ src/atoms/second_order_cone/norm.jl | 23 ++++++++++------------- src/atoms/second_order_cone/norm2.jl | 2 +- src/expressions.jl | 2 +- test/test_socp.jl | 25 ++++++++++++++----------- 6 files changed, 50 insertions(+), 28 deletions(-) diff --git a/src/atoms/affine/sum.jl b/src/atoms/affine/sum.jl index 36681e4a9..c7646df63 100644 --- a/src/atoms/affine/sum.jl +++ b/src/atoms/affine/sum.jl @@ -57,9 +57,13 @@ function conic_form!(x::SumAtom, unique_conic_forms::UniqueConicForms=UniqueConi return get_conic_form(unique_conic_forms, x) end -sum(x::AbstractExpr) = SumAtom(x) +# Dispatch to an internal helper function that handles the dimension argument in +# the same manner as Base, with dims=: denoting a regular sum +sum(x::AbstractExpr; dims=:) = _sum(x, dims) -function sum(x::AbstractExpr, dimension::Int) +_sum(x::AbstractExpr, ::Colon) = SumAtom(x) + +function _sum(x::AbstractExpr, dimension::Integer) if dimension == 1 return Constant(ones(1, x.size[1]), Positive()) * x elseif dimension == 2 @@ -68,3 +72,5 @@ function sum(x::AbstractExpr, dimension::Int) error("Sum not implemented for dimension $dimension") end end + +Base.@deprecate sum(x::AbstractExpr, dim::Int) sum(x, dims=dim) diff --git a/src/atoms/sdp_cone/operatornorm.jl b/src/atoms/sdp_cone/operatornorm.jl index cfe4373c6..3b334f177 100644 --- a/src/atoms/sdp_cone/operatornorm.jl +++ b/src/atoms/sdp_cone/operatornorm.jl @@ -5,6 +5,7 @@ # All expressions and atoms are subtypes of AbstractExpr. # Please read expressions.jl first. ############################################################################# +import LinearAlgebra: opnorm export operatornorm, sigmamax ### Operator norm @@ -42,6 +43,21 @@ end operatornorm(x::AbstractExpr) = OperatorNormAtom(x) sigmamax(x::AbstractExpr) = OperatorNormAtom(x) +function opnorm(x::AbstractExpr, p::Real=2) + if length(size(x)) <= 1 || minimum(size(x)) == 1 + throw(ArgumentError("argument to `opnorm` must be a matrix")) + end + if p == 1 + return maximum(sum(abs(x), dims=1)) + elseif p == 2 + return operatornorm(x) + elseif p == Inf + return maximum(sum(abs(x), dims=2)) + else + throw(ArgumentError("matrix p-norms only defined for p = 1, 2, and Inf")) + end +end + # Create the equivalent conic problem: # minimize t # subject to diff --git a/src/atoms/second_order_cone/norm.jl b/src/atoms/second_order_cone/norm.jl index 616d5efd8..f579de024 100755 --- a/src/atoms/second_order_cone/norm.jl +++ b/src/atoms/second_order_cone/norm.jl @@ -1,5 +1,5 @@ import LinearAlgebra.norm -export norm_inf, norm, norm_1, vecnorm +export norm_inf, norm, norm_1 # deprecate these soon norm_inf(x::AbstractExpr) = maximum(abs(x)) @@ -25,19 +25,16 @@ function norm(x::AbstractExpr, p::Real=2) error("vector p-norms not defined for p < 1") end else - # x is a matrix - if p == 1 - return maximum(sum(abs(x), 1)) - elseif p == 2 - return operatornorm(x) - elseif p == Inf - return maximum(sum(abs(x), 2)) - else - error("matrix p-norms only defined for p = 1, 2, and Inf") - end + # TODO: After the deprecation period, allow this again but make it consistent with + # LinearAlgebra, i.e. make norm(x, p) for x a matrix the same as norm(vec(x), p). + Base.depwarn("`norm(x, p)` for matrices will in the future be equivalent to " * + "`norm(vec(x), p)`. Use `opnorm(x, p)` for the Julia 0.6 behavior of " * + "computing the operator norm for matrices.", :norm) + return opnorm(x, p) end end -function vecnorm(x::AbstractExpr, p::Real=2) - return norm(vec(x), p) +if isdefined(LinearAlgebra, :vecnorm) # deprecated but defined + import LinearAlgebra: vecnorm end +Base.@deprecate vecnorm(x::AbstractExpr, p::Real=2) norm(vec(x), p) diff --git a/src/atoms/second_order_cone/norm2.jl b/src/atoms/second_order_cone/norm2.jl index 58a1bc313..ae690c1b8 100644 --- a/src/atoms/second_order_cone/norm2.jl +++ b/src/atoms/second_order_cone/norm2.jl @@ -5,7 +5,7 @@ # Please read expressions.jl first. ############################################################################# import LinearAlgebra.norm2 -export EucNormAtom, norm2, vecnorm +export EucNormAtom, norm2 export sign, monotonicity, curvature, conic_form! diff --git a/src/expressions.jl b/src/expressions.jl index a3b4c0bb8..766a9bad7 100644 --- a/src/expressions.jl +++ b/src/expressions.jl @@ -4,7 +4,7 @@ # Each type which subtypes AbstractExpr (Variable and Constant being exceptions) # must have: # -## head::Symbol -- a symbol such as :vecnorm, :+ etc +## head::Symbol -- a symbol such as :norm, :+ etc ## children::(AbstractExpr,) -- The expressions on which the current expression ## -- is operated ## id_hash::UInt64 -- identifier hash, can be a hash of children diff --git a/test/test_socp.jl b/test/test_socp.jl index d465f2230..232f07e21 100644 --- a/test/test_socp.jl +++ b/test/test_socp.jl @@ -49,11 +49,11 @@ TOL = 1e-3 @testset "frobenius norm atom" begin m = Variable(4, 5) c = [m[3, 3] == 4, m >= 1] - p = minimize(vecnorm(m, 2), c) + p = minimize(norm(vec(m), 2), c) @test vexity(p) == ConvexVexity() solve!(p) @test p.optval ≈ sqrt(35) atol=TOL - @test evaluate(vecnorm(m, 2)) ≈ sqrt(35) atol=TOL + @test evaluate(norm(vec(m), 2)) ≈ sqrt(35) atol=TOL end @testset "quad over lin atom" begin @@ -226,18 +226,21 @@ TOL = 1e-3 @test norm(g, 2) ^ 2 ≈ 0 atol=TOL end - @testset "norm consistent with Base" begin + @testset "norm consistent with Base for matrix variables" begin A = randn(4, 4) x = Variable(4, 4) x.value = A - @test evaluate(norm(x)) ≈ opnorm(A) atol=TOL - @test evaluate(norm(x, 1)) ≈ opnorm(A, 1) atol=TOL - @test evaluate(norm(x, 2)) ≈ opnorm(A, 2) atol=TOL - @test evaluate(norm(x, Inf)) ≈ opnorm(A, Inf) atol=TOL - @test evaluate(vecnorm(x, 1)) ≈ norm(vec(A), 1) atol=TOL - @test evaluate(vecnorm(x, 2)) ≈ norm(vec(A), 2) atol=TOL - @test evaluate(vecnorm(x, 7)) ≈ norm(vec(A), 7) atol=TOL - @test evaluate(vecnorm(x, Inf)) ≈ norm(vec(A), Inf) atol=TOL + # Matrix norm + @test evaluate(opnorm(x)) ≈ opnorm(A) atol=TOL + @test evaluate(opnorm(x, 1)) ≈ opnorm(A, 1) atol=TOL + @test evaluate(opnorm(x, 2)) ≈ opnorm(A, 2) atol=TOL + @test evaluate(opnorm(x, Inf)) ≈ opnorm(A, Inf) atol=TOL + # Vector norm + # TODO: Once the deprecation for norm on matrices is removed, remove the `vec` calls + @test evaluate(norm(vec(x), 1)) ≈ norm(vec(A), 1) atol=TOL + @test evaluate(norm(vec(x), 2)) ≈ norm(vec(A), 2) atol=TOL + @test evaluate(norm(vec(x), 7)) ≈ norm(vec(A), 7) atol=TOL + @test evaluate(norm(vec(x), Inf)) ≈ norm(vec(A), Inf) atol=TOL end From 27595b0b23f13943e741ecb2b74adffa89b93387 Mon Sep 17 00:00:00 2001 From: Alex Arslan Date: Tue, 6 Nov 2018 11:51:00 -0800 Subject: [PATCH 2/2] Deprecate operatornorm in favor of opnorm opnorm will create OperatorNormAtom objects directly for p=2. --- src/atoms/sdp_cone/operatornorm.jl | 9 +++++---- test/test_sdp.jl | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/atoms/sdp_cone/operatornorm.jl b/src/atoms/sdp_cone/operatornorm.jl index 3b334f177..7004ac3bb 100644 --- a/src/atoms/sdp_cone/operatornorm.jl +++ b/src/atoms/sdp_cone/operatornorm.jl @@ -6,7 +6,7 @@ # Please read expressions.jl first. ############################################################################# import LinearAlgebra: opnorm -export operatornorm, sigmamax +export sigmamax ### Operator norm @@ -18,7 +18,7 @@ struct OperatorNormAtom <: AbstractExpr function OperatorNormAtom(x::AbstractExpr) children = (x,) - return new(:operatornorm, hash(children), children, (1,1)) + return new(:opnorm, hash(children), children, (1,1)) end end @@ -40,7 +40,6 @@ function evaluate(x::OperatorNormAtom) opnorm(evaluate(x.children[1]), 2) end -operatornorm(x::AbstractExpr) = OperatorNormAtom(x) sigmamax(x::AbstractExpr) = OperatorNormAtom(x) function opnorm(x::AbstractExpr, p::Real=2) @@ -50,7 +49,7 @@ function opnorm(x::AbstractExpr, p::Real=2) if p == 1 return maximum(sum(abs(x), dims=1)) elseif p == 2 - return operatornorm(x) + return OperatorNormAtom(x) elseif p == Inf return maximum(sum(abs(x), dims=2)) else @@ -58,6 +57,8 @@ function opnorm(x::AbstractExpr, p::Real=2) end end +Base.@deprecate operatornorm(x::AbstractExpr) opnorm(x) + # Create the equivalent conic problem: # minimize t # subject to diff --git a/test/test_sdp.jl b/test/test_sdp.jl index 678809522..855d6999e 100644 --- a/test/test_sdp.jl +++ b/test/test_sdp.jl @@ -72,11 +72,11 @@ eye(n) = Matrix(1.0I, n, n) @testset "operator norm atom" begin y = Variable((3,3)) - p = minimize(operatornorm(y), y[2,1]<=4, y[2,2]>=3, sum(y)>=12) + p = minimize(opnorm(y), y[2,1]<=4, y[2,2]>=3, sum(y)>=12) @test vexity(p) == ConvexVexity() solve!(p) @test p.optval ≈ 4 atol=TOL - @test evaluate(operatornorm(y)) ≈ 4 atol=TOL + @test evaluate(opnorm(y)) ≈ 4 atol=TOL end @testset "sigma max atom" begin