Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix use of norm and remove vecnorm #242

Merged
merged 2 commits into from
Nov 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
ararslan marked this conversation as resolved.
Show resolved Hide resolved

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)
23 changes: 20 additions & 3 deletions src/atoms/sdp_cone/operatornorm.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
# All expressions and atoms are subtypes of AbstractExpr.
# Please read expressions.jl first.
#############################################################################
export operatornorm, sigmamax
import LinearAlgebra: opnorm
export sigmamax

### Operator norm

Expand All @@ -17,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

Expand All @@ -39,9 +40,25 @@ 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)
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 OperatorNormAtom(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

Base.@deprecate operatornorm(x::AbstractExpr) opnorm(x)

# 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
4 changes: 2 additions & 2 deletions test/test_sdp.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 14 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,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


Expand Down