You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@systemds.apache.org by GitBox <gi...@apache.org> on 2020/09/29 11:44:18 UTC

[GitHub] [systemds] Baunsgaard commented on a change in pull request #1070: Rewriting ALS functions

Baunsgaard commented on a change in pull request #1070:
URL: https://github.com/apache/systemds/pull/1070#discussion_r496642591



##########
File path: scripts/builtin/als_cg.dml
##########
@@ -0,0 +1,172 @@
+#-------------------------------------------------------------
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+#-------------------------------------------------------------
+
+#  
+# THIS SCRIPT COMPUTES AN APPROXIMATE FACTORIZATION OF A LOW-RANK MATRIX X INTO TWO MATRICES U AND V
+# USING THE ALTERNATING-LEAST-SQUARES (ALS) ALGORITHM WITH CONJUGATE GRADIENT.
+# MATRICES U AND V ARE COMPUTED BY MINIMIZING A LOSS FUNCTION (WITH REGULARIZATION).
+#
+# INPUT   PARAMETERS:
+# ---------------------------------------------------------------------------------------------
+# NAME    TYPE     DEFAULT  MEANING
+# ---------------------------------------------------------------------------------------------
+# X       String   ---      Location to read the input matrix X to be factorized
+# U       String   ---      Location to write the factor matrix U
+# V       String   ---      Location to write the factor matrix V
+# rank    Int      10       Rank of the factorization
+# reg     String   "L2"	    Regularization: 
+#                           "L2" = L2 regularization;
+#                           "wL2" = weighted L2 regularization
+# lambda  Double   0.000001 Regularization parameter, no regularization if 0.0
+# maxi    Int      50       Maximum number of iterations
+# check   Boolean  TRUE     Check for convergence after every iteration, i.e., updating U and V once
+# thr     Double   0.0001   Assuming check is set to TRUE, the algorithm stops and convergence is declared 
+#                           if the decrease in loss in any two consecutive iterations falls below this threshold; 
+#                           if check is FALSE thr is ignored
+# fmt     String   "text"   The output format of the factor matrices L and R, such as "text" or "csv"
+# ---------------------------------------------------------------------------------------------
+# OUTPUT: 
+# 1- An m x r matrix U, where r is the factorization rank 
+# 2- An r x n matrix V
+#
+# HOW TO INVOKE THIS SCRIPT - EXAMPLE:
+# hadoop jar SystemDS.jar -f ALS-CG.dml -nvargs X=INPUT_DIR/X U=OUTPUT_DIR/U V=OUTPUT_DIR/V rank=10 reg="L2" lambda=0.0001 fmt=csv
+
+
+m_als_cg = function(Matrix[Double] X, Integer rank = 10, String reg = "L2", Double lambda = 0.000001, Integer maxi = 50, Boolean check = TRUE, Double thr = 0.0001, String fmt = "text")
+    return (Matrix[Double] U, Matrix[Double] V)
+    {
+
+    r = rank;
+    max_iter = maxi;
+    fmt0 = fmt;
+
+    ###### MAIN PART ######
+    m = nrow (X);
+    n = ncol (X);
+
+    # initializing factor matrices
+    U = rand (rows = m, cols = r, min = -0.5, max = 0.5); # mxr
+    V = rand (rows = n, cols = r, min = -0.5, max = 0.5); # nxr
+
+    W = (X != 0);
+
+    # check for regularization
+    row_nonzeros = matrix(0,rows=1,cols=1);
+    col_nonzeros = matrix(0,rows=1,cols=1);
+    if( reg == "L2" ) {
+      print ("BEGIN ALS-CG SCRIPT WITH NONZERO SQUARED LOSS + L2 WITH LAMBDA - " + lambda);
+      row_nonzeros = matrix(1, nrow(W), 1);
+      col_nonzeros = matrix(1, ncol(W), 1);
+    }
+    else if( reg == "wL2" ) {
+      print ("BEGIN ALS-CG SCRIPT WITH NONZERO SQUARED LOSS + WEIGHTED L2 WITH LAMBDA - " + lambda);
+      row_nonzeros = rowSums(W);
+      col_nonzeros = t(colSums(W));
+    }
+    else {
+      stop ("wrong regularization! " + reg);
+    }
+
+    # Loss Function with L2:
+    # f (U, V) = 0.5 * sum (W * (U %*% V - X) ^ 2)
+    #          + 0.5 * lambda * (sum (U ^ 2) + sum (V ^ 2))
+    # Loss Function with weighted L2:
+    # f (U, V) = 0.5 * sum (W * (U %*% V - X) ^ 2)
+    #          + 0.5 * lambda * (sum (U ^ 2 * row_nonzeros) + sum (V ^ 2 * col_nonzeros))
+
+    is_U = TRUE;  # TRUE = Optimize U, FALSE = Optimize V

Review comment:
       I prefer inline comments, just above the statement.

##########
File path: scripts/builtin/als_cg.dml
##########
@@ -0,0 +1,172 @@
+#-------------------------------------------------------------
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+#-------------------------------------------------------------
+
+#  
+# THIS SCRIPT COMPUTES AN APPROXIMATE FACTORIZATION OF A LOW-RANK MATRIX X INTO TWO MATRICES U AND V
+# USING THE ALTERNATING-LEAST-SQUARES (ALS) ALGORITHM WITH CONJUGATE GRADIENT.
+# MATRICES U AND V ARE COMPUTED BY MINIMIZING A LOSS FUNCTION (WITH REGULARIZATION).
+#
+# INPUT   PARAMETERS:
+# ---------------------------------------------------------------------------------------------
+# NAME    TYPE     DEFAULT  MEANING
+# ---------------------------------------------------------------------------------------------
+# X       String   ---      Location to read the input matrix X to be factorized
+# U       String   ---      Location to write the factor matrix U
+# V       String   ---      Location to write the factor matrix V
+# rank    Int      10       Rank of the factorization
+# reg     String   "L2"	    Regularization: 
+#                           "L2" = L2 regularization;
+#                           "wL2" = weighted L2 regularization
+# lambda  Double   0.000001 Regularization parameter, no regularization if 0.0
+# maxi    Int      50       Maximum number of iterations
+# check   Boolean  TRUE     Check for convergence after every iteration, i.e., updating U and V once
+# thr     Double   0.0001   Assuming check is set to TRUE, the algorithm stops and convergence is declared 
+#                           if the decrease in loss in any two consecutive iterations falls below this threshold; 
+#                           if check is FALSE thr is ignored
+# fmt     String   "text"   The output format of the factor matrices L and R, such as "text" or "csv"
+# ---------------------------------------------------------------------------------------------
+# OUTPUT: 
+# 1- An m x r matrix U, where r is the factorization rank 
+# 2- An r x n matrix V
+#
+# HOW TO INVOKE THIS SCRIPT - EXAMPLE:
+# hadoop jar SystemDS.jar -f ALS-CG.dml -nvargs X=INPUT_DIR/X U=OUTPUT_DIR/U V=OUTPUT_DIR/V rank=10 reg="L2" lambda=0.0001 fmt=csv
+
+
+m_als_cg = function(Matrix[Double] X, Integer rank = 10, String reg = "L2", Double lambda = 0.000001, Integer maxi = 50, Boolean check = TRUE, Double thr = 0.0001, String fmt = "text")
+    return (Matrix[Double] U, Matrix[Double] V)
+    {
+
+    r = rank;
+    max_iter = maxi;
+    fmt0 = fmt;
+
+    ###### MAIN PART ######
+    m = nrow (X);
+    n = ncol (X);
+
+    # initializing factor matrices
+    U = rand (rows = m, cols = r, min = -0.5, max = 0.5); # mxr
+    V = rand (rows = n, cols = r, min = -0.5, max = 0.5); # nxr
+
+    W = (X != 0);
+
+    # check for regularization
+    row_nonzeros = matrix(0,rows=1,cols=1);
+    col_nonzeros = matrix(0,rows=1,cols=1);
+    if( reg == "L2" ) {
+      print ("BEGIN ALS-CG SCRIPT WITH NONZERO SQUARED LOSS + L2 WITH LAMBDA - " + lambda);
+      row_nonzeros = matrix(1, nrow(W), 1);
+      col_nonzeros = matrix(1, ncol(W), 1);
+    }
+    else if( reg == "wL2" ) {
+      print ("BEGIN ALS-CG SCRIPT WITH NONZERO SQUARED LOSS + WEIGHTED L2 WITH LAMBDA - " + lambda);
+      row_nonzeros = rowSums(W);
+      col_nonzeros = t(colSums(W));
+    }
+    else {
+      stop ("wrong regularization! " + reg);
+    }
+
+    # Loss Function with L2:
+    # f (U, V) = 0.5 * sum (W * (U %*% V - X) ^ 2)
+    #          + 0.5 * lambda * (sum (U ^ 2) + sum (V ^ 2))
+    # Loss Function with weighted L2:
+    # f (U, V) = 0.5 * sum (W * (U %*% V - X) ^ 2)
+    #          + 0.5 * lambda * (sum (U ^ 2 * row_nonzeros) + sum (V ^ 2 * col_nonzeros))

Review comment:
       please move these to the individual if statements, and consider making this part of the docs in the top of the algorithm to describe the reg parameter.

##########
File path: scripts/builtin/als_ds.dml
##########
@@ -0,0 +1,160 @@
+#-------------------------------------------------------------
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+#-------------------------------------------------------------
+
+#
+# ALS algorithm using a direct solve method for individual least squares problems. This script
+# computes an approximate factorization of a low-rank matrix V into two matrices L and R.
+# Matrices L and R are computed by minimizing a loss function (with regularization).
+#
+# INPUT   PARAMETERS:
+# ---------------------------------------------------------------------------------------------
+# NAME    TYPE     DEFAULT  MEANING
+# ---------------------------------------------------------------------------------------------
+# V       String   ---      Location to read the input matrix V to be factorized
+# L       String   ---      Location to write the factor matrix L
+# R       String   ---      Location to write the factor matrix R
+# rank    Int      10       Rank of the factorization
+# reg     String   "L2"     Regularization:
+#                           "L2" = L2 regularization;
+#                           "wL2" = weighted L2 regularization
+# lambda  Double   0.000001 Regularization parameter, no regularization if 0.0
+# maxi    Int      50       Maximum number of iterations
+# check   Boolean  FALSE    Check for convergence after every iteration, i.e., updating L and R once
+# thr     Double   0.0001   Assuming check is set to TRUE, the algorithm stops and convergence is declared
+#                           if the decrease in loss in any two consecutive iterations falls below this threshold;
+#                           if check is FALSE thr is ignored
+# fmt     String   "text"   The output format of the factor matrices L and R, such as "text" or "csv"
+# ---------------------------------------------------------------------------------------------
+# OUTPUT:
+# 1- An m x r matrix L, where r is the factorization rank
+# 2- An r x n matrix R
+#
+
+m_als_ds = function(Matrix[Double] V, Integer rank = 10, String reg = "L2", Double lambda = 0.000001, Integer maxi = 50, Boolean check = FALSE, Double thr = 0.0001, String fmt = "text")
+    return (Matrix[Double] L, Matrix[Double] R)
+    {
+
+    r = rank;
+    max_iter = maxi;
+    fmt0 = fmt;
+
+    # check the input matrix V, if some rows or columns contain only zeros remove them from V
+    V_nonzero_ind = V != 0;
+    row_nonzeros = rowSums (V_nonzero_ind);
+    col_nonzeros = t (colSums (V_nonzero_ind));
+    orig_nonzero_rows_ind = row_nonzeros != 0;
+    orig_nonzero_cols_ind = col_nonzeros != 0;
+    num_zero_rows = nrow (V) - sum (orig_nonzero_rows_ind);
+    num_zero_cols = ncol (V) - sum (orig_nonzero_cols_ind);
+    if (num_zero_rows > 0) {
+      print ("Matrix V contains empty rows! These rows will be removed.");
+      V = removeEmpty (target = V, margin = "rows");
+    }
+    if (num_zero_cols > 0) {
+      print ("Matrix V contains empty columns! These columns will be removed.");
+      V = removeEmpty (target = V, margin = "cols");
+    }
+    if (num_zero_rows > 0 | num_zero_cols > 0) {
+      print ("Recomputing nonzero rows and columns!");
+      V_nonzero_ind = V != 0;
+      row_nonzeros = rowSums (V_nonzero_ind);
+      col_nonzeros = t (colSums (V_nonzero_ind));
+    }
+
+    ###### MAIN PART ######
+    m = nrow (V);
+    n = ncol (V);
+
+    # initializing factor matrices
+    L = rand (rows = m, cols = r, min = -0.5, max = 0.5);
+    R = rand (rows = n, cols = r, min = -0.5, max = 0.5);
+
+    # initializing transformed matrices
+    Vt = t(V);
+
+    # check for regularization
+    if (reg == "L2") {
+      print ("BEGIN ALS SCRIPT WITH NONZERO SQUARED LOSS + L2 WITH LAMBDA - " + lambda);
+    } else if (reg == "wL2") {
+      print ("BEGIN ALS SCRIPT WITH NONZERO SQUARED LOSS + WEIGHTED L2 WITH LAMBDA - " + lambda);
+    } else {
+      stop ("wrong regularization! " + reg);
+    }
+
+    loss_init = 0.0; # only used if check is TRUE
+    if (check) {
+      loss_init = sum (V_nonzero_ind * (V - (L %*% t(R)))^2) + lambda * (sum ((L^2) * row_nonzeros) + sum ((R^2) * col_nonzeros));
+      print ("----- Initial train loss: " + loss_init + " -----");
+    }
+
+    lambda_I = diag (matrix (lambda, rows = r, cols = 1));
+    it = 0;
+    converged = FALSE;
+    while ((it < max_iter) & (!converged)) {
+      it = it + 1;
+      # keep R fixed and update L
+      parfor (i in 1:m) {
+        R_nonzero_ind = t(V[i,] != 0);
+        R_nonzero = removeEmpty (target=R * R_nonzero_ind, margin="rows");
+        A1 = (t(R_nonzero) %*% R_nonzero) + (as.scalar(row_nonzeros[i,1]) * lambda_I); # coefficient matrix
+        L[i,] = t(solve (A1, t(V[i,] %*% R)));
+      }
+
+      # keep L fixed and update R
+      parfor (j in 1:n) {
+        L_nonzero_ind = t(Vt[j,] != 0)
+        L_nonzero = removeEmpty (target=L * L_nonzero_ind, margin="rows");
+        A2 = (t(L_nonzero) %*% L_nonzero) + (as.scalar(col_nonzeros[j,1]) * lambda_I); # coefficient matrix
+        R[j,] = t(solve (A2, t(Vt[j,] %*% L)));
+      }
+
+      # check for convergence
+      if (check) {
+        loss_cur = sum (V_nonzero_ind * (V - (L %*% t(R)))^2) + lambda * (sum ((L^2) * row_nonzeros) + sum ((R^2) * col_nonzeros));
+        loss_dec = (loss_init - loss_cur) / loss_init;
+        print ("Train loss at iteration (R) " + it + ": " + loss_cur + " loss-dec " + loss_dec);
+        if (loss_dec >= 0 & loss_dec < thr | loss_init == 0) {
+          print ("----- ALS converged after " + it + " iterations!");
+          converged = TRUE;
+        }
+        loss_init = loss_cur;
+      }
+    } # end of while loop
+
+    if (check) {
+      print ("----- Final train loss: " + loss_init + " -----");
+    }
+
+    if (!converged) {
+      print ("Max iteration achieved but not converged!");
+    }
+
+    # inject 0s in L if original V had empty rows
+    if (num_zero_rows > 0) {
+      L = removeEmpty (target = diag (orig_nonzero_rows_ind), margin = "cols") %*% L;
+    }
+    # inject 0s in R if original V had empty rows
+    if (num_zero_cols > 0) {
+      R = removeEmpty (target = diag (orig_nonzero_cols_ind), margin = "cols") %*% R;
+    }
+    Rt = t (R);
+
+    }

Review comment:
       please add a newline in the end of the scripts, to make GitHub happy.

##########
File path: src/test/java/org/apache/sysds/test/functions/builtin/BuiltinALSDSTest.java
##########
@@ -0,0 +1,53 @@
+package org.apache.sysds.test.functions.builtin;
+
+import org.apache.sysds.test.AutomatedTestBase;
+import org.apache.sysds.test.TestConfiguration;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class BuiltinALSDSTest extends AutomatedTestBase {
+
+    private final static String TEST_NAME = "als_ds";
+    private final static String TEST_DIR = "functions/builtin/";
+    private static final String TEST_CLASS_DIR = TEST_DIR + BuiltinALSDSTest.class.getSimpleName() + "/";
+
+    private final static int rows = 300;
+    private final static int cols = 20;
+
+    @Override
+    public void setUp() {
+        addTestConfiguration(TEST_NAME,new TestConfiguration(TEST_CLASS_DIR, TEST_NAME,new String[]{"B"}));
+    }
+
+    @Test
+    public void testALSDS() {
+        runtestALSDS();
+    }
+
+    private void runtestALSDS(){
+        loadTestConfiguration(getTestConfiguration(TEST_NAME));
+        String HOME = SCRIPT_DIR + TEST_DIR;
+        fullDMLScriptName = HOME + TEST_NAME + ".dml";
+        List<String> proArgs = new ArrayList<>();
+
+        proArgs.add("-explain");
+        proArgs.add("-stats");
+        proArgs.add("-args");
+        proArgs.add(input("V"));
+        proArgs.add(output("L"));
+        proArgs.add(output("R"));
+        programArgs = proArgs.toArray(new String[proArgs.size()]);
+
+        double[][] V = getRandomMatrix(rows, cols, 0, 1, 0.8, -1);
+        writeInputMatrixWithMTD("V", V, true);
+
+        runTest(true, EXCEPTION_NOT_EXPECTED, null, -1);
+
+
+

Review comment:
       same points as the other tests case.
   
   Also in this case, you add many blank newlines, that could be removed.

##########
File path: src/test/java/org/apache/sysds/test/functions/builtin/BuiltinALSDSTest.java
##########
@@ -0,0 +1,53 @@
+package org.apache.sysds.test.functions.builtin;

Review comment:
       the reason github complains about build is that you are missing a license here.
   
   use `mvn package -P rat` to find out where the error is, it generates a file in /target/rat.txt that tells you.

##########
File path: scripts/builtin/als_cg.dml
##########
@@ -0,0 +1,172 @@
+#-------------------------------------------------------------
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+#-------------------------------------------------------------
+
+#  
+# THIS SCRIPT COMPUTES AN APPROXIMATE FACTORIZATION OF A LOW-RANK MATRIX X INTO TWO MATRICES U AND V
+# USING THE ALTERNATING-LEAST-SQUARES (ALS) ALGORITHM WITH CONJUGATE GRADIENT.
+# MATRICES U AND V ARE COMPUTED BY MINIMIZING A LOSS FUNCTION (WITH REGULARIZATION).
+#
+# INPUT   PARAMETERS:
+# ---------------------------------------------------------------------------------------------
+# NAME    TYPE     DEFAULT  MEANING
+# ---------------------------------------------------------------------------------------------
+# X       String   ---      Location to read the input matrix X to be factorized
+# U       String   ---      Location to write the factor matrix U
+# V       String   ---      Location to write the factor matrix V
+# rank    Int      10       Rank of the factorization
+# reg     String   "L2"	    Regularization: 
+#                           "L2" = L2 regularization;
+#                           "wL2" = weighted L2 regularization
+# lambda  Double   0.000001 Regularization parameter, no regularization if 0.0
+# maxi    Int      50       Maximum number of iterations
+# check   Boolean  TRUE     Check for convergence after every iteration, i.e., updating U and V once
+# thr     Double   0.0001   Assuming check is set to TRUE, the algorithm stops and convergence is declared 
+#                           if the decrease in loss in any two consecutive iterations falls below this threshold; 
+#                           if check is FALSE thr is ignored
+# fmt     String   "text"   The output format of the factor matrices L and R, such as "text" or "csv"
+# ---------------------------------------------------------------------------------------------

Review comment:
       i would like a verbose flag, specifying to print or not.

##########
File path: scripts/builtin/als_ds.dml
##########
@@ -0,0 +1,160 @@
+#-------------------------------------------------------------
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+#-------------------------------------------------------------
+
+#
+# ALS algorithm using a direct solve method for individual least squares problems. This script
+# computes an approximate factorization of a low-rank matrix V into two matrices L and R.
+# Matrices L and R are computed by minimizing a loss function (with regularization).
+#
+# INPUT   PARAMETERS:
+# ---------------------------------------------------------------------------------------------
+# NAME    TYPE     DEFAULT  MEANING
+# ---------------------------------------------------------------------------------------------
+# V       String   ---      Location to read the input matrix V to be factorized
+# L       String   ---      Location to write the factor matrix L
+# R       String   ---      Location to write the factor matrix R
+# rank    Int      10       Rank of the factorization
+# reg     String   "L2"     Regularization:
+#                           "L2" = L2 regularization;
+#                           "wL2" = weighted L2 regularization
+# lambda  Double   0.000001 Regularization parameter, no regularization if 0.0
+# maxi    Int      50       Maximum number of iterations
+# check   Boolean  FALSE    Check for convergence after every iteration, i.e., updating L and R once
+# thr     Double   0.0001   Assuming check is set to TRUE, the algorithm stops and convergence is declared
+#                           if the decrease in loss in any two consecutive iterations falls below this threshold;
+#                           if check is FALSE thr is ignored
+# fmt     String   "text"   The output format of the factor matrices L and R, such as "text" or "csv"
+# ---------------------------------------------------------------------------------------------

Review comment:
       verbose flag here as well.

##########
File path: scripts/builtin/als_ds.dml
##########
@@ -0,0 +1,160 @@
+#-------------------------------------------------------------
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+#-------------------------------------------------------------
+
+#
+# ALS algorithm using a direct solve method for individual least squares problems. This script

Review comment:
       please write Alternating Least Squares here as well. since the abbreviation is not enough to find it easily.

##########
File path: scripts/builtin/als_cg.dml
##########
@@ -0,0 +1,172 @@
+#-------------------------------------------------------------
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+#-------------------------------------------------------------
+
+#  
+# THIS SCRIPT COMPUTES AN APPROXIMATE FACTORIZATION OF A LOW-RANK MATRIX X INTO TWO MATRICES U AND V
+# USING THE ALTERNATING-LEAST-SQUARES (ALS) ALGORITHM WITH CONJUGATE GRADIENT.
+# MATRICES U AND V ARE COMPUTED BY MINIMIZING A LOSS FUNCTION (WITH REGULARIZATION).
+#
+# INPUT   PARAMETERS:
+# ---------------------------------------------------------------------------------------------
+# NAME    TYPE     DEFAULT  MEANING
+# ---------------------------------------------------------------------------------------------
+# X       String   ---      Location to read the input matrix X to be factorized
+# U       String   ---      Location to write the factor matrix U
+# V       String   ---      Location to write the factor matrix V
+# rank    Int      10       Rank of the factorization
+# reg     String   "L2"	    Regularization: 
+#                           "L2" = L2 regularization;
+#                           "wL2" = weighted L2 regularization
+# lambda  Double   0.000001 Regularization parameter, no regularization if 0.0
+# maxi    Int      50       Maximum number of iterations
+# check   Boolean  TRUE     Check for convergence after every iteration, i.e., updating U and V once
+# thr     Double   0.0001   Assuming check is set to TRUE, the algorithm stops and convergence is declared 
+#                           if the decrease in loss in any two consecutive iterations falls below this threshold; 
+#                           if check is FALSE thr is ignored
+# fmt     String   "text"   The output format of the factor matrices L and R, such as "text" or "csv"
+# ---------------------------------------------------------------------------------------------
+# OUTPUT: 
+# 1- An m x r matrix U, where r is the factorization rank 
+# 2- An r x n matrix V
+#
+# HOW TO INVOKE THIS SCRIPT - EXAMPLE:
+# hadoop jar SystemDS.jar -f ALS-CG.dml -nvargs X=INPUT_DIR/X U=OUTPUT_DIR/U V=OUTPUT_DIR/V rank=10 reg="L2" lambda=0.0001 fmt=csv
+
+
+m_als_cg = function(Matrix[Double] X, Integer rank = 10, String reg = "L2", Double lambda = 0.000001, Integer maxi = 50, Boolean check = TRUE, Double thr = 0.0001, String fmt = "text")
+    return (Matrix[Double] U, Matrix[Double] V)
+    {

Review comment:
       indentation could be double checked. It looks confusing to me that the outer indentation is the same as the inner.
   
   

##########
File path: src/test/java/org/apache/sysds/test/functions/builtin/BuiltinALSCGTest.java
##########
@@ -0,0 +1,50 @@
+package org.apache.sysds.test.functions.builtin;
+
+import org.apache.sysds.test.AutomatedTestBase;
+import org.apache.sysds.test.TestConfiguration;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class BuiltinALSCGTest extends AutomatedTestBase {
+
+    private final static String TEST_NAME = "als_cg";
+    private final static String TEST_DIR = "functions/builtin/";
+    private static final String TEST_CLASS_DIR = TEST_DIR + BuiltinALSCGTest.class.getSimpleName() + "/";
+
+    private final static int rows = 300;
+    private final static int cols = 20;
+
+    @Override
+    public void setUp() {
+        addTestConfiguration(TEST_NAME,new TestConfiguration(TEST_CLASS_DIR, TEST_NAME,new String[]{"B"}));
+    }
+
+    @Test
+    public void testALSCG() {
+        runtestALSCG();
+    }
+
+    private void runtestALSCG(){
+
+        loadTestConfiguration(getTestConfiguration(TEST_NAME));
+        String HOME = SCRIPT_DIR + TEST_DIR;
+        fullDMLScriptName = HOME + TEST_NAME + ".dml";
+        List<String> proArgs = new ArrayList<>();
+
+        proArgs.add("-explain");
+        proArgs.add("-stats");

Review comment:
       Once you are happy with the tests, then remove the -explain and -stats flag if you don't use their outputs.

##########
File path: src/test/java/org/apache/sysds/test/functions/builtin/BuiltinALSCGTest.java
##########
@@ -0,0 +1,50 @@
+package org.apache.sysds.test.functions.builtin;
+
+import org.apache.sysds.test.AutomatedTestBase;
+import org.apache.sysds.test.TestConfiguration;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class BuiltinALSCGTest extends AutomatedTestBase {
+
+    private final static String TEST_NAME = "als_cg";
+    private final static String TEST_DIR = "functions/builtin/";
+    private static final String TEST_CLASS_DIR = TEST_DIR + BuiltinALSCGTest.class.getSimpleName() + "/";
+
+    private final static int rows = 300;
+    private final static int cols = 20;
+
+    @Override
+    public void setUp() {
+        addTestConfiguration(TEST_NAME,new TestConfiguration(TEST_CLASS_DIR, TEST_NAME,new String[]{"B"}));
+    }
+
+    @Test
+    public void testALSCG() {
+        runtestALSCG();
+    }
+
+    private void runtestALSCG(){
+
+        loadTestConfiguration(getTestConfiguration(TEST_NAME));
+        String HOME = SCRIPT_DIR + TEST_DIR;
+        fullDMLScriptName = HOME + TEST_NAME + ".dml";
+        List<String> proArgs = new ArrayList<>();
+
+        proArgs.add("-explain");
+        proArgs.add("-stats");
+        proArgs.add("-args");
+        proArgs.add(input("X"));
+        proArgs.add(output("U"));
+        proArgs.add(output("V"));
+        programArgs = proArgs.toArray(new String[proArgs.size()]);
+
+        double[][] X = getRandomMatrix(rows, cols, 0, 1, 0.8, -1);
+        writeInputMatrixWithMTD("X", X, true);
+
+        runTest(true, EXCEPTION_NOT_EXPECTED, null, -1);

Review comment:
       You do not verify the output. you just run the code. 
   Please add some check to verify that the code actually outputs the correct results, or fits appropriately to the given problem.
   
   Since the test is executed with random data, then this might not make much sense, so you might want to make your own fictive problem that you know the algorithm solves.

##########
File path: src/test/scripts/functions/builtin/als_cg.dml
##########
@@ -0,0 +1,25 @@
+#-------------------------------------------------------------
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+#-------------------------------------------------------------
+
+X = read($1)
+[U, V] = als_cg(X=X)
+write(U, $2)
+write(V, $3)

Review comment:
       looks good.

##########
File path: src/test/java/org/apache/sysds/test/functions/builtin/BuiltinALSCGTest.java
##########
@@ -0,0 +1,50 @@
+package org.apache.sysds.test.functions.builtin;

Review comment:
       License

##########
File path: src/test/scripts/functions/builtin/als_ds.dml
##########
@@ -0,0 +1,25 @@
+#-------------------------------------------------------------
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+#-------------------------------------------------------------
+
+V = read($1)
+[L, R] = als_ds(V=V)
+write(L, $2)
+write(R, $3)

Review comment:
       also good.

##########
File path: scripts/builtin/als_cg.dml
##########
@@ -0,0 +1,172 @@
+#-------------------------------------------------------------
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+#-------------------------------------------------------------
+
+#  
+# THIS SCRIPT COMPUTES AN APPROXIMATE FACTORIZATION OF A LOW-RANK MATRIX X INTO TWO MATRICES U AND V
+# USING THE ALTERNATING-LEAST-SQUARES (ALS) ALGORITHM WITH CONJUGATE GRADIENT.
+# MATRICES U AND V ARE COMPUTED BY MINIMIZING A LOSS FUNCTION (WITH REGULARIZATION).
+#
+# INPUT   PARAMETERS:
+# ---------------------------------------------------------------------------------------------
+# NAME    TYPE     DEFAULT  MEANING
+# ---------------------------------------------------------------------------------------------
+# X       String   ---      Location to read the input matrix X to be factorized
+# U       String   ---      Location to write the factor matrix U
+# V       String   ---      Location to write the factor matrix V
+# rank    Int      10       Rank of the factorization
+# reg     String   "L2"	    Regularization: 
+#                           "L2" = L2 regularization;
+#                           "wL2" = weighted L2 regularization
+# lambda  Double   0.000001 Regularization parameter, no regularization if 0.0
+# maxi    Int      50       Maximum number of iterations
+# check   Boolean  TRUE     Check for convergence after every iteration, i.e., updating U and V once
+# thr     Double   0.0001   Assuming check is set to TRUE, the algorithm stops and convergence is declared 
+#                           if the decrease in loss in any two consecutive iterations falls below this threshold; 
+#                           if check is FALSE thr is ignored
+# fmt     String   "text"   The output format of the factor matrices L and R, such as "text" or "csv"
+# ---------------------------------------------------------------------------------------------
+# OUTPUT: 
+# 1- An m x r matrix U, where r is the factorization rank 
+# 2- An r x n matrix V
+#
+# HOW TO INVOKE THIS SCRIPT - EXAMPLE:
+# hadoop jar SystemDS.jar -f ALS-CG.dml -nvargs X=INPUT_DIR/X U=OUTPUT_DIR/U V=OUTPUT_DIR/V rank=10 reg="L2" lambda=0.0001 fmt=csv
+
+
+m_als_cg = function(Matrix[Double] X, Integer rank = 10, String reg = "L2", Double lambda = 0.000001, Integer maxi = 50, Boolean check = TRUE, Double thr = 0.0001, String fmt = "text")
+    return (Matrix[Double] U, Matrix[Double] V)
+    {
+
+    r = rank;
+    max_iter = maxi;
+    fmt0 = fmt;
+
+    ###### MAIN PART ######
+    m = nrow (X);
+    n = ncol (X);
+
+    # initializing factor matrices
+    U = rand (rows = m, cols = r, min = -0.5, max = 0.5); # mxr
+    V = rand (rows = n, cols = r, min = -0.5, max = 0.5); # nxr
+
+    W = (X != 0);
+
+    # check for regularization
+    row_nonzeros = matrix(0,rows=1,cols=1);
+    col_nonzeros = matrix(0,rows=1,cols=1);
+    if( reg == "L2" ) {
+      print ("BEGIN ALS-CG SCRIPT WITH NONZERO SQUARED LOSS + L2 WITH LAMBDA - " + lambda);
+      row_nonzeros = matrix(1, nrow(W), 1);
+      col_nonzeros = matrix(1, ncol(W), 1);
+    }
+    else if( reg == "wL2" ) {
+      print ("BEGIN ALS-CG SCRIPT WITH NONZERO SQUARED LOSS + WEIGHTED L2 WITH LAMBDA - " + lambda);
+      row_nonzeros = rowSums(W);
+      col_nonzeros = t(colSums(W));
+    }
+    else {
+      stop ("wrong regularization! " + reg);
+    }
+
+    # Loss Function with L2:
+    # f (U, V) = 0.5 * sum (W * (U %*% V - X) ^ 2)
+    #          + 0.5 * lambda * (sum (U ^ 2) + sum (V ^ 2))
+    # Loss Function with weighted L2:
+    # f (U, V) = 0.5 * sum (W * (U %*% V - X) ^ 2)
+    #          + 0.5 * lambda * (sum (U ^ 2 * row_nonzeros) + sum (V ^ 2 * col_nonzeros))
+
+    is_U = TRUE;  # TRUE = Optimize U, FALSE = Optimize V

Review comment:
       also should this be a parameter?

##########
File path: scripts/builtin/als_cg.dml
##########
@@ -0,0 +1,172 @@
+#-------------------------------------------------------------
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+#-------------------------------------------------------------
+
+#  
+# THIS SCRIPT COMPUTES AN APPROXIMATE FACTORIZATION OF A LOW-RANK MATRIX X INTO TWO MATRICES U AND V
+# USING THE ALTERNATING-LEAST-SQUARES (ALS) ALGORITHM WITH CONJUGATE GRADIENT.
+# MATRICES U AND V ARE COMPUTED BY MINIMIZING A LOSS FUNCTION (WITH REGULARIZATION).

Review comment:
       This description is all caps, while the other description is not..
   I would prefer all normal text. But i know some of the scripts does it differently.
   Please choose one.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org