Skip to content

Commit

Permalink
Fix use of norm and remove vecnorm
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ararslan committed Nov 6, 2018
1 parent 4152844 commit 5dbd45d
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 28 deletions.
10 changes: 8 additions & 2 deletions src/atoms/affine/sum.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
16 changes: 16 additions & 0 deletions src/atoms/sdp_cone/operatornorm.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
23 changes: 10 additions & 13 deletions src/atoms/second_order_cone/norm.jl
Original file line number Diff line number Diff line change
@@ -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))
Expand All @@ -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)
2 changes: 1 addition & 1 deletion src/atoms/second_order_cone/norm2.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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!


Expand Down
2 changes: 1 addition & 1 deletion src/expressions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 11 additions & 11 deletions test/test_socp.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -226,18 +226,18 @@ TOL = 1e-3
@test norm(g, 2) ^ 2 0 atol=TOL
end

@testset "norm consistent with Base" begin
@testset "matrix norm consistent with Base" 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
@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
@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


Expand Down

0 comments on commit 5dbd45d

Please sign in to comment.