Repository: systemml Updated Branches: refs/heads/master 4b615bc08 -> 20f97e0b5
[SYSTEMML-2024] Fix robustness codegen row output size handling This patch addresses perftest failures of Mlogreg icp 1 on the 1M x 1K scenario. Specific row operations with column range indexing on all side inputs lead to incorrect output sizes (inferred from the inputs according to the type of the row operator) and eventually index out of bounds exceptions on subsequent cell operators. We now explicitly compile the output size into the operator for types with potential ambiguity. The cplan comparison and hashing (for the plan cache) has been modified accordingly. Project: http://git-wip-us.apache.org/repos/asf/systemml/repo Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/5491c9de Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/5491c9de Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/5491c9de Branch: refs/heads/master Commit: 5491c9de3b77a45cf09320d4864f340e2be26f0f Parents: 4b615bc Author: Matthias Boehm <[email protected]> Authored: Mon Nov 20 21:04:41 2017 -0800 Committer: Matthias Boehm <[email protected]> Committed: Mon Nov 20 23:54:18 2017 -0800 ---------------------------------------------------------------------- .../sysml/hops/codegen/cplan/CNodeRow.java | 4 +- .../hops/codegen/template/TemplateRow.java | 4 +- .../sysml/runtime/codegen/SpoofRowwise.java | 12 +- .../instructions/spark/SpoofSPInstruction.java | 7 +- .../functions/codegen/AlgorithmMLogreg.java | 156 ++++++++++++++----- .../functions/codegen/Algorithm_MLogreg.R | 24 +-- .../functions/codegen/rowAggPattern30.dml | 2 + 7 files changed, 148 insertions(+), 61 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/systemml/blob/5491c9de/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeRow.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeRow.java b/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeRow.java index 41cf108..3e42346 100644 --- a/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeRow.java +++ b/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeRow.java @@ -81,6 +81,7 @@ public class CNodeRow extends CNodeTpl public void setConstDim2(long dim2) { _constDim2 = dim2; + _hash = 0; } public long getConstDim2() { @@ -191,7 +192,8 @@ public class CNodeRow extends CNodeTpl CNodeRow that = (CNodeRow)o; return super.equals(o) && _type == that._type - && _numVectors == that._numVectors + && _numVectors == that._numVectors + && _constDim2 == that._constDim2 && equalInputReferences( _output, that._output, _inputs, that._inputs); } http://git-wip-us.apache.org/repos/asf/systemml/blob/5491c9de/src/main/java/org/apache/sysml/hops/codegen/template/TemplateRow.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/codegen/template/TemplateRow.java b/src/main/java/org/apache/sysml/hops/codegen/template/TemplateRow.java index fcc3e93..8620971 100644 --- a/src/main/java/org/apache/sysml/hops/codegen/template/TemplateRow.java +++ b/src/main/java/org/apache/sysml/hops/codegen/template/TemplateRow.java @@ -51,7 +51,6 @@ import org.apache.sysml.hops.Hop.Direction; import org.apache.sysml.hops.Hop.OpOp1; import org.apache.sysml.hops.Hop.OpOp2; import org.apache.sysml.parser.Expression.DataType; -import org.apache.sysml.runtime.codegen.SpoofRowwise.RowType; import org.apache.sysml.runtime.matrix.data.LibMatrixMult; import org.apache.sysml.runtime.matrix.data.Pair; @@ -219,8 +218,7 @@ public class TemplateRow extends TemplateBase CNodeRow tpl = new CNodeRow(inputs, output); tpl.setRowType(TemplateUtils.getRowType(hop, inHops2.get("X"), inHops2.get("B1"))); - if( tpl.getRowType()==RowType.NO_AGG_CONST - || tpl.getRowType()==RowType.COL_AGG_CONST ) + if( tpl.getRowType().isConstDim2(hop.getDim2()) ) tpl.setConstDim2(hop.getDim2()); tpl.setNumVectorIntermediates(TemplateUtils .determineMinVectorIntermediates(output)); http://git-wip-us.apache.org/repos/asf/systemml/blob/5491c9de/src/main/java/org/apache/sysml/runtime/codegen/SpoofRowwise.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/runtime/codegen/SpoofRowwise.java b/src/main/java/org/apache/sysml/runtime/codegen/SpoofRowwise.java index 889ee1f..150b45e 100644 --- a/src/main/java/org/apache/sysml/runtime/codegen/SpoofRowwise.java +++ b/src/main/java/org/apache/sysml/runtime/codegen/SpoofRowwise.java @@ -69,7 +69,11 @@ public abstract class SpoofRowwise extends SpoofOperator } public boolean isRowTypeB1ColumnAgg() { return (this == COL_AGG_B1) || (this == COL_AGG_B1_T); - } + } + public boolean isConstDim2(long dim2) { + return (this == NO_AGG_CONST || this == COL_AGG_CONST) + || (dim2>0 && isRowTypeB1()); + } } protected final RowType _type; @@ -119,7 +123,7 @@ public abstract class SpoofRowwise extends SpoofOperator } public MatrixBlock execute(ArrayList<MatrixBlock> inputs, ArrayList<ScalarObject> scalarObjects, MatrixBlock out, boolean allocTmp, boolean aggIncr) - throws DMLRuntimeException + throws DMLRuntimeException { //sanity check if( inputs==null || inputs.size() < 1 || out==null ) @@ -128,7 +132,7 @@ public abstract class SpoofRowwise extends SpoofOperator //result allocation and preparations final int m = inputs.get(0).getNumRows(); final int n = inputs.get(0).getNumColumns(); - final int n2 = (_type==RowType.NO_AGG_CONST) ? (int)_constDim2 : + final int n2 = _type.isConstDim2(_constDim2) ? (int)_constDim2 : _type.isRowTypeB1() || hasMatrixSideInput(inputs) ? getMinColsMatrixSideInputs(inputs) : -1; if( !aggIncr || !out.isAllocated() ) @@ -184,7 +188,7 @@ public abstract class SpoofRowwise extends SpoofOperator //result allocation and preparations final int m = inputs.get(0).getNumRows(); final int n = inputs.get(0).getNumColumns(); - final int n2 = (_type==RowType.NO_AGG_CONST) ? (int)_constDim2 : + final int n2 = _type.isConstDim2(_constDim2) ? (int)_constDim2 : _type.isRowTypeB1() || hasMatrixSideInput(inputs) ? getMinColsMatrixSideInputs(inputs) : -1; allocateOutputMatrix(m, n, n2, out); http://git-wip-us.apache.org/repos/asf/systemml/blob/5491c9de/src/main/java/org/apache/sysml/runtime/instructions/spark/SpoofSPInstruction.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/runtime/instructions/spark/SpoofSPInstruction.java b/src/main/java/org/apache/sysml/runtime/instructions/spark/SpoofSPInstruction.java index 5058b0b..2f3eedb 100644 --- a/src/main/java/org/apache/sysml/runtime/instructions/spark/SpoofSPInstruction.java +++ b/src/main/java/org/apache/sysml/runtime/instructions/spark/SpoofSPInstruction.java @@ -133,7 +133,7 @@ public class SpoofSPInstruction extends SPInstruction { //execute generated operator if(_class.getSuperclass() == SpoofCellwise.class) //CELL { - SpoofCellwise op = (SpoofCellwise) CodegenUtils.createInstance(_class); + SpoofCellwise op = (SpoofCellwise) CodegenUtils.createInstance(_class); AggregateOperator aggop = getAggregateOperator(op.getAggOp()); if( _out.getDataType()==DataType.MATRIX ) { @@ -182,7 +182,7 @@ public class SpoofSPInstruction extends SPInstruction { OutProdType type = ((SpoofOuterProduct)op).getOuterProdType(); //update matrix characteristics - updateOutputMatrixCharacteristics(sec, op); + updateOutputMatrixCharacteristics(sec, op); MatrixCharacteristics mcOut = sec.getMatrixCharacteristics(_out.getName()); out = in.mapPartitionsToPair(new OuterProductFunction( @@ -212,8 +212,7 @@ public class SpoofSPInstruction extends SPInstruction { mcIn.getCols()+", ncolpb="+mcIn.getColsPerBlock()+"."); } SpoofRowwise op = (SpoofRowwise) CodegenUtils.createInstance(_class); - long clen2 = (op.getRowType()==RowType.NO_AGG_CONST - || op.getRowType()==RowType.COL_AGG_CONST) ? op.getConstDim2() : + long clen2 = op.getRowType().isConstDim2(op.getConstDim2()) ? op.getConstDim2() : op.getRowType().isRowTypeB1() ? sec.getMatrixCharacteristics(_in[1].getName()).getCols() : -1; RowwiseFunction fmmc = new RowwiseFunction(_class.getName(), _classBytes, bcVect2, bcMatrices, scalars, (int)mcIn.getCols(), (int)clen2); http://git-wip-us.apache.org/repos/asf/systemml/blob/5491c9de/src/test/java/org/apache/sysml/test/integration/functions/codegen/AlgorithmMLogreg.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/sysml/test/integration/functions/codegen/AlgorithmMLogreg.java b/src/test/java/org/apache/sysml/test/integration/functions/codegen/AlgorithmMLogreg.java index 2e06940..9c908b6 100644 --- a/src/test/java/org/apache/sysml/test/integration/functions/codegen/AlgorithmMLogreg.java +++ b/src/test/java/org/apache/sysml/test/integration/functions/codegen/AlgorithmMLogreg.java @@ -34,7 +34,7 @@ import org.apache.sysml.test.integration.TestConfiguration; import org.apache.sysml.test.utils.TestUtils; public class AlgorithmMLogreg extends AutomatedTestBase -{ +{ private final static String TEST_NAME1 = "Algorithm_MLogreg"; private final static String TEST_DIR = "functions/codegen/"; private final static String TEST_CLASS_DIR = TEST_DIR + AlgorithmMLogreg.class.getSimpleName() + "/"; @@ -43,13 +43,12 @@ public class AlgorithmMLogreg extends AutomatedTestBase private final static double eps = 1e-5; - private final static int rows = 3468; - private final static int cols = 327; + private final static int rows = 2468; + private final static int cols = 227; private final static double sparsity1 = 0.7; //dense private final static double sparsity2 = 0.1; //sparse - private final static int intercept = 0; private final static double epsilon = 0.000000001; private final static double maxiter = 10; @@ -60,86 +59,166 @@ public class AlgorithmMLogreg extends AutomatedTestBase } @Test - public void testMlogregBinDenseRewritesCP() { - runMlogregTest(TEST_NAME1, 2, true, false, ExecType.CP); + public void testMlogregBin0DenseRewritesCP() { + runMlogregTest(TEST_NAME1, 2, 0, true, false, ExecType.CP); } @Test - public void testMlogregBinSparseRewritesCP() { - runMlogregTest(TEST_NAME1, 2, true, true, ExecType.CP); + public void testMlogregBin0SparseRewritesCP() { + runMlogregTest(TEST_NAME1, 2, 0, true, true, ExecType.CP); } @Test - public void testMlogregBinDenseCP() { - runMlogregTest(TEST_NAME1, 2, false, false, ExecType.CP); + public void testMlogregBin0DenseCP() { + runMlogregTest(TEST_NAME1, 2, 0, false, false, ExecType.CP); } @Test - public void testMlogregBinSparseCP() { - runMlogregTest(TEST_NAME1, 2, false, true, ExecType.CP); + public void testMlogregBin0SparseCP() { + runMlogregTest(TEST_NAME1, 2, 0, false, true, ExecType.CP); } @Test - public void testMlogregMulDenseRewritesCP() { - runMlogregTest(TEST_NAME1, 5, true, false, ExecType.CP); + public void testMlogregMul0DenseRewritesCP() { + runMlogregTest(TEST_NAME1, 5, 0, true, false, ExecType.CP); } @Test - public void testMlogregMulSparseRewritesCP() { - runMlogregTest(TEST_NAME1, 5, true, true, ExecType.CP); + public void testMlogregMul0SparseRewritesCP() { + runMlogregTest(TEST_NAME1, 5, 0, true, true, ExecType.CP); } @Test - public void testMlogregMulDenseCP() { - runMlogregTest(TEST_NAME1, 5, false, false, ExecType.CP); + public void testMlogregMul0DenseCP() { + runMlogregTest(TEST_NAME1, 5, 0, false, false, ExecType.CP); } @Test - public void testMlogregMulSparseCP() { - runMlogregTest(TEST_NAME1, 5, false, true, ExecType.CP); + public void testMlogregMul0SparseCP() { + runMlogregTest(TEST_NAME1, 5, 0, false, true, ExecType.CP); } @Test - public void testMlogregBinDenseRewritesSP() { - runMlogregTest(TEST_NAME1, 2, true, false, ExecType.SPARK); + public void testMlogregBin0DenseRewritesSP() { + runMlogregTest(TEST_NAME1, 2, 0, true, false, ExecType.SPARK); } @Test - public void testMlogregBinSparseRewritesSP() { - runMlogregTest(TEST_NAME1, 2, true, true, ExecType.SPARK); + public void testMlogregBin0SparseRewritesSP() { + runMlogregTest(TEST_NAME1, 2, 0, true, true, ExecType.SPARK); } @Test - public void testMlogregBinDenseSP() { - runMlogregTest(TEST_NAME1, 2, false, false, ExecType.SPARK); + public void testMlogregBin0DenseSP() { + runMlogregTest(TEST_NAME1, 2, 0, false, false, ExecType.SPARK); } @Test - public void testMlogregBinSparseSP() { - runMlogregTest(TEST_NAME1, 2, false, true, ExecType.SPARK); + public void testMlogregBin0SparseSP() { + runMlogregTest(TEST_NAME1, 2, 0, false, true, ExecType.SPARK); } @Test - public void testMlogregMulDenseRewritesSP() { - runMlogregTest(TEST_NAME1, 5, true, false, ExecType.SPARK); + public void testMlogregMul0DenseRewritesSP() { + runMlogregTest(TEST_NAME1, 5, 0, true, false, ExecType.SPARK); } @Test - public void testMlogregMulSparseRewritesSP() { - runMlogregTest(TEST_NAME1, 5, true, true, ExecType.SPARK); + public void testMlogregMul0SparseRewritesSP() { + runMlogregTest(TEST_NAME1, 5, 0, true, true, ExecType.SPARK); } @Test - public void testMlogregMulDenseSP() { - runMlogregTest(TEST_NAME1, 5, false, false, ExecType.SPARK); + public void testMlogregMul0DenseSP() { + runMlogregTest(TEST_NAME1, 5, 0, false, false, ExecType.SPARK); } @Test - public void testMlogregMulSparseSP() { - runMlogregTest(TEST_NAME1, 5, false, true, ExecType.SPARK); + public void testMlogregMul0SparseSP() { + runMlogregTest(TEST_NAME1, 5, 0, false, true, ExecType.SPARK); } - private void runMlogregTest( String testname, int classes, boolean rewrites, boolean sparse, ExecType instType) + @Test + public void testMlogregBin1DenseRewritesCP() { + runMlogregTest(TEST_NAME1, 2, 1, true, false, ExecType.CP); + } + + @Test + public void testMlogregBin1SparseRewritesCP() { + runMlogregTest(TEST_NAME1, 2, 1, true, true, ExecType.CP); + } + + @Test + public void testMlogregBin1DenseCP() { + runMlogregTest(TEST_NAME1, 2, 1, false, false, ExecType.CP); + } + + @Test + public void testMlogregBin1SparseCP() { + runMlogregTest(TEST_NAME1, 2, 1, false, true, ExecType.CP); + } + + @Test + public void testMlogregMul1DenseRewritesCP() { + runMlogregTest(TEST_NAME1, 5, 1, true, false, ExecType.CP); + } + + @Test + public void testMlogregMul1SparseRewritesCP() { + runMlogregTest(TEST_NAME1, 5, 1, true, true, ExecType.CP); + } + + @Test + public void testMlogregMul1DenseCP() { + runMlogregTest(TEST_NAME1, 5, 1, false, false, ExecType.CP); + } + + @Test + public void testMlogregMul1SparseCP() { + runMlogregTest(TEST_NAME1, 5, 1, false, true, ExecType.CP); + } + + @Test + public void testMlogregBin2DenseRewritesCP() { + runMlogregTest(TEST_NAME1, 2, 2, true, false, ExecType.CP); + } + + @Test + public void testMlogregBin2SparseRewritesCP() { + runMlogregTest(TEST_NAME1, 2, 2, true, true, ExecType.CP); + } + + @Test + public void testMlogregBin2DenseCP() { + runMlogregTest(TEST_NAME1, 2, 2, false, false, ExecType.CP); + } + + @Test + public void testMlogregBin2SparseCP() { + runMlogregTest(TEST_NAME1, 2, 2, false, true, ExecType.CP); + } + + @Test + public void testMlogregMul2DenseRewritesCP() { + runMlogregTest(TEST_NAME1, 5, 2, true, false, ExecType.CP); + } + + @Test + public void testMlogregMul2SparseRewritesCP() { + runMlogregTest(TEST_NAME1, 5, 2, true, true, ExecType.CP); + } + + @Test + public void testMlogregMul2DenseCP() { + runMlogregTest(TEST_NAME1, 5, 2, false, false, ExecType.CP); + } + + @Test + public void testMlogregMul2SparseCP() { + runMlogregTest(TEST_NAME1, 5, 2, false, true, ExecType.CP); + } + + private void runMlogregTest( String testname, int classes, int intercept, boolean rewrites, boolean sparse, ExecType instType) { boolean oldFlag = OptimizerUtils.ALLOW_ALGEBRAIC_SIMPLIFICATION; RUNTIME_PLATFORM platformOld = rtplatform; @@ -182,7 +261,8 @@ public class AlgorithmMLogreg extends AutomatedTestBase HashMap<CellIndex, Double> dmlfile = readDMLMatrixFromHDFS("w"); HashMap<CellIndex, Double> rfile = readRMatrixFromFS("w"); TestUtils.compareMatrices(dmlfile, rfile, eps, "Stat-DML", "Stat-R"); - Assert.assertTrue(heavyHittersContainsSubString("spoof") || heavyHittersContainsSubString("sp_spoof")); + Assert.assertTrue(heavyHittersContainsSubString("spoof") + || heavyHittersContainsSubString("sp_spoof")); } finally { rtplatform = platformOld; http://git-wip-us.apache.org/repos/asf/systemml/blob/5491c9de/src/test/scripts/functions/codegen/Algorithm_MLogreg.R ---------------------------------------------------------------------- diff --git a/src/test/scripts/functions/codegen/Algorithm_MLogreg.R b/src/test/scripts/functions/codegen/Algorithm_MLogreg.R index 121aba7..0321501 100644 --- a/src/test/scripts/functions/codegen/Algorithm_MLogreg.R +++ b/src/test/scripts/functions/codegen/Algorithm_MLogreg.R @@ -61,13 +61,15 @@ if (intercept_status == 1 | intercept_status == 2) if (intercept_status == 2) # scale-&-shift X columns to mean 0, variance 1 { # Important assumption: X [, D] = matrix (1, rows = N, cols = 1) - avg_X_cols = t(colSums(X)) / N; - var_X_cols = (t(colSums (X ^ 2)) - N * (avg_X_cols ^ 2)) / (N - 1); + avg_X_cols = colSums(X) / N; + var_X_cols = (colSums (X ^ 2) - N * (avg_X_cols ^ 2)) / (N - 1); is_unsafe = (var_X_cols <= 0.0); scale_X = 1.0 / sqrt (var_X_cols * (1 - is_unsafe) + is_unsafe); - scale_X [D, 1] = 1; + scale_X [D] = 1; shift_X = - avg_X_cols * scale_X; - shift_X [D, 1] = 0; + shift_X [D] = 0; + scale_X = as.matrix(scale_X); + shift_X = as.matrix(shift_X); rowSums_X_sq = (X ^ 2) %*% (scale_X ^ 2) + X %*% (2 * scale_X * shift_X) + sum (shift_X ^ 2); } else { scale_X = matrix (1, D, 1); @@ -110,7 +112,7 @@ obj = N * log (K + 1); ### obj = - sum (Y * LT) + sum (log (row Grad = t(X) %*% (P [, 1:K] - Y [, 1:K]); if (intercept_status == 2) { - Grad = diag (scale_X) %*% Grad + shift_X %*% Grad [D, ]; + Grad = diag (as.vector(scale_X)) %*% Grad + shift_X %*% t(Grad [D, ]); } Grad = Grad + lambda * B; norm_Grad = sqrt (sum (Grad ^ 2)); @@ -141,7 +143,7 @@ while (! converge) while (! innerconverge) { if (intercept_status == 2) { - ssX_V = diag (scale_X) %*% V; + ssX_V = diag (as.vector(scale_X)) %*% V; ssX_V [D, ] = ssX_V [D, ] + t(shift_X) %*% V; } else { ssX_V = V; @@ -149,7 +151,7 @@ while (! converge) Q = P [, 1:K] * (X %*% ssX_V); HV = t(X) %*% (Q - P [, 1:K] * (rowSums (Q) %*% matrix (1, 1, K))); if (intercept_status == 2) { - HV = diag (scale_X) %*% HV + shift_X %*% HV [D, ]; + HV = diag (as.vector(scale_X)) %*% HV + shift_X %*% HV [D, ]; } HV = HV + lambda * V; alpha = norm_R2 / sum (V * HV); @@ -189,8 +191,8 @@ while (! converge) qk = - 0.5 * (gs - sum (S * R)); B_new = B + S; if (intercept_status == 2) { - ssX_B_new = diag (scale_X) %*% B_new; - ssX_B_new [D, ] = ssX_B_new [D, ] + t(shift_X) %*% B_new; + ssX_B_new = diag (as.vector(scale_X)) %*% B_new; + ssX_B_new [D, ] = ssX_B_new [D, ] + t(shift_X) %*% B_new; } else { ssX_B_new = B_new; } @@ -254,7 +256,7 @@ while (! converge) P = P_new; Grad = t(X) %*% (P [, 1:K] - Y [, 1:K]); if (intercept_status == 2) { - Grad = diag (scale_X) %*% Grad + shift_X %*% Grad [D, ]; + Grad = diag (as.vector(scale_X)) %*% Grad + shift_X %*% t(Grad [D, ]); } Grad = Grad + lambda * B; norm_Grad = sqrt (sum (Grad ^ 2)); @@ -269,7 +271,7 @@ while (! converge) } if (intercept_status == 2) { - B_out = diag (scale_X) %*% B; + B_out = diag (as.vector(scale_X)) %*% B; B_out [D, ] = B_out [D, ] + t(shift_X) %*% B; } else { B_out = B; http://git-wip-us.apache.org/repos/asf/systemml/blob/5491c9de/src/test/scripts/functions/codegen/rowAggPattern30.dml ---------------------------------------------------------------------- diff --git a/src/test/scripts/functions/codegen/rowAggPattern30.dml b/src/test/scripts/functions/codegen/rowAggPattern30.dml index 7df215d..07f5e5d 100644 --- a/src/test/scripts/functions/codegen/rowAggPattern30.dml +++ b/src/test/scripts/functions/codegen/rowAggPattern30.dml @@ -28,5 +28,7 @@ K = 4; while(FALSE){} Q = P[,1:K] * (X %*% ssX_V); R = t(X) %*% (Q - P[,1:K] * rowSums(Q)); +if( nrow(R)!=10 | ncol(R)!=4 ) + stop("Wrong output dimensions."); write(R, $1)
