Repository: tensorly/tensorly

Issue:
Inaccurate reconstruction error for PARAFAC with missing data
When running (non-negative) parafac on a tensor with missing data (i.e. using ```mask```), in the current version the reconstruction error at every iteration is calculated based on the norm of the original data tensor. However, the masked part of the tensor is imputated at every iteration, so the norm of the tensor is not constant. Hence the values of the reconstruction error are inaccurate in this case, and the error is not strictly decreasing at every iteration.

Recalculating the norm of the tensor at every iteration appears to restore the expected behavior.

Discussion:

Solution patch:
diff --git a/tensorly/decomposition/_nn_cp.py b/tensorly/decomposition/_nn_cp.py
index 2dd7a04..3a2673a 100755
--- a/tensorly/decomposition/_nn_cp.py
+++ b/tensorly/decomposition/_nn_cp.py
@@ -1,7 +1,7 @@
 import warnings
 import tensorly as tl
 from ._base_decomposition import DecompositionMixin
-from ._cp import initialize_cp
+from ._cp import initialize_cp, error_calc
 from ..solvers.nnls import hals_nnls
 from ..cp_tensor import (
     CPTensor,
@@ -10,7 +10,6 @@ from ..cp_tensor import (
     cp_normalize,
     validate_cp_rank,
 )
-from ..tenalg.svd import svd_interface
 
 # Authors: Jean Kossaifi <jean.kossaifi+tensors@gmail.com>
 #          Chris Swierczewski <csw@amazon.com>
@@ -139,17 +138,19 @@ def non_negative_parafac(
                 weights, factors = cp_normalize((weights, factors))
 
         if tol:
-            # ||tensor - rec||^2 = ||tensor||^2 + ||rec||^2 - 2*<tensor, rec>
-            factors_norm = cp_norm((weights, factors))
-
-            # mttkrp and factor for the last mode. This is equivalent to the
-            # inner product <tensor, factorization>
-            iprod = tl.sum(tl.sum(mttkrp * factor, axis=0))
-            rec_error = (
-                tl.sqrt(tl.abs(norm_tensor**2 + factors_norm**2 - 2 * iprod))
-                / norm_tensor
+            # Calculate the reconstruction error. We can use the same method from CP.
+            unnorml_rec_error, tensor, norm_tensor = error_calc(
+                tensor,
+                norm_tensor,
+                weights,
+                factors,
+                sparsity=None,
+                mask=mask,
+                mttkrp=mttkrp,
             )
+            rec_error = unnorml_rec_error / norm_tensor
             rec_errors.append(rec_error)
+
             if iteration >= 1:
                 rec_error_decrease = rec_errors[-2] - rec_errors[-1]
 
@@ -346,6 +347,7 @@ def non_negative_parafac_hals(
                 / norm_tensor
             )
             rec_errors.append(rec_error)
+
             if iteration >= 1:
                 rec_error_decrease = rec_errors[-2] - rec_errors[-1]
 
@@ -462,6 +464,7 @@ class CP_NN(DecompositionMixin):
         cvg_criterion="abs_rec_error",
         fixed_modes=None,
     ):
+        self.rank = rank
         self.n_iter_max = n_iter_max
         self.init = init
         self.svd = svd
@@ -489,6 +492,7 @@ class CP_NN(DecompositionMixin):
 
         cp_tensor, errors = non_negative_parafac(
             tensor,
+            rank=self.rank,
             n_iter_max=self.n_iter_max,
             init=self.init,
             svd=self.svd,
diff --git a/tensorly/tucker_tensor.py b/tensorly/tucker_tensor.py
index 50bfbc7..d5b3487 100644
--- a/tensorly/tucker_tensor.py
+++ b/tensorly/tucker_tensor.py
@@ -47,7 +47,9 @@ def _validate_tucker_tensor(tucker_tensor):
     return tuple(shape), tuple(rank)
 
 
-def tucker_to_tensor(tucker_tensor, skip_factor=None, transpose_factors=False):
+def tucker_to_tensor(
+    tucker_tensor, skip_factor=None, transpose_factors=False, modes=None
+):
     """Converts the Tucker tensor into a full tensor
 
     Parameters
@@ -59,6 +61,8 @@ def tucker_to_tensor(tucker_tensor, skip_factor=None, transpose_factors=False):
         Note that in any case, `modes`, if provided, should have a lengh of ``tensor.ndim``
     transpose_factors : bool, optional, default is False
         if True, the matrices or vectors in in the list are transposed
+    modes : None or int list, optional, default is None
+        list of the modes on which to perform the decomposition
 
     Returns
     -------
@@ -66,7 +70,9 @@ def tucker_to_tensor(tucker_tensor, skip_factor=None, transpose_factors=False):
        full tensor of shape ``(factors[0].shape[0], ..., factors[-1].shape[0])``
     """
     core, factors = tucker_tensor
-    return multi_mode_dot(core, factors, skip=skip_factor, transpose=transpose_factors)
+    return multi_mode_dot(
+        core, factors, skip=skip_factor, transpose=transpose_factors, modes=modes
+    )
 
 
 def tucker_normalize(tucker_tensor):


Test patch:
diff --git a/tensorly/tests/test_tucker_tensor.py b/tensorly/tests/test_tucker_tensor.py
index 21e716a..0309b0b 100644
--- a/tensorly/tests/test_tucker_tensor.py
+++ b/tensorly/tests/test_tucker_tensor.py
@@ -91,6 +91,45 @@ def test_tucker_to_tensor():
     assert_array_equal(true_res, res)
 
 
+def test_tucker_to_tensor_with_partial_modes():
+    """Test for tucker_to_tensor with partial modes"""
+    X = tl.tensor(
+        np.array(
+            [
+                [[1.0, 13], [4, 16], [7, 19], [10, 22]],
+                [[2, 14], [5, 17], [8, 20], [11, 23]],
+                [[3, 15], [6, 18], [9, 21], [12, 24]],
+            ]
+        )
+    )
+    ranks = [3, 4]
+    U = [
+        tl.tensor(np.arange(R * s, dtype=float).reshape((R, s)))
+        for (R, s) in zip(ranks, tl.shape(X)[1:])
+    ]
+    true_res = np.array(
+        [
+            [
+                [120.0, 456.0, 792.0, 1128.0],
+                [400.0, 1472.0, 2544.0, 3616.0],
+                [680.0, 2488.0, 4296.0, 6104.0],
+            ],
+            [
+                [126.0, 486.0, 846.0, 1206.0],
+                [422.0, 1582.0, 2742.0, 3902.0],
+                [718.0, 2678.0, 4638.0, 6598.0],
+            ],
+            [
+                [132.0, 516.0, 900.0, 1284.0],
+                [444.0, 1692.0, 2940.0, 4188.0],
+                [756.0, 2868.0, 4980.0, 7092.0],
+            ],
+        ]
+    )
+    res = tucker_to_tensor((X, U), modes=[1, 2])
+    assert_array_equal(true_res, res)
+
+
 def test_tucker_to_unfolded():
     """Test for tucker_to_unfolded