TUCKER_ALS: TTM with negative values is broken in ttensor (#62) (#66)

* Replace usage in tucker_als
* Update test for tucker_als to ensure result matches expectation
* Add early error handling in ttensor ttm for negative dims

Author: ntjohnson1

pyttb/pyttb_utils.py

@@ -198,7 +198,7 @@ def tt_dimscheck(
 
     # Fix "minus" case
     if np.max(dims) < 0:
-        # Check that all memebers in range
+        # Check that all members in range
         if not np.all(np.isin(-dims, np.arange(0, N + 1))):
             assert False, "Invalid magnitude for negative dims selection"
         dims = np.setdiff1d(np.arange(1, N + 1), -dims) - 1

pyttb/ttensor.py

@@ -434,7 +434,9 @@ def ttm(self, matrix, dims=None, transpose=False):
             dims = np.arange(self.ndims)
         elif isinstance(dims, list):
             dims = np.array(dims)
-        elif np.isscalar(dims) or isinstance(dims, list):
+        elif np.isscalar(dims):
+            if dims < 0:
+                raise ValueError("Negative dims is currently unsupported, see #62")
             dims = np.array([dims])
 
         if not isinstance(matrix, list):

pyttb/tucker_als.py

@@ -124,14 +124,11 @@ def tucker_als(
 
         # Iterate over all N modes of the tensor
         for n in dimorder:
-            if (
-                n == 0
-            ):  # TODO proposal to change ttm to include_dims and exclude_dims to resolve -0 ambiguity
-                dims = np.arange(1, tensor.ndims)
-                Utilde = tensor.ttm(U, dims, True)
-            else:
-                Utilde = tensor.ttm(U, -n, True)
-
+            # TODO proposal to change ttm to include_dims and exclude_dims to resolve -0 ambiguity
+            dims = np.arange(0, tensor.ndims)
+            dims = dims[dims != n]
+            Utilde = tensor.ttm(U, dims, True)
+            print(f"Utilde[{n}] = {Utilde}")
             # Maximize norm(Utilde x_n W') wrt W and
             # maintain orthonormality of W
             U[n] = Utilde.nvecs(n, rank[n])
@@ -140,13 +137,11 @@ def tucker_als(
         core = Utilde.ttm(U, n, True)
 
         # Compute fit
-        # TODO this abs is missing from MATLAB, but I get negative numbers for trivial examples
         normresidual = np.sqrt(abs(normX**2 - core.norm() ** 2))
         fit = 1 - (normresidual / normX)  # fraction explained by model
         fitchange = abs(fitold - fit)
 
         if iter % printitn == 0:
-            print(f" NormX: {normX} Core norm: {core.norm()}")
             print(f" Iter {iter}: fit = {fit:e} fitdelta = {fitchange:7.1e}\n")
 
         # Check for convergence

tests/test_ttensor.py

@@ -310,9 +310,15 @@ def test_ttensor_ttm(random_ttensor):
 
     # Negative Tests
     big_wrong_size = 123
-    matrices[0] = np.random.random((big_wrong_size, big_wrong_size))
+    bad_matrices = matrices.copy()
+    bad_matrices[0] = np.random.random((big_wrong_size, big_wrong_size))
     with pytest.raises(ValueError):
-        _ = ttensorInstance.ttm(matrices, np.arange(len(matrices)))
+        _ = ttensorInstance.ttm(bad_matrices, np.arange(len(bad_matrices)))
+
+    with pytest.raises(ValueError):
+        # Negative dims currently broken, ensure we catch early and
+        # remove once resolved
+        ttensorInstance.ttm(matrices, -1)
 
 
 @pytest.mark.indevelopment

tests/test_tucker_als.py

@@ -19,6 +19,7 @@ def test_tucker_als_tensor_default_init(capsys, sample_tensor):
     (Solution, Uinit, output) = ttb.tucker_als(T, 2)
     capsys.readouterr()
     assert pytest.approx(output["fit"], 1) == 0
+    assert np.all(np.isclose(Solution.double(), T.double()))
 
     (Solution, Uinit, output) = ttb.tucker_als(T, 2, init=Uinit)
     capsys.readouterr()