You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@systemml.apache.org by GitBox <gi...@apache.org> on 2020/05/17 20:15:00 UTC
[GitHub] [systemml] mboehm7 commented on a change in pull request #910: [MINOR] L2SVM, now directly support 1 Hot input
mboehm7 commented on a change in pull request #910:
URL: https://github.com/apache/systemml/pull/910#discussion_r426299182
##########
File path: scripts/builtin/l2svm.dml
##########
@@ -44,58 +47,62 @@
m_l2svm = function(Matrix[Double] X, Matrix[Double] Y, Boolean intercept = FALSE,
-Double epsilon = 0.001, Double lambda = 1, Integer maxiterations = 100, Boolean verbose = FALSE)
+ Double epsilon = 0.001, Double lambda = 1, Integer max_iterations = 100,
+ Boolean verbose = FALSE, Integer column_id = -1)
Review comment:
This Python-like indentation is a bit unpleasant to the eye.
##########
File path: scripts/builtin/confusionMatrix.dml
##########
@@ -0,0 +1,51 @@
+#-------------------------------------------------------------
+#
+# 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.
+#
+#-------------------------------------------------------------
+
+# INPUT PARAMETERS:
+# ---------------------------------------------------------------------------------------------
+# NAME TYPE DEFAULT MEANING
+# ---------------------------------------------------------------------------------------------
+# P Double --- matrix of Predictions
+# Y Double --- matrix of Golden standard One Hot Encoded
+# ---------------------------------------------------------------------------------------------
+# OUTPUT:
+# ---------------------------------------------------------------------------------------------
+# NAME TYPE DEFAULT MEANING
+# ---------------------------------------------------------------------------------------------
+# ConfusionMatrix Double --- The Confusion Matrix
+
+
+m_confusionMatrix = function(Matrix[Double] P, Matrix[Double] Y)
+ return(Matrix[Double] confusion)
+{
+ if(nrow(P) != nrow(Y))
+ stop("CONFUSION MATRIX: The number of rows have to be equal in both P ["+nrow(P)+"] and Y ["+nrow(Y)+"]")
+
+ res = matrix(0, max(P), max(Y))
+ for(i in 1:nrow(P) ){
+ row = as.scalar(Y[i,1])
+ col = as.scalar(P[i,1])
+
+ res[row,col] = as.scalar(res[row,col]) + 1
+ }
Review comment:
Please replace the entire for loop with a `table(P[1,], Y[1,])` (it's fine to have a remapping to table). Also note that the construction of `res` use inconsistent lhs/rhs matrices (P/Y vs Y/P).
##########
File path: scripts/builtin/l2svm.dml
##########
@@ -112,32 +119,37 @@ Double epsilon = 0.001, Double lambda = 1, Integer maxiterations = 100, Boolean
step_sz = step_sz - g/h
continue1 = (g*g/h >= epsilon)
}
-
+
#update weights
w = w + step_sz*s
Xw = Xw + step_sz*Xd
-
+
out = 1 - Y * Xw
sv = (out > 0)
out = sv * out
obj = 0.5 * sum(out * out) + lambda/2 * sum(w * w)
g_new = t(X) %*% (out * Y) - lambda * w
-
-
- if(verbose) print("Iter, Obj "+ iter + ", "+obj)
-
+
+ if(verbose) {
+ if(column_id != -1){
+ print("Iter:" + toString(iter) + ", Col:" + column_id + ", Obj:" + obj)
+ }
+ else {
+ print("Iter:" + toString(iter) + ", Obj:" + obj)
+ }
+ }
+
tmp = sum(s * g_old)
if(step_sz*tmp < epsilon*obj){
continue = 0
}
Review comment:
Somehow I missed this on the initial commit, but this can become: `continue = (step_sz*tmp >= epsilon*obj & sum(s^2) != 0);`
##########
File path: scripts/builtin/l2svm.dml
##########
@@ -112,32 +119,37 @@ Double epsilon = 0.001, Double lambda = 1, Integer maxiterations = 100, Boolean
step_sz = step_sz - g/h
continue1 = (g*g/h >= epsilon)
}
-
+
#update weights
w = w + step_sz*s
Xw = Xw + step_sz*Xd
-
+
out = 1 - Y * Xw
sv = (out > 0)
out = sv * out
obj = 0.5 * sum(out * out) + lambda/2 * sum(w * w)
g_new = t(X) %*% (out * Y) - lambda * w
-
-
- if(verbose) print("Iter, Obj "+ iter + ", "+obj)
-
+
+ if(verbose) {
+ if(column_id != -1){
+ print("Iter:" + toString(iter) + ", Col:" + column_id + ", Obj:" + obj)
+ }
+ else {
+ print("Iter:" + toString(iter) + ", Obj:" + obj)
+ }
Review comment:
could we maybe replace this with something more concise like:
```
if(verbose) {
colstr = ifelse(column_id!=-1, ", Col:"+column_id, "")
print("Iter:" + toString(iter) + colstr + ", Obj:" + obj)
}
```
##########
File path: scripts/builtin/msvm.dml
##########
@@ -42,34 +43,45 @@
# ---------------------------------------------------------------------------------------------
# model Double --- model matrix
-m_msvm = function(Matrix[Double] X, Matrix[Double] Y, Boolean intercept = FALSE, Integer num_classes =10,
+m_msvm = function(Matrix[Double] X, Matrix[Double] Y, Boolean intercept = FALSE,
Double epsilon = 0.001, Double lambda = 1.0, Integer max_iterations = 100, Boolean verbose = FALSE)
return(Matrix[Double] model)
{
if(verbose)
print("Built-in Multiclass-SVM started")
- num_samples = nrow(X)
- num_features = ncol(X)
-
- num_rows_in_w = num_features
+ num_rows_in_w = ncol(X)
if(intercept) {
num_rows_in_w = num_rows_in_w + 1
}
-
- w = matrix(0, rows=num_rows_in_w, cols=num_classes)
- parfor(iter_class in 1:num_classes) {
- Y_local = 2 * (Y == iter_class) - 1
- if(verbose) {
- print("iter class: " + iter_class)
- print("y local: " + toString(Y_local))
+ if(ncol(Y) > 1){
+ w = matrix(0, rows=num_rows_in_w, cols=ncol(Y))
+ # If multiple columns, assume the input to be OneHotEncoded already.
+ if((min(Y) != 0 | min(Y) != -1) & max(Y) != 1)
+ stop("MSVM: Invalid OneHot input matrix Y")
+
+ parfor(class in 1:ncol(Y)){
+ w[,class] = l2svm(X=X, Y=Y[,class], intercept=intercept,
+ epsilon=epsilon, lambda=lambda, max_iterations=max_iterations,
+ verbose= verbose, column_id=class)
}
Review comment:
Instead of replicating this code of running the core of msvm, please simply modify Y as follows:
```
if( ncol(Y) > 1 )
Y = rowMax(Y * t(seq(1,ncol(Y))))
```
##########
File path: src/test/java/org/apache/sysds/test/functions/builtin/BuiltinMulticlassSVMTest.java
##########
@@ -99,16 +98,16 @@ private void runMSVMTest(boolean sparse, boolean intercept, int classes, double
double sparsity = sparse ? spSparse : spDense;
String HOME = SCRIPT_DIR + TEST_DIR;
fullDMLScriptName = HOME + TEST_NAME + ".dml";
- programArgs = new String[]{ "-explain", "-stats",
+ programArgs = new String[]{
"-nvargs", "X=" + input("X"), "Y=" + input("Y"), "model=" + output("model"),
- "inc=" + String.valueOf(intercept).toUpperCase(), "num_classes=" + classes, "eps=" + eps, "lam=" + lambda, "max=" + run};
+ "inc=" + String.valueOf(intercept).toUpperCase(), "eps=" + eps, "lam=" + lambda, "max=" + run};
fullRScriptName = HOME + TEST_NAME + ".R";
- rCmd = getRCmd(inputDir(), Boolean.toString(intercept), Integer.toString(classes), Double.toString(eps),
+ rCmd = getRCmd(inputDir(), Boolean.toString(intercept), Double.toString(eps),
Review comment:
The test is currently failing because the removed parameter shifts the output filename from position 7 to 6 but the R script is not updated consistently.
##########
File path: src/test/java/org/apache/sysds/test/functions/jmlc/ReuseModelVariablesTest.java
##########
@@ -35,7 +35,7 @@
public class ReuseModelVariablesTest extends AutomatedTestBase
{
private final static String TEST_NAME1 = "reuse-glm-predict";
- private final static String TEST_NAME2 = "reuse-msvm-predict";
+ private final static String TEST_NAME2 = "reuse--predict";
Review comment:
why?
##########
File path: src/test/java/org/apache/sysds/test/functions/builtin/BuiltinMulticlassSVMTest.java
##########
@@ -53,38 +52,38 @@ public void setUp() {
@Test
public void testMSVMDense() {
- runMSVMTest(false, false, num_classes, eps, 1.0, max_iter, LopProperties.ExecType.CP);
+ runMSVMTest(false, false, eps, 1.0, max_iter, LopProperties.ExecType.CP);
}
@Test
public void testMSVMSparse() {
- runMSVMTest(true, false, num_classes, eps, 1.0, max_iter, LopProperties.ExecType.CP);
+ runMSVMTest(true, false, eps, 1.0, max_iter, LopProperties.ExecType.CP);
}
@Test
public void testMSVMInterceptSpark() {
- runMSVMTest(true,true, num_classes, eps, 1.0, max_iter, LopProperties.ExecType.SPARK);
+ runMSVMTest(true,true, eps, 1.0, max_iter, LopProperties.ExecType.SPARK);
}
@Test
public void testMSVMSparseLambda2() {
- runMSVMTest(true,true, num_classes, eps,2.0, max_iter, LopProperties.ExecType.CP);
+ runMSVMTest(true,true, eps,2.0, max_iter, LopProperties.ExecType.CP);
}
@Test
public void testMSVMSparseLambda100CP() {
- runMSVMTest(true,true, num_classes, 1, 100, max_iter, LopProperties.ExecType.CP);
+ runMSVMTest(true,true, 1, 100, max_iter, LopProperties.ExecType.CP);
}
@Test
public void testMSVMSparseLambda100Spark() {
- runMSVMTest(true,true, num_classes, 1, 100, max_iter, LopProperties.ExecType.SPARK);
+ runMSVMTest(true,true, 1, 100, max_iter, LopProperties.ExecType.SPARK);
}
@Test
public void testMSVMIteration() {
- runMSVMTest(true,true, num_classes, 1, 2.0, 100, LopProperties.ExecType.CP);
+ runMSVMTest(true,true, 1, 2.0, 100, LopProperties.ExecType.CP);
}
@Test
public void testMSVMDenseIntercept() {
- runMSVMTest(false,true, num_classes, eps, 1.0, max_iter, LopProperties.ExecType.CP);
+ runMSVMTest(false,true, eps, 1.0, max_iter, LopProperties.ExecType.CP);
}
- private void runMSVMTest(boolean sparse, boolean intercept, int classes, double eps,
+ private void runMSVMTest(boolean sparse, boolean intercept, double eps,
double lambda, int run, LopProperties.ExecType instType)
Review comment:
again, personally I would use a single indentation level for parameters (as people read from left to right) but I don't have a strong opinion here.
----------------------------------------------------------------
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