From 3ee7ae4357a7e9de1d8e447f680f63ff43ba8f11 Mon Sep 17 00:00:00 2001
From: Alex Arslan <ararslan@comcast.net>
Date: Tue, 6 Nov 2018 23:35:21 -0800
Subject: [PATCH] Remove vecdot

The `vecdot` function was deprecated in favor of `dot` in Julia PR
27401; `dot` on matrices now treats them like vectors.

The change here updates our extension of `dot` to match this behavior
and deprecates `vecnorm`.
---
 src/atoms/affine/dot.jl | 37 +++++++++++++++++--------------------
 test/test_affine.jl     |  6 +++---
 2 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/src/atoms/affine/dot.jl b/src/atoms/affine/dot.jl
index 96ffb8393..fb235371b 100644
--- a/src/atoms/affine/dot.jl
+++ b/src/atoms/affine/dot.jl
@@ -1,26 +1,23 @@
 import LinearAlgebra.dot
-export vecdot, dot
+export dot
 
+ismatrix(x::AbstractExpr) = (s = size(x); length(s) == 2 && s[1] > 1 && s[2] > 1)
+ismatrix(::AbstractMatrix) = true
+ismatrix(::Any) = false
 
-vecdot(x::AbstractExpr, y::AbstractExpr) = sum(broadcast(*, x, y))
-vecdot(x::Value, y::AbstractExpr) = sum(broadcast(*, Constant(x), y))
-vecdot(x::AbstractExpr, y::Value) = sum(broadcast(*, x, Constant(y)))
+# NOTE: Using asvec avoids broadcast-specific behaviors that we want to avoid, such
+# as extending singleton dimensions. We need to ensure that the inputs have the same
+# length, which broadcast will check for us if both inputs are vectors.
+asvec(x) = convert(AbstractExpr, ismatrix(x) ? vec(x) : x)
+_vecdot(x, y) = sum(broadcast(*, asvec(x), asvec(y)))
 
-dot(x::AbstractExpr, y::AbstractExpr) = (ismatrix(x) || ismatrix(y)) ? error("dot not implemented for matrices. perhaps you're looking for vecdot?") : vecdot(x, y)
-dot(x::Value, y::AbstractExpr) = (ismatrix(x) || ismatrix(y)) ? error("dot not implemented for matrices. perhaps you're looking for vecdot?") : vecdot(x, y)
-dot(x::AbstractExpr, y::Value) = (ismatrix(x) || ismatrix(y)) ? error("dot not implemented for matrices. perhaps you're looking for vecdot?") : vecdot(x, y)
+dot(x::AbstractExpr, y::AbstractExpr) = _vecdot(x, y)
+dot(x::Value, y::AbstractExpr) = _vecdot(x, y)
+dot(x::AbstractExpr, y::Value) = _vecdot(x, y)
 
-# tests if an array is a matrix (2D array) with both dimensions of size > 1
-function ismatrix(x)
-    sz = size(x)
-    if length(sz) != 2
-        return false
-    else
-        for s in sz
-            if s == 1
-                return false
-            end
-        end
-    end
-    return true
+if isdefined(LinearAlgebra, :vecdot) # defined but deprecated
+    import LinearAlgebra: vecdot
 end
+Base.@deprecate vecdot(x::AbstractExpr, y::AbstractExpr) dot(x, y)
+Base.@deprecate vecdot(x::Value, y::AbstractExpr) dot(x, y)
+Base.@deprecate vecdot(x::AbstractExpr, y::Value) dot(x, y)
diff --git a/test/test_affine.jl b/test/test_affine.jl
index 6447b9407..00f549b02 100644
--- a/test/test_affine.jl
+++ b/test/test_affine.jl
@@ -60,13 +60,13 @@ eye(n) = Matrix(1.0I, n, n)
         @test (evaluate(dot([2.0; 2.0], x)))[1] ≈ 4.4 atol=TOL
     end
 
-    @testset "vecdot atom" begin
+    @testset "dot atom for matrix variables" begin
         x = Variable(2,2)
-        p = minimize(vecdot(fill(2.0, (2,2)), x), x >= 1.1)
+        p = minimize(dot(fill(2.0, (2,2)), x), x >= 1.1)
         @test vexity(p) == AffineVexity()
         solve!(p)
         @test p.optval ≈ 8.8 atol=TOL
-        @test (evaluate(vecdot(fill(2.0, (2, 2)), x)))[1] ≈ 8.8 atol=TOL
+        @test (evaluate(dot(fill(2.0, (2, 2)), x)))[1] ≈ 8.8 atol=TOL
     end
 
     @testset "add atom" begin