Repository: systemml Updated Branches: refs/heads/master 5315e1d2d -> 0bcae49ff
[SYSTEMML-1662] Extend HopDagValidator Closes #534. Project: http://git-wip-us.apache.org/repos/asf/systemml/repo Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/0bcae49f Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/0bcae49f Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/0bcae49f Branch: refs/heads/master Commit: 0bcae49ffea047452c99d5f260775323abf6b6e2 Parents: 5315e1d Author: Dylan Hutchison <[email protected]> Authored: Sat Jun 10 00:34:15 2017 -0700 Committer: Matthias Boehm <[email protected]> Committed: Sat Jun 10 00:34:15 2017 -0700 ---------------------------------------------------------------------- .../java/org/apache/sysml/hops/AggBinaryOp.java | 7 +- .../java/org/apache/sysml/hops/AggUnaryOp.java | 7 +- .../java/org/apache/sysml/hops/BinaryOp.java | 5 + .../org/apache/sysml/hops/ConvolutionOp.java | 5 + .../java/org/apache/sysml/hops/DataGenOp.java | 13 ++- src/main/java/org/apache/sysml/hops/DataOp.java | 29 +++++- .../java/org/apache/sysml/hops/FunctionOp.java | 6 +- src/main/java/org/apache/sysml/hops/Hop.java | 16 ++- .../org/apache/sysml/hops/HopsException.java | 23 +++++ .../java/org/apache/sysml/hops/IndexingOp.java | 8 +- .../org/apache/sysml/hops/LeftIndexingOp.java | 6 +- .../java/org/apache/sysml/hops/LiteralOp.java | 6 +- .../java/org/apache/sysml/hops/MultipleOp.java | 4 + .../sysml/hops/ParameterizedBuiltinOp.java | 7 ++ .../org/apache/sysml/hops/QuaternaryOp.java | 8 +- .../java/org/apache/sysml/hops/ReorgOp.java | 20 +++- .../java/org/apache/sysml/hops/TernaryOp.java | 20 +++- .../java/org/apache/sysml/hops/UnaryOp.java | 5 + .../apache/sysml/hops/codegen/SpoofFusedOp.java | 5 +- .../sysml/hops/rewrite/HopDagValidator.java | 103 +++++++++++-------- .../sysml/hops/rewrite/ProgramRewriter.java | 30 +++--- 21 files changed, 257 insertions(+), 76 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/AggBinaryOp.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/AggBinaryOp.java b/src/main/java/org/apache/sysml/hops/AggBinaryOp.java index ac17ef2..21dbbf1 100644 --- a/src/main/java/org/apache/sysml/hops/AggBinaryOp.java +++ b/src/main/java/org/apache/sysml/hops/AggBinaryOp.java @@ -115,7 +115,12 @@ public class AggBinaryOp extends Hop implements MultiThreadedHop //compute unknown dims and nnz refreshSizeInformation(); } - + + @Override + public void checkArity() throws HopsException { + HopsException.check(_input.size() == 2, this, "should have arity 2 but has arity %d", _input.size()); + } + public void setHasLeftPMInput(boolean flag) { _hasLeftPMInput = flag; } http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/AggUnaryOp.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/AggUnaryOp.java b/src/main/java/org/apache/sysml/hops/AggUnaryOp.java index 70a71bc..ee4ded2 100644 --- a/src/main/java/org/apache/sysml/hops/AggUnaryOp.java +++ b/src/main/java/org/apache/sysml/hops/AggUnaryOp.java @@ -72,7 +72,12 @@ public class AggUnaryOp extends Hop implements MultiThreadedHop getInput().add(0, inp); inp.getParent().add(this); } - + + @Override + public void checkArity() throws HopsException { + HopsException.check(_input.size() == 1, this, "should have arity 1 but has arity %d", _input.size()); + } + public AggOp getOp() { return _op; http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/BinaryOp.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/BinaryOp.java b/src/main/java/org/apache/sysml/hops/BinaryOp.java index ccf73c4..f3a2fa0 100644 --- a/src/main/java/org/apache/sysml/hops/BinaryOp.java +++ b/src/main/java/org/apache/sysml/hops/BinaryOp.java @@ -111,6 +111,11 @@ public class BinaryOp extends Hop refreshSizeInformation(); } + @Override + public void checkArity() throws HopsException { + HopsException.check(_input.size() == 2, this, "should have arity 2 but has arity %d", _input.size()); + } + public OpOp2 getOp() { return op; } http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/ConvolutionOp.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/ConvolutionOp.java b/src/main/java/org/apache/sysml/hops/ConvolutionOp.java index 7ab2988..8303e73 100644 --- a/src/main/java/org/apache/sysml/hops/ConvolutionOp.java +++ b/src/main/java/org/apache/sysml/hops/ConvolutionOp.java @@ -59,6 +59,11 @@ public class ConvolutionOp extends Hop implements MultiThreadedHop refreshSizeInformation(); } + @Override + public void checkArity() throws HopsException { + HopsException.check(_input.size() >= 1, this, "should have at least one input but has %d inputs", _input.size()); + } + public ConvOp getOp() { return op; http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/DataGenOp.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/DataGenOp.java b/src/main/java/org/apache/sysml/hops/DataGenOp.java index 64948c9..3b3dc44 100644 --- a/src/main/java/org/apache/sysml/hops/DataGenOp.java +++ b/src/main/java/org/apache/sysml/hops/DataGenOp.java @@ -38,6 +38,10 @@ import org.apache.sysml.parser.Expression.ValueType; import org.apache.sysml.parser.Statement; import org.apache.sysml.runtime.controlprogram.parfor.ProgramConverter; +/** + * A DataGenOp can be rand (or matrix constructor), sequence, and sample - + * these operators have different parameters and use a map of parameter type to hop position. + */ public class DataGenOp extends Hop implements MultiThreadedHop { @@ -113,7 +117,14 @@ public class DataGenOp extends Hop implements MultiThreadedHop //compute unknown dims and nnz refreshSizeInformation(); } - + + @Override + public void checkArity() throws HopsException { + int sz = _input.size(); + int pz = _paramIndexMap.size(); + HopsException.check(sz == pz, this, "has %d inputs but %d parameters", sz, pz); + } + @Override public String getOpString() { return "dg(" + _op.toString().toLowerCase() +")"; http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/DataOp.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/DataOp.java b/src/main/java/org/apache/sysml/hops/DataOp.java index 36e72f3..bba0898 100644 --- a/src/main/java/org/apache/sysml/hops/DataOp.java +++ b/src/main/java/org/apache/sysml/hops/DataOp.java @@ -34,7 +34,10 @@ import org.apache.sysml.runtime.controlprogram.caching.MatrixObject.UpdateType; import org.apache.sysml.runtime.matrix.MatrixCharacteristics; import org.apache.sysml.runtime.util.LocalFileUtils; - +/** + * A DataOp can be either a persistent read/write or transient read/write - writes will always have at least one input, + * but all types can have parameters (e.g., for csv literals of delimiter, header, etc). + */ public class DataOp extends Hop { private DataOpTypes _dataop; @@ -182,7 +185,29 @@ public class DataOp extends Hop if (dop == DataOpTypes.TRANSIENTWRITE) setInputFormatType(FileFormatTypes.BINARY); } - + + /** Check for N (READ) or N+1 (WRITE) inputs. */ + @Override + public void checkArity() throws HopsException { + int sz = _input.size(); + int pz = _paramIndexMap.size(); + switch (_dataop) { + case PERSISTENTREAD: + case TRANSIENTREAD: + HopsException.check(sz == pz, this, + "in %s operator type has %d inputs and %d parameters", + _dataop.name(), sz, pz); + break; + case PERSISTENTWRITE: + case TRANSIENTWRITE: + case FUNCTIONOUTPUT: + HopsException.check(sz == pz + 1, this, + "in %s operator type has %d inputs and %d parameters (expect 1 more input for write operator type)", + _dataop.name(), sz, pz); + break; + } + } + public DataOpTypes getDataOpType() { return _dataop; http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/FunctionOp.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/FunctionOp.java b/src/main/java/org/apache/sysml/hops/FunctionOp.java index 03db4da..c677bb8 100644 --- a/src/main/java/org/apache/sysml/hops/FunctionOp.java +++ b/src/main/java/org/apache/sysml/hops/FunctionOp.java @@ -79,7 +79,11 @@ public class FunctionOp extends Hop in.getParent().add(this); } } - + + /** FunctionOps may have any number of inputs. */ + @Override + public void checkArity() throws HopsException {} + public String getFunctionNamespace() { return _fnamespace; http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/Hop.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/Hop.java b/src/main/java/org/apache/sysml/hops/Hop.java index 742f22a..6541edc 100644 --- a/src/main/java/org/apache/sysml/hops/Hop.java +++ b/src/main/java/org/apache/sysml/hops/Hop.java @@ -78,8 +78,8 @@ public abstract class Hop protected long _nnz = -1; protected UpdateType _updateType = UpdateType.COPY; - protected ArrayList<Hop> _parent = new ArrayList<Hop>(); - protected ArrayList<Hop> _input = new ArrayList<Hop>(); + protected ArrayList<Hop> _parent = new ArrayList<>(); + protected ArrayList<Hop> _input = new ArrayList<>(); protected ExecType _etype = null; //currently used exec type protected ExecType _etypeForced = null; //exec type forced via platform or external optimizer @@ -135,6 +135,18 @@ public abstract class Hop public long getHopID() { return _ID; } + + /** + * Check whether this Hop has a correct number of inputs. + * + * (Some Hops can have a variable number of inputs, such as DataOp, DataGenOp, ParameterizedBuiltinOp, + * ReorgOp, TernaryOp, QuaternaryOp, MultipleOp, ConvolutionOp, and SpoofFusedOp.) + * + * Parameterized Hops (such as DataOp) can check that the number of parameters matches the number of inputs. + * + * @throws HopsException if this Hop has an illegal number of inputs (a kind of Illegal State) + */ + public abstract void checkArity() throws HopsException; public ExecType getExecType() { http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/HopsException.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/HopsException.java b/src/main/java/org/apache/sysml/hops/HopsException.java index eb62abf..17d9a09 100644 --- a/src/main/java/org/apache/sysml/hops/HopsException.java +++ b/src/main/java/org/apache/sysml/hops/HopsException.java @@ -45,4 +45,27 @@ public class HopsException extends DMLException super(message, cause); } + /** + * If the condition fails, print the message formatted with objects. + * @param condition Condition to test + * @param message Message to print if the condition fails + * @param objects Objects to print with the message, as per String.format + * @throws HopsException Thrown if condition is false + */ + public static void check(boolean condition, String message, Object... objects) throws HopsException { + if (!condition) + throw new HopsException(String.format(message, objects)); + } + /** + * If the condition fails, print the Op and its Id, along with the message formatted with objects. + * @param condition Condition to test + * @param hop Hop to print as a cause of the problem, if the condition fails + * @param message Message to print if the condition fails + * @param objects Objects to print with the message, as per String.format + * @throws HopsException Thrown if condition is false + */ + public static void check(boolean condition, Hop hop, String message, Object... objects) throws HopsException { + if (!condition) + throw new HopsException(String.format(hop.getOpString()+" id="+hop.getHopID()+" "+message, objects)); + } } http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/IndexingOp.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/IndexingOp.java b/src/main/java/org/apache/sysml/hops/IndexingOp.java index b77947f..e446552 100644 --- a/src/main/java/org/apache/sysml/hops/IndexingOp.java +++ b/src/main/java/org/apache/sysml/hops/IndexingOp.java @@ -74,8 +74,12 @@ public class IndexingOp extends Hop setRowLowerEqualsUpper(passedRowsLEU); setColLowerEqualsUpper(passedColsLEU); } - - + + @Override + public void checkArity() throws HopsException { + HopsException.check(_input.size() == 5, this, "should have 5 inputs but has %d inputs", _input.size()); + } + public boolean isRowLowerEqualsUpper(){ return _rowLowerEqualsUpper; } http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/LeftIndexingOp.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/LeftIndexingOp.java b/src/main/java/org/apache/sysml/hops/LeftIndexingOp.java index f927af7..c6769d4 100644 --- a/src/main/java/org/apache/sysml/hops/LeftIndexingOp.java +++ b/src/main/java/org/apache/sysml/hops/LeftIndexingOp.java @@ -77,7 +77,11 @@ public class LeftIndexingOp extends Hop setColLowerEqualsUpper(passedColsLEU); } - + @Override + public void checkArity() throws HopsException { + HopsException.check(_input.size() == 6, this, "should have 6 inputs but has %d inputs", 6); + } + public boolean getRowLowerEqualsUpper(){ return _rowLowerEqualsUpper; } http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/LiteralOp.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/LiteralOp.java b/src/main/java/org/apache/sysml/hops/LiteralOp.java index e089177..503d270 100644 --- a/src/main/java/org/apache/sysml/hops/LiteralOp.java +++ b/src/main/java/org/apache/sysml/hops/LiteralOp.java @@ -62,7 +62,11 @@ public class LiteralOp extends Hop this.value_boolean = value; } - + @Override + public void checkArity() throws HopsException { + HopsException.check(_input.isEmpty(), this, "should have 0 inputs but has %d inputs", _input.size()); + } + @Override public Lop constructLops() throws HopsException, LopsException http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/MultipleOp.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/MultipleOp.java b/src/main/java/org/apache/sysml/hops/MultipleOp.java index 419bca2..688204f 100644 --- a/src/main/java/org/apache/sysml/hops/MultipleOp.java +++ b/src/main/java/org/apache/sysml/hops/MultipleOp.java @@ -71,6 +71,10 @@ public class MultipleOp extends Hop { refreshSizeInformation(); } + /** MultipleOp may have any number of inputs. */ + @Override + public void checkArity() throws HopsException {} + public MultiInputOp getOp() { return multipleOperandOperation; } http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/ParameterizedBuiltinOp.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/ParameterizedBuiltinOp.java b/src/main/java/org/apache/sysml/hops/ParameterizedBuiltinOp.java index 7bff4bd..88ec360 100644 --- a/src/main/java/org/apache/sysml/hops/ParameterizedBuiltinOp.java +++ b/src/main/java/org/apache/sysml/hops/ParameterizedBuiltinOp.java @@ -112,6 +112,13 @@ public class ParameterizedBuiltinOp extends Hop implements MultiThreadedHop refreshSizeInformation(); } + @Override + public void checkArity() throws HopsException { + int sz = _input.size(); + int pz = _paramIndexMap.size(); + HopsException.check(sz == pz, this, "has %d inputs but %d parameters", sz, pz); + } + public HashMap<String, Integer> getParamIndexMap(){ return _paramIndexMap; } http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/QuaternaryOp.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/QuaternaryOp.java b/src/main/java/org/apache/sysml/hops/QuaternaryOp.java index f352c04..e3dfa54 100644 --- a/src/main/java/org/apache/sysml/hops/QuaternaryOp.java +++ b/src/main/java/org/apache/sysml/hops/QuaternaryOp.java @@ -168,7 +168,13 @@ public class QuaternaryOp extends Hop implements MultiThreadedHop inU.getParent().add(this); inV.getParent().add(this); } - + + @Override + public void checkArity() throws HopsException { + HopsException.check(_input.size() == 3 || _input.size() == 4, this, + "should have arity 3 or 4 but has arity %d", _input.size()); + } + public OpOp4 getOp(){ return _op; } http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/ReorgOp.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/ReorgOp.java b/src/main/java/org/apache/sysml/hops/ReorgOp.java index 1ca2463..7645c46 100644 --- a/src/main/java/org/apache/sysml/hops/ReorgOp.java +++ b/src/main/java/org/apache/sysml/hops/ReorgOp.java @@ -41,7 +41,7 @@ import org.apache.sysml.runtime.matrix.MatrixCharacteristics; * Reorg (cell) operation: aij * Properties: * Symbol: ', rdiag, rshape, rsort - * 1 Operand + * 1 Operand (except sort and reshape take additional arguments) * * Semantic: change indices (in mapper or reducer) * @@ -91,6 +91,24 @@ public class ReorgOp extends Hop implements MultiThreadedHop } @Override + public void checkArity() throws HopsException { + int sz = _input.size(); + switch( op ) { + case TRANSPOSE: + case DIAG: + case REV: + HopsException.check(sz == 1, this, "should have arity 1 for op %s but has arity %d", op, sz); + break; + case RESHAPE: + case SORT: + HopsException.check(sz == 4, this, "should have arity 4 for op %s but has arity %d", op, sz); + break; + default: + throw new HopsException("Unsupported lops construction for operation type '" + op + "'."); + } + } + + @Override public void setMaxNumThreads( int k ) { _maxNumThreads = k; } http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/TernaryOp.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/TernaryOp.java b/src/main/java/org/apache/sysml/hops/TernaryOp.java index ec863c4..b875387 100644 --- a/src/main/java/org/apache/sysml/hops/TernaryOp.java +++ b/src/main/java/org/apache/sysml/hops/TernaryOp.java @@ -44,7 +44,7 @@ import org.apache.sysml.parser.Expression.DataType; import org.apache.sysml.parser.Expression.ValueType; import org.apache.sysml.runtime.matrix.MatrixCharacteristics; -/* Primary use cases for now, are +/** Primary use cases for now, are * quantile (<n-1-matrix>, <n-1-matrix>, <literal>): quantile (A, w, 0.5) * quantile (<n-1-matrix>, <n-1-matrix>, <scalar>): quantile (A, w, s) * interquantile (<n-1-matrix>, <n-1-matrix>, <scalar>): interquantile (A, w, s) @@ -55,9 +55,10 @@ import org.apache.sysml.runtime.matrix.MatrixCharacteristics; * interquantile (A, s) * * Note: this hop should be called AggTernaryOp in consistency with AggUnaryOp and AggBinaryOp; - * however, since there does not exist a real TernaryOp yet - we can leave it as is for now. + * however, since there does not exist a real TernaryOp yet - we can leave it as is for now. + * + * CTABLE op takes 2 extra inputs with target dimensions for padding and pruning. */ - public class TernaryOp extends Hop { @@ -105,7 +106,18 @@ public class TernaryOp extends Hop inp5.getParent().add(this); _dimInputsPresent = true; } - + + @Override + public void checkArity() throws HopsException { + int sz = _input.size(); + if (_dimInputsPresent) { + // only CTABLE + HopsException.check(sz == 5, this, "should have arity 5 for op %s but has arity %d", _op, sz); + } else { + HopsException.check(sz == 3, this, "should have arity 3 for op %s but has arity %d", _op, sz); + } + } + public OpOp3 getOp(){ return _op; } http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/UnaryOp.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/UnaryOp.java b/src/main/java/org/apache/sysml/hops/UnaryOp.java index 451960b..176d131 100644 --- a/src/main/java/org/apache/sysml/hops/UnaryOp.java +++ b/src/main/java/org/apache/sysml/hops/UnaryOp.java @@ -71,6 +71,11 @@ public class UnaryOp extends Hop implements MultiThreadedHop refreshSizeInformation(); } + @Override + public void checkArity() throws HopsException { + HopsException.check(_input.size() == 1, this, "should have arity 1 but has arity %d", _input.size()); + } + // this is for OpOp1, e.g. A = -B (0-B); and a=!b public OpOp1 getOp() { return _op; http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/codegen/SpoofFusedOp.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/codegen/SpoofFusedOp.java b/src/main/java/org/apache/sysml/hops/codegen/SpoofFusedOp.java index 89d205b..779df4f 100644 --- a/src/main/java/org/apache/sysml/hops/codegen/SpoofFusedOp.java +++ b/src/main/java/org/apache/sysml/hops/codegen/SpoofFusedOp.java @@ -61,7 +61,10 @@ public class SpoofFusedOp extends Hop implements MultiThreadedHop _distSupported = dist; _dimsType = type; } - + + @Override + public void checkArity() throws HopsException {} + @Override public void setMaxNumThreads(int k) { _numThreads = k; http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/rewrite/HopDagValidator.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/rewrite/HopDagValidator.java b/src/main/java/org/apache/sysml/hops/rewrite/HopDagValidator.java index 83069ad..52fb36f 100644 --- a/src/main/java/org/apache/sysml/hops/rewrite/HopDagValidator.java +++ b/src/main/java/org/apache/sysml/hops/rewrite/HopDagValidator.java @@ -20,6 +20,8 @@ package org.apache.sysml.hops.rewrite; import java.util.ArrayList; +import java.util.HashSet; +import java.util.Set; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -28,85 +30,98 @@ import org.apache.sysml.hops.DataOp; import org.apache.sysml.hops.Hop; import org.apache.sysml.hops.HopsException; import org.apache.sysml.hops.LiteralOp; +import org.apache.sysml.parser.Expression; import org.apache.sysml.runtime.DMLRuntimeException; import org.apache.sysml.utils.Explain; +import static org.apache.sysml.hops.HopsException.check; + /** * This class allows to check hop dags for validity, e.g., parent-child linking. - * It purpose is soley for debugging purposes (enabled in ProgramRewriter). - * + * Use it for debugging (enabled in {@link ProgramRewriter}). */ -public class HopDagValidator -{ +public class HopDagValidator { private static final Log LOG = LogFactory.getLog(HopDagValidator.class.getName()); + + private HopDagValidator() {} - public static void validateHopDag(ArrayList<Hop> roots) - throws HopsException - { + public static void validateHopDag(final ArrayList<Hop> roots) throws HopsException { if( roots == null ) return; - try - { + try { Hop.resetVisitStatus(roots); + ValidatorState state = new ValidatorState(); for( Hop hop : roots ) - rValidateHop(hop); + rValidateHop(hop, state); } - catch(HopsException ex) - { + catch(HopsException ex) { try { LOG.error( "\n"+Explain.explainHops(roots) ); }catch(DMLRuntimeException e){} - throw ex; } } - public static void validateHopDag(Hop root) - throws HopsException - { + public static void validateHopDag(final Hop root) throws HopsException { if( root == null ) return; - - try - { + try { root.resetVisitStatus(); - rValidateHop(root); + ValidatorState state = new ValidatorState(); + rValidateHop(root, state); } - catch(HopsException ex) - { + catch(HopsException ex) { try { LOG.error( "\n"+Explain.explain(root) ); }catch(DMLRuntimeException e){} - throw ex; } } + + private static class ValidatorState { + final Set<Long> seen = new HashSet<>(); + } - private static void rValidateHop( Hop hop ) - throws HopsException - { - if(hop.isVisited()) - return; + private static void rValidateHop(final Hop hop, final ValidatorState state) throws HopsException { + final long id = hop.getHopID(); + + final boolean seen = !state.seen.add(id); + check(seen == hop.isVisited(), hop, + "seen previously is %b but does not match hop visit status", seen); + if (seen) return; // we saw the Hop previously, no need to re-validate //check parent linking for( Hop parent : hop.getParent() ) - if( !parent.getInput().contains(hop) ) - throw new HopsException("Hop id="+hop.getHopID()+" not properly linked to its parent pid="+parent.getHopID()+" "+parent.getClass().getName()); - + check(parent.getInput().contains(hop), hop, + "not properly linked to its parent pid=%d %s", + parent.getHopID(), parent.getClass().getName()); + + final ArrayList<Hop> input = hop.getInput(); + final Expression.DataType dt = hop.getDataType(); + final Expression.ValueType vt = hop.getValueType(); + //check child linking - for( Hop child : hop.getInput() ) - if( !child.getParent().contains(hop) ) - throw new HopsException("Hop id="+hop.getHopID()+" not properly linked to its child cid="+child.getHopID()+" "+child.getClass().getName()); - - //check empty childs - if( hop.getInput().isEmpty() ) - if( !(hop instanceof DataOp || hop instanceof LiteralOp) ) - throw new HopsException("Hop id="+hop.getHopID()+" is not a dataop/literal but has no childs."); - - //recursively process childs - for( Hop child : hop.getInput() ) - rValidateHop(child); - + for( Hop child : input ) + check(child.getParent().contains(hop), hop, "not properly linked to its child cid=%d %s", + child.getHopID(), child.getClass().getName()); + + //check empty children (other variable-length Hops must have at least one child) + if( input.isEmpty() ) + check(hop instanceof DataOp || hop instanceof LiteralOp, hop, + "is not a dataop/literal but has no children"); + + // check Hop has a legal arity (number of children) + hop.checkArity(); + + // check Matrix data type Hops must have Double Value type + if (dt == Expression.DataType.MATRIX ) + check(vt == Expression.ValueType.DOUBLE, hop, + "has Matrix type but Value Type %s is not DOUBLE", vt); + + //recursively process children + for( Hop child : input ) + rValidateHop(child, state); + hop.setVisited(); } } http://git-wip-us.apache.org/repos/asf/systemml/blob/0bcae49f/src/main/java/org/apache/sysml/hops/rewrite/ProgramRewriter.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/rewrite/ProgramRewriter.java b/src/main/java/org/apache/sysml/hops/rewrite/ProgramRewriter.java index a34ccbe..0e65f3f 100644 --- a/src/main/java/org/apache/sysml/hops/rewrite/ProgramRewriter.java +++ b/src/main/java/org/apache/sysml/hops/rewrite/ProgramRewriter.java @@ -260,18 +260,20 @@ public class ProgramRewriter public ArrayList<Hop> rewriteHopDAGs(ArrayList<Hop> roots, ProgramRewriteStatus state) throws HopsException - { + { for( HopRewriteRule r : _dagRuleSet ) { Hop.resetVisitStatus( roots ); //reset for each rule roots = r.rewriteHopDAGs(roots, state); - if( CHECK ) { - LOG.info("Validation after: "+r.getClass().getName()); - HopDagValidator.validateHopDag(roots); - } + if( CHECK ) + try { + HopDagValidator.validateHopDag(roots); + } catch (HopsException e) { + LOG.error("Invalid hop after rewriting by " + r.getClass().getName(), e); + throw e; + } } - return roots; } @@ -279,19 +281,21 @@ public class ProgramRewriter throws HopsException { if( root == null ) - return root; + return null; for( HopRewriteRule r : _dagRuleSet ) { root.resetVisitStatus(); //reset for each rule root = r.rewriteHopDAG(root, state); - - if( CHECK ) { - LOG.info("Validation after: "+r.getClass().getName()); - HopDagValidator.validateHopDag(root); - } + + if( CHECK ) + try { + HopDagValidator.validateHopDag(root); + } catch (HopsException e) { + LOG.error("Invalid hop after rewriting by " + r.getClass().getName(), e); + throw e; + } } - return root; }
