Fixes #17, full=true returns full SVD. (#20)

Author: olof3

src/GenericSVD.jl

@@ -13,24 +13,22 @@ function svd!(X::AbstractMatrix; full::Bool=false, thin::Union{Bool,Nothing} = n
     end
     if thin != nothing
         @warn "obsolete keyword `thin` in generic svd!"
-        thinx = thin
-    else
-        thinx = !full
+        full = !thin
     end
-    generic_svdfact!(X; thin=thinx)
+    generic_svdfact!(X; full=full)
 end
 
 LinearAlgebra.svdvals!(X::AbstractMatrix) = generic_svdvals!(X)
 
-function generic_svdfact!(X::AbstractMatrix; sorted=true, thin=true)
+function generic_svdfact!(X::AbstractMatrix; sorted=true, full=false)
     m,n = size(X)
     wide = m < n
     if wide
         m,n = n,m
         X = X'
     end
     B,P = bidiagonalize_tall!(X)
-    U,Vt = unpack(P,thin=thin)
+    U,Vt = unpack(P,full=full)
     U,S,Vt = svd_bidiag!(B,U,Vt)
     # as of Julia v0.7 we need to revert a mysterious transpose here
     Vt=Vt'
@@ -45,8 +43,8 @@ function generic_svdfact!(X::AbstractMatrix; sorted=true, thin=true)
     if sorted
         Idx = sortperm(S,rev=true)
         S = S[Idx]
-        U = U[:,Idx]
-        Vt = Vt[Idx,:]
+        U[:, 1:n] .= U[:,Idx]
+        Vt[1:n, :] .= Vt[Idx,:]
     end
     wide ? SVD(Vt',S,U') : SVD(U,S,Vt)
 end

src/bidiagonalize.jl

@@ -54,12 +54,12 @@ function bidiagonalize_tall!(A::Matrix{T}) where T
     bidiagonalize_tall!(A, B)
 end
 
-function unpack(P::PackedUVt{T};thin=true) where T
+function unpack(P::PackedUVt{T};full=false) where T
     A = P.A
     m,n = size(A)
 
     # U = Q_1 ... Q_n I_{m,n}
-    w = thin ? n : m
+    w = full ? m : n
     U = Matrix(one(T)*I,m,w) # eye(T,m,w)
     for i = n:-1:1
         τi = A[i,i]

test/misc.jl

@@ -29,3 +29,33 @@ GenericSVD.svd_zerodiag_col!(B,Vt,1,3)
 @test B[3,3] == 0
 @test B[2,3] == 0
 @test Matrix(B)*Vt ≈ B1
+
+
+
+# Test of the full keyword
+A_43 = big.(reshape(1:12, 4, 3))
+
+U, S, V = svd(A_43, full=true)
+@test U'*U ≈ Matrix(I, 4, 4)
+@test V'*V ≈ Matrix(I, 3, 3)
+@test U*[diagm(0 => S); zeros(1,3)]*V' ≈ A_43
+@test issorted(S, rev=true)
+
+U, S, V = svd(A_43, full=false)
+@test U'*U ≈ Matrix(I, 3, 3)
+@test V'*V ≈ Matrix(I, 3, 3)
+@test U*diagm(0 => S)*V' ≈ A_43
+@test issorted(S, rev=true)
+
+A_34 = big.(reshape(1:12, 3, 4))
+U, S, V = svd(A_34, full=true)
+@test U'*U ≈ Matrix(I, 3, 3)
+@test V'*V ≈ Matrix(I, 4, 4)
+@test U*[diagm(0 => S) zeros(3,1)]*V' ≈ A_34
+@test issorted(S, rev=true)
+
+U, S, V = svd(A_34, full=false)
+@test U'*U ≈ Matrix(I, 3, 3)
+@test V'*V ≈ Matrix(I, 3, 3)
+@test U*diagm(0 => S)*V' ≈ A_34
+@test issorted(S, rev=true)