You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@systemml.apache.org by mb...@apache.org on 2018/02/09 06:04:13 UTC

[1/3] systemml git commit: [SYSTEMML-2135] Extended removeEmpty with outputs of zero rows/cols

Repository: systemml
Updated Branches:
  refs/heads/master 8b054804e -> 7e11deaa8


[SYSTEMML-2135] Extended removeEmpty with outputs of zero rows/cols

This patch extends the support for matrices with zero rows and columns
to removeEmpty. So far this operation returned a single row/column of
zeros for empty inputs. For backwards compatibility, we keep these
semantics, but introduce a new flag that allows to disable this special
case in order to return matrices with zero rows and columns,
respectively. In addition, this patch also includes the related update
of the dml language guide.


Project: http://git-wip-us.apache.org/repos/asf/systemml/repo
Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/e0b66b30
Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/e0b66b30
Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/e0b66b30

Branch: refs/heads/master
Commit: e0b66b300c8bd4582579bac8b6dea26e132447b0
Parents: 8b05480
Author: Matthias Boehm <mb...@gmail.com>
Authored: Thu Feb 8 18:12:32 2018 -0800
Committer: Matthias Boehm <mb...@gmail.com>
Committed: Thu Feb 8 22:04:03 2018 -0800

----------------------------------------------------------------------
 docs/dml-language-reference.md                  |  2 +-
 .../sysml/hops/ParameterizedBuiltinOp.java      |  3 +-
 .../ParameterizedBuiltinFunctionExpression.java | 21 ++++++-
 .../java/org/apache/sysml/parser/dml/Dml.g4     |  2 +-
 .../java/org/apache/sysml/parser/pydml/Pydml.g4 |  2 +-
 .../runtime/compress/CompressedMatrixBlock.java |  8 +--
 .../estim/CompressedSizeEstimatorSample.java    |  2 +-
 .../cp/ParameterizedBuiltinCPInstruction.java   | 14 ++---
 .../ParameterizedBuiltinSPInstruction.java      | 44 +++++++-------
 .../spark/utils/RDDAggregateUtils.java          |  6 +-
 .../sysml/runtime/io/WriterBinaryBlock.java     |  9 ++-
 .../runtime/matrix/data/LibMatrixReorg.java     | 20 ++++---
 .../sysml/runtime/matrix/data/MatrixBlock.java  |  8 +--
 .../functions/misc/ZeroRowsColsMatrixTest.java  | 60 ++++++++++----------
 .../functions/misc/ZeroMatrix_RemoveEmpty.R     |  2 +-
 .../functions/misc/ZeroMatrix_RemoveEmpty.dml   |  6 +-
 16 files changed, 113 insertions(+), 96 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/systemml/blob/e0b66b30/docs/dml-language-reference.md
----------------------------------------------------------------------
diff --git a/docs/dml-language-reference.md b/docs/dml-language-reference.md
index 5fb9de5..355b507 100644
--- a/docs/dml-language-reference.md
+++ b/docs/dml-language-reference.md
@@ -648,7 +648,7 @@ nrow(), <br/> ncol(), <br/> length() | Return the number of rows, number of colu
 prod() | Return the product of all cells in matrix | Input: matrix <br/> Output: scalarj | prod(X)
 rand() | Generates a random matrix | Input: (rows=&lt;value&gt;, cols=&lt;value&gt;, min=&lt;value&gt;, max=&lt;value&gt;, sparsity=&lt;value&gt;, pdf=&lt;string&gt;, seed=&lt;value&gt;) <br/> rows/cols: Number of rows/cols (expression) <br/> min/max: Min/max value for cells (either constant value, or variable that evaluates to constant value) <br/> sparsity: fraction of non-zero cells (constant value) <br/> pdf: "uniform" (min, max) distribution, or "normal" (0,1) distribution; or "poisson" (lambda=1) distribution. string; default value is "uniform". Note that, for the Poisson distribution, users can provide the mean/lambda parameter as follows: <br/> rand(rows=1000,cols=1000, pdf="poisson", lambda=2.5). <br/> The default value for lambda is 1. <br/> seed: Every invocation of rand() internally generates a random seed with which the cell values are generated. One can optionally provide a seed when repeatability is desired.  <br/> Output: matrix | X = rand(rows=10, cols=20, min=0, m
 ax=1, pdf="uniform", sparsity=0.2) <br/> The example generates a 10 x 20 matrix, with cell values uniformly chosen at random between 0 and 1, and approximately 20% of cells will have non-zero values.
 rbind() | Row-wise matrix concatenation. Concatenates the second matrix as additional rows to the first matrix | Input: (X &lt;matrix&gt;, Y &lt;matrix&gt;) <br/>Output: &lt;matrix&gt; <br/> X and Y are matrices, where the number of columns in X and the number of columns in Y are the same. | A = matrix(1, rows=2,cols=3) <br/> B = matrix(2, rows=2,cols=3) <br/> C = rbind(A,B) <br/> print("Dimensions of C: " + nrow(C) + " X " + ncol(C)) <br/> Output: <br/> Dimensions of C: 4 X 3
-removeEmpty() | Removes all empty rows or columns from the input matrix target X according to the specified margin. Also, allows to apply a filter F before removing the empty rows/cols. | Input : (target= X &lt;matrix&gt;, margin="...", select=F) <br/> Output : &lt;matrix&gt; <br/> Valid values for margin are "rows" or "cols". | A = removeEmpty(target=X, margin="rows", select=F)
+removeEmpty() | Removes all empty rows or columns from the input matrix target X according to the specified margin. The optional select vector F specifies selected rows or columns; if not provided, the semantics are F=(rowSums(X!=0)&gt;0) and F=(colSums(X!=0)&gt;0) for removeEmpty "rows" and "cols", respectively. The optional empty.return flag indicates if a row or column of zeros should be returned for empty inputs. | Input : (target= X &lt;matrix&gt;, margin="..."[, select=F][, empty.return=TRUE]) <br/> Output : &lt;matrix&gt; <br/> Valid values for margin are "rows" or "cols". | A = removeEmpty(target=X, margin="rows", select=F)
 replace() | Creates a copy of input matrix X, where all values that are equal to the scalar pattern s1 are replaced with the scalar replacement s2. | Input : (target= X &lt;matrix&gt;, pattern=&lt;scalar&gt;, replacement=&lt;scalar&gt;) <br/> Output : &lt;matrix&gt; <br/> If s1 is NaN, then all NaN values of X are treated as equal and hence replaced with s2. Positive and negative infinity are treated as different values. | A = replace(target=X, pattern=s1, replacement=s2)
 rev() | Reverses the rows in a matrix | Input : (&lt;matrix&gt;) <br/> Output : &lt;matrix&gt; | <span style="white-space: nowrap;">A = matrix("1 2 3 4", rows=2, cols=2)</span> <br/> <span style="white-space: nowrap;">B = matrix("1 2 3 4", rows=4, cols=1)</span> <br/> <span style="white-space: nowrap;">C = matrix("1 2 3 4", rows=1, cols=4)</span> <br/> revA = rev(A) <br/> revB = rev(B) <br/> revC = rev(C) <br/> Matrix revA: [[3, 4], [1, 2]]<br/> Matrix revB: [[4], [3], [2], [1]]<br/> Matrix revC: [[1, 2, 3, 4]]<br/>
 seq() | Creates a single column vector with values starting from &lt;from&gt;, to &lt;to&gt;, in increments of &lt;increment&gt; | Input: (&lt;from&gt;, &lt;to&gt;, &lt;increment&gt;) <br/> Output: &lt;matrix&gt; | S = seq (10, 200, 10)

http://git-wip-us.apache.org/repos/asf/systemml/blob/e0b66b30/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 aa48061..e9800a5 100644
--- a/src/main/java/org/apache/sysml/hops/ParameterizedBuiltinOp.java
+++ b/src/main/java/org/apache/sysml/hops/ParameterizedBuiltinOp.java
@@ -726,7 +726,8 @@ public class ParameterizedBuiltinOp extends Hop implements MultiThreadedHop
 			inMap.put("offset", loffset);
 			inMap.put("maxdim", lmaxdim);
 			inMap.put("margin", inputlops.get("margin"));
-		
+			inMap.put("empty.return", inputlops.get("empty.return"));
+			
 			if ( !FORCE_DIST_RM_EMPTY && isRemoveEmptyBcSP())
 				_bRmEmptyBC = true;
 			

http://git-wip-us.apache.org/repos/asf/systemml/blob/e0b66b30/src/main/java/org/apache/sysml/parser/ParameterizedBuiltinFunctionExpression.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/parser/ParameterizedBuiltinFunctionExpression.java b/src/main/java/org/apache/sysml/parser/ParameterizedBuiltinFunctionExpression.java
index b365abb..dbb2a1e 100644
--- a/src/main/java/org/apache/sysml/parser/ParameterizedBuiltinFunctionExpression.java
+++ b/src/main/java/org/apache/sysml/parser/ParameterizedBuiltinFunctionExpression.java
@@ -23,10 +23,13 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 import org.antlr.v4.runtime.ParserRuleContext;
 import org.apache.sysml.hops.Hop.ParamBuiltinOp;
 import org.apache.sysml.parser.LanguageException.LanguageErrorCodes;
+import org.apache.sysml.runtime.util.UtilFunctions;
 import org.apache.wink.json4j.JSONObject;
 
 
@@ -476,6 +479,15 @@ public class ParameterizedBuiltinFunctionExpression extends DataIdentifier
 	}
 
 	private void validateRemoveEmpty(DataIdentifier output, boolean conditional) throws LanguageException {
+		
+		//check for invalid parameters
+		Set<String> valid = UtilFunctions.asSet("target", "margin", "select", "empty.return");
+		Set<String> invalid = _varParams.keySet().stream()
+			.filter(k -> !valid.contains(k)).collect(Collectors.toSet());
+		if( !invalid.isEmpty() )
+			raiseValidateError("Invalid parameters for removeEmpty: "
+				+ Arrays.toString(invalid.toArray(new String[0])), false);
+		
 		//check existence and correctness of arguments
 		Expression target = getVarParam("target");
 		if( target==null ) {
@@ -498,11 +510,18 @@ public class ParameterizedBuiltinFunctionExpression extends DataIdentifier
 			raiseValidateError("Index matrix 'select' is of type '"+select.getOutput().getDataType()+"'. Please specify the select matrix.", conditional, LanguageErrorCodes.INVALID_PARAMETERS);
 		}
 		
+		Expression empty = getVarParam("empty.return");
+		if( empty!=null && (!empty.getOutput().getDataType().isScalar() || empty.getOutput().getValueType() != ValueType.BOOLEAN) ){
+			raiseValidateError("Boolean parameter 'empty.return' is of type "+empty.getOutput().getDataType()
+				+"["+empty.getOutput().getValueType()+"].", conditional, LanguageErrorCodes.INVALID_PARAMETERS);
+		}
+		if( empty == null ) //default handling
+			_varParams.put("empty.return", new BooleanIdentifier(true));
+		
 		// Output is a matrix with unknown dims
 		output.setDataType(DataType.MATRIX);
 		output.setValueType(ValueType.DOUBLE);
 		output.setDimensions(-1, -1);
-		
 	}
 	
 	private void validateGroupedAgg(DataIdentifier output, boolean conditional) 

http://git-wip-us.apache.org/repos/asf/systemml/blob/e0b66b30/src/main/java/org/apache/sysml/parser/dml/Dml.g4
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/parser/dml/Dml.g4 b/src/main/java/org/apache/sysml/parser/dml/Dml.g4
index fb72ed2..9491855 100644
--- a/src/main/java/org/apache/sysml/parser/dml/Dml.g4
+++ b/src/main/java/org/apache/sysml/parser/dml/Dml.g4
@@ -182,7 +182,7 @@ strictParameterizedKeyValueString : paramName=ID '=' paramVal=STRING ;
 ID : (ALPHABET (ALPHABET|DIGIT|'_')*  '::')? ALPHABET (ALPHABET|DIGIT|'_')*
     // Special ID cases:
    // | 'matrix' // --> This is a special case which causes lot of headache
-   | 'as.scalar' | 'as.matrix' | 'as.frame' | 'as.double' | 'as.integer' | 'as.logical' | 'index.return' | 'lower.tail'
+   | 'as.scalar' | 'as.matrix' | 'as.frame' | 'as.double' | 'as.integer' | 'as.logical' | 'index.return' | 'empty.return' | 'lower.tail'
 ;
 // Unfortunately, we have datatype name clashing with builtin function name: matrix :(
 // Therefore, ugly work around for checking datatype

http://git-wip-us.apache.org/repos/asf/systemml/blob/e0b66b30/src/main/java/org/apache/sysml/parser/pydml/Pydml.g4
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/parser/pydml/Pydml.g4 b/src/main/java/org/apache/sysml/parser/pydml/Pydml.g4
index 320793e..34d0c34 100644
--- a/src/main/java/org/apache/sysml/parser/pydml/Pydml.g4
+++ b/src/main/java/org/apache/sysml/parser/pydml/Pydml.g4
@@ -302,7 +302,7 @@ ID : (ALPHABET (ALPHABET|DIGIT|'_')*  '.')? ALPHABET (ALPHABET|DIGIT|'_')*
     // Special ID cases:
    // | 'matrix' // --> This is a special case which causes lot of headache
    // | 'scalar' |  'float' | 'int' | 'bool' // corresponds to as.scalar, as.double, as.integer and as.logical
-   | 'index.return'
+   | 'index.return' | 'empty.return'
 ;
 // Unfortunately, we have datatype name clashing with builtin function name: matrix :(
 // Therefore, ugly work around for checking datatype

http://git-wip-us.apache.org/repos/asf/systemml/blob/e0b66b30/src/main/java/org/apache/sysml/runtime/compress/CompressedMatrixBlock.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/compress/CompressedMatrixBlock.java b/src/main/java/org/apache/sysml/runtime/compress/CompressedMatrixBlock.java
index 0ce3dc8..d9cfc72 100644
--- a/src/main/java/org/apache/sysml/runtime/compress/CompressedMatrixBlock.java
+++ b/src/main/java/org/apache/sysml/runtime/compress/CompressedMatrixBlock.java
@@ -2165,19 +2165,19 @@ public class CompressedMatrixBlock extends MatrixBlock implements Externalizable
 	}
 
 	@Override
-	public MatrixBlock removeEmptyOperations(MatrixBlock ret, boolean rows, MatrixBlock select) 
+	public MatrixBlock removeEmptyOperations(MatrixBlock ret, boolean rows, boolean emptyReturn, MatrixBlock select) 
 			throws DMLRuntimeException {
 		printDecompressWarning("removeEmptyOperations");
 		MatrixBlock tmp = isCompressed() ? decompress() : this;
-		return tmp.removeEmptyOperations(ret, rows, select);
+		return tmp.removeEmptyOperations(ret, rows, emptyReturn, select);
 	}
 
 	@Override
-	public MatrixBlock removeEmptyOperations(MatrixBlock ret, boolean rows)
+	public MatrixBlock removeEmptyOperations(MatrixBlock ret, boolean rows, boolean emptyReturn)
 			throws DMLRuntimeException {
 		printDecompressWarning("removeEmptyOperations");
 		MatrixBlock tmp = isCompressed() ? decompress() : this;
-		return tmp.removeEmptyOperations(ret, rows);
+		return tmp.removeEmptyOperations(ret, rows, emptyReturn);
 	}
 
 	@Override

http://git-wip-us.apache.org/repos/asf/systemml/blob/e0b66b30/src/main/java/org/apache/sysml/runtime/compress/estim/CompressedSizeEstimatorSample.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/compress/estim/CompressedSizeEstimatorSample.java b/src/main/java/org/apache/sysml/runtime/compress/estim/CompressedSizeEstimatorSample.java
index 2492c0b..f3d842a 100644
--- a/src/main/java/org/apache/sysml/runtime/compress/estim/CompressedSizeEstimatorSample.java
+++ b/src/main/java/org/apache/sysml/runtime/compress/estim/CompressedSizeEstimatorSample.java
@@ -63,7 +63,7 @@ public class CompressedSizeEstimatorSample extends CompressedSizeEstimator
 			for( int i=0; i<sampleSize; i++ )
 				select.quickSetValue(_sampleRows[i], 0, 1);
 			_data = _data.removeEmptyOperations(new MatrixBlock(), 
-					!CompressedMatrixBlock.TRANSPOSE_INPUT, select);
+					!CompressedMatrixBlock.TRANSPOSE_INPUT, true, select);
 		}
 		
 		//establish estimator-local cache for numeric solve

http://git-wip-us.apache.org/repos/asf/systemml/blob/e0b66b30/src/main/java/org/apache/sysml/runtime/instructions/cp/ParameterizedBuiltinCPInstruction.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/instructions/cp/ParameterizedBuiltinCPInstruction.java b/src/main/java/org/apache/sysml/runtime/instructions/cp/ParameterizedBuiltinCPInstruction.java
index 5a7442a..18ec72c 100644
--- a/src/main/java/org/apache/sysml/runtime/instructions/cp/ParameterizedBuiltinCPInstruction.java
+++ b/src/main/java/org/apache/sysml/runtime/instructions/cp/ParameterizedBuiltinCPInstruction.java
@@ -197,19 +197,17 @@ public class ParameterizedBuiltinCPInstruction extends ComputationCPInstruction
 			
 		}
 		else if ( opcode.equalsIgnoreCase("rmempty") ) {
+			String margin = params.get("margin");
+			if( !(margin.equals("rows") || margin.equals("cols")) )
+				throw new DMLRuntimeException("Unspupported margin identifier '"+margin+"'.");
+			
 			// acquire locks
 			MatrixBlock target = ec.getMatrixInput(params.get("target"), getExtendedOpcode());
 			MatrixBlock select = params.containsKey("select")? ec.getMatrixInput(params.get("select"), getExtendedOpcode()):null;
 			
 			// compute the result
-			String margin = params.get("margin");
-			MatrixBlock soresBlock = null;
-			if( margin.equals("rows") )
-				soresBlock = target.removeEmptyOperations(new MatrixBlock(), true, select);
-			else if( margin.equals("cols") ) 
-				soresBlock = target.removeEmptyOperations(new MatrixBlock(), false, select);
-			else
-				throw new DMLRuntimeException("Unspupported margin identifier '"+margin+"'.");
+			MatrixBlock soresBlock = target.removeEmptyOperations(new MatrixBlock(),
+				margin.equals("rows"), margin.equals("empty.return"), select);
 			
 			//release locks
 			ec.setMatrixOutput(output.getName(), soresBlock, getExtendedOpcode());

http://git-wip-us.apache.org/repos/asf/systemml/blob/e0b66b30/src/main/java/org/apache/sysml/runtime/instructions/spark/ParameterizedBuiltinSPInstruction.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/instructions/spark/ParameterizedBuiltinSPInstruction.java b/src/main/java/org/apache/sysml/runtime/instructions/spark/ParameterizedBuiltinSPInstruction.java
index 49dd6e5..425739d 100644
--- a/src/main/java/org/apache/sysml/runtime/instructions/spark/ParameterizedBuiltinSPInstruction.java
+++ b/src/main/java/org/apache/sysml/runtime/instructions/spark/ParameterizedBuiltinSPInstruction.java
@@ -320,6 +320,7 @@ public class ParameterizedBuiltinSPInstruction extends ComputationSPInstruction
 			String rddOffVar = params.get("offset");
 			
 			boolean rows = sec.getScalarInput(params.get("margin"), ValueType.STRING, true).getStringValue().equals("rows");
+			boolean emptyReturn = Boolean.parseBoolean(params.get("empty.return").toLowerCase());
 			long maxDim = sec.getScalarInput(params.get("maxdim"), ValueType.DOUBLE, false).getLongValue();
 			MatrixCharacteristics mcIn = sec.getMatrixCharacteristics(rddInVar);
 			
@@ -340,14 +341,14 @@ public class ParameterizedBuiltinSPInstruction extends ComputationSPInstruction
 					broadcastOff = sec.getBroadcastForVariable( rddOffVar );
 					// Broadcast offset vector
 					out = in
-						.flatMapToPair(new RDDRemoveEmptyFunctionInMem(rows, maxDim, brlen, bclen, broadcastOff));		
+						.flatMapToPair(new RDDRemoveEmptyFunctionInMem(rows, maxDim, brlen, bclen, broadcastOff));
 				}
 				else {
 					off = sec.getBinaryBlockRDDHandleForVariable( rddOffVar );
 					out = in
 						.join( off.flatMapToPair(new ReplicateVectorFunction(!rows,numRep)) )
-						.flatMapToPair(new RDDRemoveEmptyFunction(rows, maxDim, brlen, bclen));		
-				}				
+						.flatMapToPair(new RDDRemoveEmptyFunction(rows, maxDim, brlen, bclen));
+				}
 	
 				out = RDDAggregateUtils.mergeByKey(out, false);
 				
@@ -365,7 +366,8 @@ public class ParameterizedBuiltinSPInstruction extends ComputationSPInstruction
 			}
 			else //special case: empty output (ensure valid dims)
 			{
-				MatrixBlock out = new MatrixBlock(rows?1:(int)mcIn.getRows(), rows?(int)mcIn.getCols():1, true); 
+				int n = emptyReturn ? 1 : 0;
+				MatrixBlock out = new MatrixBlock(rows?n:(int)mcIn.getRows(), rows?(int)mcIn.getCols():n, true); 
 				sec.setMatrixOutput(output.getName(), out, getExtendedOpcode());
 			}
 		}
@@ -521,13 +523,12 @@ public class ParameterizedBuiltinSPInstruction extends ComputationSPInstruction
 	{
 		private static final long serialVersionUID = 4906304771183325289L;
 
-		private boolean _rmRows; 
-		private long _len;
-		private long _brlen;
-		private long _bclen;
-				
-		public RDDRemoveEmptyFunction(boolean rmRows, long len, long brlen, long bclen) 
-		{
+		private final boolean _rmRows;
+		private final long _len;
+		private final long _brlen;
+		private final long _bclen;
+		
+		public RDDRemoveEmptyFunction(boolean rmRows, long len, long brlen, long bclen) {
 			_rmRows = rmRows;
 			_len = len;
 			_brlen = brlen;
@@ -545,7 +546,7 @@ public class ParameterizedBuiltinSPInstruction extends ComputationSPInstruction
 			//execute remove empty operations
 			ArrayList<IndexedMatrixValue> out = new ArrayList<>();
 			LibMatrixReorg.rmempty(data, offsets, _rmRows, _len, _brlen, _bclen, out);
-
+			
 			//prepare and return outputs
 			return SparkUtils.fromIndexedMatrixBlock(out).iterator();
 		}
@@ -555,13 +556,13 @@ public class ParameterizedBuiltinSPInstruction extends ComputationSPInstruction
 	{
 		private static final long serialVersionUID = 4906304771183325289L;
 
-		private boolean _rmRows; 
-		private long _len;
-		private long _brlen;
-		private long _bclen;
+		private final boolean _rmRows;
+		private final long _len;
+		private final long _brlen;
+		private final long _bclen;
 		
 		private PartitionedBroadcast<MatrixBlock> _off = null;
-				
+		
 		public RDDRemoveEmptyFunctionInMem(boolean rmRows, long len, long brlen, long bclen, PartitionedBroadcast<MatrixBlock> off) 
 		{
 			_rmRows = rmRows;
@@ -577,12 +578,9 @@ public class ParameterizedBuiltinSPInstruction extends ComputationSPInstruction
 		{
 			//prepare inputs (for internal api compatibility)
 			IndexedMatrixValue data = SparkUtils.toIndexedMatrixBlock(arg0._1(),arg0._2());
-			//IndexedMatrixValue offsets = SparkUtils.toIndexedMatrixBlock(arg0._1(),arg0._2()._2());
-			IndexedMatrixValue offsets = null;
-			if(_rmRows)
-				offsets = SparkUtils.toIndexedMatrixBlock(arg0._1(), _off.getBlock((int)arg0._1().getRowIndex(), 1));
-			else
-				offsets = SparkUtils.toIndexedMatrixBlock(arg0._1(), _off.getBlock(1, (int)arg0._1().getColumnIndex()));
+			IndexedMatrixValue offsets = _rmRows ?
+				SparkUtils.toIndexedMatrixBlock(arg0._1(), _off.getBlock((int)arg0._1().getRowIndex(), 1)) :
+				SparkUtils.toIndexedMatrixBlock(arg0._1(), _off.getBlock(1, (int)arg0._1().getColumnIndex()));
 			
 			//execute remove empty operations
 			ArrayList<IndexedMatrixValue> out = new ArrayList<>();

http://git-wip-us.apache.org/repos/asf/systemml/blob/e0b66b30/src/main/java/org/apache/sysml/runtime/instructions/spark/utils/RDDAggregateUtils.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/instructions/spark/utils/RDDAggregateUtils.java b/src/main/java/org/apache/sysml/runtime/instructions/spark/utils/RDDAggregateUtils.java
index 70174a9..97870e3 100644
--- a/src/main/java/org/apache/sysml/runtime/instructions/spark/utils/RDDAggregateUtils.java
+++ b/src/main/java/org/apache/sysml/runtime/instructions/spark/utils/RDDAggregateUtils.java
@@ -564,7 +564,7 @@ public class RDDAggregateUtils
 		private MatrixBlock _corr = null;
 		
 		public AggregateSingleBlockFunction( AggregateOperator op ) {
-			_op = op;	
+			_op = op;
 		}
 		
 		@Override
@@ -572,11 +572,11 @@ public class RDDAggregateUtils
 			throws Exception 
 		{
 			//prepare combiner block
-			if( arg0.getNumRows() <= 0 || arg0.getNumColumns() <= 0) {
+			if( arg0.getNumRows() == 0 && arg0.getNumColumns() == 0) {
 				arg0.copy(arg1);
 				return arg0;
 			}
-			else if( arg1.getNumRows() <= 0 || arg1.getNumColumns() <= 0 ) {
+			else if( arg1.getNumRows() == 0 && arg1.getNumColumns() == 0 ) {
 				return arg0;
 			}
 			

http://git-wip-us.apache.org/repos/asf/systemml/blob/e0b66b30/src/main/java/org/apache/sysml/runtime/io/WriterBinaryBlock.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/io/WriterBinaryBlock.java b/src/main/java/org/apache/sysml/runtime/io/WriterBinaryBlock.java
index a8f31cb..0d8e221 100644
--- a/src/main/java/org/apache/sysml/runtime/io/WriterBinaryBlock.java
+++ b/src/main/java/org/apache/sysml/runtime/io/WriterBinaryBlock.java
@@ -77,15 +77,14 @@ public class WriterBinaryBlock extends MatrixWriter
 		JobConf job = new JobConf(ConfigurationManager.getCachedJobConf());
 		Path path = new Path( fname );
 		FileSystem fs = IOUtilFunctions.getFileSystem(path, job);
-		
 		SequenceFile.Writer writer = null;
 		try {
 			writer = new SequenceFile.Writer(fs, job, path,
-					                        MatrixIndexes.class, MatrixBlock.class);
-			
+				MatrixIndexes.class, MatrixBlock.class);
 			MatrixIndexes index = new MatrixIndexes(1, 1);
-			MatrixBlock block = new MatrixBlock((int)Math.min(rlen, brlen),
-												(int)Math.min(clen, bclen), true);
+			MatrixBlock block = new MatrixBlock(
+				(int)Math.max(Math.min(rlen, brlen),1),
+				(int)Math.max(Math.min(clen, bclen),1), true);
 			writer.append(index, block);
 		}
 		finally {

http://git-wip-us.apache.org/repos/asf/systemml/blob/e0b66b30/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixReorg.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixReorg.java b/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixReorg.java
index 59df934..dcf8c2f 100644
--- a/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixReorg.java
+++ b/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixReorg.java
@@ -512,28 +512,30 @@ public class LibMatrixReorg
 	 * @param in input matrix
 	 * @param ret output matrix
 	 * @param rows ?
+	 * @param emptyReturn return row/column of zeros for empty input
 	 * @param select ?
 	 * @return matrix block
 	 * @throws DMLRuntimeException if DMLRuntimeException occurs
 	 */
-	public static MatrixBlock rmempty(MatrixBlock in, MatrixBlock ret, boolean rows, MatrixBlock select) 
+	public static MatrixBlock rmempty(MatrixBlock in, MatrixBlock ret, boolean rows, boolean emptyReturn, MatrixBlock select) 
 		throws DMLRuntimeException
 	{
 		//check for empty inputs 
 		//(the semantics of removeEmpty are that for an empty m-by-n matrix, the output 
 		//is an empty 1-by-n or m-by-1 matrix because we don't allow matrices with dims 0)
 		if( in.isEmptyBlock(false) && select == null  ) {
+			int n = emptyReturn ? 1 : 0;
 			if( rows )
-				ret.reset(1, in.clen, in.sparse);
+				ret.reset(n, in.clen, in.sparse);
 			else //cols
-				ret.reset(in.rlen, 1, in.sparse);
+				ret.reset(in.rlen, n, in.sparse);
 			return ret;
 		}
 		
 		if( rows )
-			return removeEmptyRows(in, ret, select);
+			return removeEmptyRows(in, ret, select, emptyReturn);
 		else //cols
-			return removeEmptyColumns(in, ret, select);
+			return removeEmptyColumns(in, ret, select, emptyReturn);
 	}
 
 	/**
@@ -1704,7 +1706,7 @@ public class LibMatrixReorg
 			new MatrixIndexes(ci, cj);
 	}
 
-	private static MatrixBlock removeEmptyRows(MatrixBlock in, MatrixBlock ret, MatrixBlock select) 
+	private static MatrixBlock removeEmptyRows(MatrixBlock in, MatrixBlock ret, MatrixBlock select, boolean emptyReturn) 
 		throws DMLRuntimeException 
 	{
 		final int m = in.rlen;
@@ -1769,7 +1771,7 @@ public class LibMatrixReorg
 		//Step 2: reset result and copy rows
 		//dense stays dense if correct input representation (but robust for any input), 
 		//sparse might be dense/sparse
-		rlen2 = Math.max(rlen2, 1); //ensure valid output
+		rlen2 = Math.max(rlen2, emptyReturn ? 1 : 0); //ensure valid output
 		boolean sp = MatrixBlock.evalSparseFormatInMemory(rlen2, n, in.nonZeros);
 		ret.reset(rlen2, n, sp);
 		if( in.isEmptyBlock(false) )
@@ -1825,7 +1827,7 @@ public class LibMatrixReorg
 		return ret;
 	}
 
-	private static MatrixBlock removeEmptyColumns(MatrixBlock in, MatrixBlock ret, MatrixBlock select) 
+	private static MatrixBlock removeEmptyColumns(MatrixBlock in, MatrixBlock ret, MatrixBlock select, boolean emptyReturn) 
 		throws DMLRuntimeException 
 	{
 		final int m = in.rlen;
@@ -1872,7 +1874,7 @@ public class LibMatrixReorg
 		//Step 3: reset result and copy columns
 		//dense stays dense if correct input representation (but robust for any input), 
 		// sparse might be dense/sparse
-		clen2 = Math.max(clen2, 1); //ensure valid output
+		clen2 = Math.max(clen2, emptyReturn ? 1 : 0); //ensure valid output
 		boolean sp = MatrixBlock.evalSparseFormatInMemory(m, clen2, in.nonZeros);
 		ret.reset(m, clen2, sp);
 		if( in.isEmptyBlock(false) )

http://git-wip-us.apache.org/repos/asf/systemml/blob/e0b66b30/src/main/java/org/apache/sysml/runtime/matrix/data/MatrixBlock.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/matrix/data/MatrixBlock.java b/src/main/java/org/apache/sysml/runtime/matrix/data/MatrixBlock.java
index 8767b13..c59ec51 100644
--- a/src/main/java/org/apache/sysml/runtime/matrix/data/MatrixBlock.java
+++ b/src/main/java/org/apache/sysml/runtime/matrix/data/MatrixBlock.java
@@ -4873,17 +4873,17 @@ public class MatrixBlock extends MatrixValue implements CacheBlock, Externalizab
 		return result;
 	}
 
-	public MatrixBlock removeEmptyOperations( MatrixBlock ret, boolean rows, MatrixBlock select )
+	public MatrixBlock removeEmptyOperations( MatrixBlock ret, boolean rows, boolean emptyReturn, MatrixBlock select )
 		throws DMLRuntimeException 
 	{	
 		MatrixBlock result = checkType(ret);
-		return LibMatrixReorg.rmempty(this, result, rows, select);
+		return LibMatrixReorg.rmempty(this, result, rows, emptyReturn, select);
 	}
 
-	public MatrixBlock removeEmptyOperations( MatrixBlock ret, boolean rows)
+	public MatrixBlock removeEmptyOperations( MatrixBlock ret, boolean rows, boolean emptyReturn)
 		throws DMLRuntimeException 
 	{
-		return removeEmptyOperations(ret, rows, null);
+		return removeEmptyOperations(ret, rows, emptyReturn, null);
 	}
 
 	public MatrixBlock rexpandOperations( MatrixBlock ret, double max, boolean rows, boolean cast, boolean ignore, int k )

http://git-wip-us.apache.org/repos/asf/systemml/blob/e0b66b30/src/test/java/org/apache/sysml/test/integration/functions/misc/ZeroRowsColsMatrixTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/sysml/test/integration/functions/misc/ZeroRowsColsMatrixTest.java b/src/test/java/org/apache/sysml/test/integration/functions/misc/ZeroRowsColsMatrixTest.java
index 349828a..960eac5 100644
--- a/src/test/java/org/apache/sysml/test/integration/functions/misc/ZeroRowsColsMatrixTest.java
+++ b/src/test/java/org/apache/sysml/test/integration/functions/misc/ZeroRowsColsMatrixTest.java
@@ -53,35 +53,35 @@ public class ZeroRowsColsMatrixTest extends AutomatedTestBase
 		addTestConfiguration(TEST_NAME4, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME4, new String[] { "R" })); 
 	}
 	
-//	@Test
-//	public void testEmptyMatrixRemoveEmptyNoRewritesCP() {
-//		runEmptyMatrixTest(TEST_NAME1, false, ExecType.CP);
-//	}
-//	
-//	@Test
-//	public void testEmptyMatrixRemoveEmptyRewritesCP() {
-//		runEmptyMatrixTest(TEST_NAME1, true, ExecType.CP);
-//	}
-//	
-//	@Test
-//	public void testEmptyMatrixRemoveEmptyNoRewritesMR() {
-//		runEmptyMatrixTest(TEST_NAME1, false, ExecType.MR);
-//	}
-//	
-//	@Test
-//	public void testEmptyMatrixRemoveEmptyRewritesMR() {
-//		runEmptyMatrixTest(TEST_NAME1, true, ExecType.MR);
-//	}
-//	
-//	@Test
-//	public void testEmptyMatrixRemoveEmptyNoRewritesSP() {
-//		runEmptyMatrixTest(TEST_NAME1, false, ExecType.SPARK);
-//	}
-//	
-//	@Test
-//	public void testEmptyMatrixRemoveEmptyRewritesSP() {
-//		runEmptyMatrixTest(TEST_NAME1, true, ExecType.SPARK);
-//	}
+	@Test
+	public void testEmptyMatrixRemoveEmptyNoRewritesCP() {
+		runEmptyMatrixTest(TEST_NAME1, false, ExecType.CP);
+	}
+	
+	@Test
+	public void testEmptyMatrixRemoveEmptyRewritesCP() {
+		runEmptyMatrixTest(TEST_NAME1, true, ExecType.CP);
+	}
+	
+	@Test
+	public void testEmptyMatrixRemoveEmptyNoRewritesMR() {
+		runEmptyMatrixTest(TEST_NAME1, false, ExecType.MR);
+	}
+	
+	@Test
+	public void testEmptyMatrixRemoveEmptyRewritesMR() {
+		runEmptyMatrixTest(TEST_NAME1, true, ExecType.MR);
+	}
+	
+	@Test
+	public void testEmptyMatrixRemoveEmptyNoRewritesSP() {
+		runEmptyMatrixTest(TEST_NAME1, false, ExecType.SPARK);
+	}
+	
+	@Test
+	public void testEmptyMatrixRemoveEmptyRewritesSP() {
+		runEmptyMatrixTest(TEST_NAME1, true, ExecType.SPARK);
+	}
 	
 	@Test
 	public void testEmptyMatrixCbindNoRewritesCP() {
@@ -196,7 +196,7 @@ public class ZeroRowsColsMatrixTest extends AutomatedTestBase
 			
 			String HOME = SCRIPT_DIR + TEST_DIR;
 			fullDMLScriptName = HOME + TEST_NAME + ".dml";
-			programArgs = new String[]{"-args", String.valueOf(dim), output("R")};
+			programArgs = new String[]{"-explain","recompile_runtime","-args", String.valueOf(dim), output("R")};
 			
 			fullRScriptName = HOME + TEST_NAME +".R";
 			rCmd = getRCmd(String.valueOf(dim), expectedDir());

http://git-wip-us.apache.org/repos/asf/systemml/blob/e0b66b30/src/test/scripts/functions/misc/ZeroMatrix_RemoveEmpty.R
----------------------------------------------------------------------
diff --git a/src/test/scripts/functions/misc/ZeroMatrix_RemoveEmpty.R b/src/test/scripts/functions/misc/ZeroMatrix_RemoveEmpty.R
index 900e15e..0814e92 100644
--- a/src/test/scripts/functions/misc/ZeroMatrix_RemoveEmpty.R
+++ b/src/test/scripts/functions/misc/ZeroMatrix_RemoveEmpty.R
@@ -24,6 +24,6 @@ args <- commandArgs(TRUE)
 options(digits=22)
 library("Matrix")
 
-R = matrix(7, as.integer(args[1]), 7);
+R = matrix(7, as.integer(args[1]), 3);
 
 writeMM(as(R, "CsparseMatrix"), paste(args[2], "R", sep="")); 

http://git-wip-us.apache.org/repos/asf/systemml/blob/e0b66b30/src/test/scripts/functions/misc/ZeroMatrix_RemoveEmpty.dml
----------------------------------------------------------------------
diff --git a/src/test/scripts/functions/misc/ZeroMatrix_RemoveEmpty.dml b/src/test/scripts/functions/misc/ZeroMatrix_RemoveEmpty.dml
index 7ca8634..479bd9e 100644
--- a/src/test/scripts/functions/misc/ZeroMatrix_RemoveEmpty.dml
+++ b/src/test/scripts/functions/misc/ZeroMatrix_RemoveEmpty.dml
@@ -21,8 +21,8 @@
 
 
 A = matrix(0, $1, $1);
-B = removeEmpty(target=A, margin="rows", outEmpty=FALSE) + 7;
-C = removeEmpty(target=A, margin="cols", outEmpty=FALSE) + 3;
-R = matrix(7+sum(B)+sum(C), $1, 7);
+B = removeEmpty(target=A, margin="rows", empty.return=FALSE);
+C = removeEmpty(target=A, margin="cols", empty.return=FALSE);
+R = matrix(7+nrow(B)+ncol(C), $1, 3);
 
 write(R, $2);
\ No newline at end of file


[3/3] systemml git commit: [SYSTEMML-2135] Fix aggregate correctness over matrices w/ zero dims

Posted by mb...@apache.org.
[SYSTEMML-2135] Fix aggregate correctness over matrices w/ zero dims

This patch fixes issues of incorrect results of full aggregates over
matrices with zero rows or columns. While sum returns 0, others like
min/max are defined by their initial values, and yet other are undefined
and result in NaNs.

Furthermore, this also includes a fix of simplification rewrites for
empty aggregates (awareness of zero rows/cols) and a general cleanup of
the initial values for min and max, which otherwise would lead to
incorrect results in distributed mr operations.


Project: http://git-wip-us.apache.org/repos/asf/systemml/repo
Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/7e11deaa
Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/7e11deaa
Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/7e11deaa

Branch: refs/heads/master
Commit: 7e11deaa8b22a5749251e629ca1c4b972a6ef054
Parents: 5983e96
Author: Matthias Boehm <mb...@gmail.com>
Authored: Thu Feb 8 20:43:56 2018 -0800
Committer: Matthias Boehm <mb...@gmail.com>
Committed: Thu Feb 8 22:04:16 2018 -0800

----------------------------------------------------------------------
 .../java/org/apache/sysml/hops/UnaryOp.java     | 13 ++---
 .../RewriteAlgebraicSimplificationDynamic.java  | 38 ++++++-------
 .../runtime/codegen/LibSpoofPrimitives.java     |  4 +-
 .../sysml/runtime/codegen/SpoofCellwise.java    | 18 +++---
 .../runtime/codegen/SpoofMultiAggregate.java    |  4 +-
 .../sysml/runtime/compress/ColGroupOffset.java  |  7 +--
 .../sysml/runtime/compress/ColGroupValue.java   |  8 ++-
 .../runtime/compress/CompressedMatrixBlock.java |  3 +-
 .../runtime/instructions/InstructionUtils.java  | 24 ++++----
 .../instructions/spark/SpoofSPInstruction.java  |  4 +-
 .../sysml/runtime/matrix/data/LibMatrixAgg.java | 40 ++++++++++----
 .../sysml/runtime/matrix/data/MatrixBlock.java  |  6 +-
 .../runtime/transform/encode/EncoderBin.java    |  4 +-
 .../functions/misc/ZeroRowsColsMatrixTest.java  | 58 ++++++++++----------
 .../functions/misc/ZeroMatrix_Aggregates.R      |  8 ++-
 .../functions/misc/ZeroMatrix_Aggregates.dml    | 13 +++--
 16 files changed, 136 insertions(+), 116 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/systemml/blob/7e11deaa/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 c844432..e16cb4c 100644
--- a/src/main/java/org/apache/sysml/hops/UnaryOp.java
+++ b/src/main/java/org/apache/sysml/hops/UnaryOp.java
@@ -529,14 +529,13 @@ public class UnaryOp extends Hop implements MultiThreadedHop
 		}
 	}
 
-	private double getCumulativeInitValue()
-	{
+	private double getCumulativeInitValue() {
 		switch( _op ) {
-			case CUMSUM: 	return 0;
-			case CUMPROD: 	return 1;
-			case CUMMIN: 	return Double.MAX_VALUE;
-			case CUMMAX: 	return -Double.MAX_VALUE;
-			default: 		return Double.NaN;
+			case CUMSUM:  return 0;
+			case CUMPROD: return 1;
+			case CUMMIN:  return Double.POSITIVE_INFINITY;
+			case CUMMAX:  return Double.NEGATIVE_INFINITY;
+			default:      return Double.NaN;
 		}
 	}
 	

http://git-wip-us.apache.org/repos/asf/systemml/blob/7e11deaa/src/main/java/org/apache/sysml/hops/rewrite/RewriteAlgebraicSimplificationDynamic.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/hops/rewrite/RewriteAlgebraicSimplificationDynamic.java b/src/main/java/org/apache/sysml/hops/rewrite/RewriteAlgebraicSimplificationDynamic.java
index d64e4b8..f07a379 100644
--- a/src/main/java/org/apache/sysml/hops/rewrite/RewriteAlgebraicSimplificationDynamic.java
+++ b/src/main/java/org/apache/sysml/hops/rewrite/RewriteAlgebraicSimplificationDynamic.java
@@ -767,27 +767,25 @@ public class RewriteAlgebraicSimplificationDynamic extends HopRewriteRule
 			AggUnaryOp uhi = (AggUnaryOp)hi;
 			Hop input = uhi.getInput().get(0);
 			
-			if( HopRewriteUtils.isValidOp(uhi.getOp(), LOOKUP_VALID_EMPTY_AGGREGATE) ){		
-				
-				if( HopRewriteUtils.isEmpty(input) )
-				{
-					Hop hnew = null;
-					if( uhi.getDirection() == Direction.RowCol ) 
-						hnew = new LiteralOp(0.0);
-					else if( uhi.getDirection() == Direction.Col ) 
-						hnew = HopRewriteUtils.createDataGenOp(uhi, input, 0); //nrow(uhi)=1
-					else //if( uhi.getDirection() == Direction.Row ) 
-						hnew = HopRewriteUtils.createDataGenOp(input, uhi, 0); //ncol(uhi)=1
-					
-					//add new child to parent input
-					HopRewriteUtils.replaceChildReference(parent, hi, hnew, pos);
-					hi = hnew;
-					
-					LOG.debug("Applied simplifyEmptyAggregate");
-				}
-			}			
+			//check for valid empty aggregates, except for matrices with zero rows/cols
+			if( HopRewriteUtils.isValidOp(uhi.getOp(), LOOKUP_VALID_EMPTY_AGGREGATE) 
+				&& HopRewriteUtils.isEmpty(input)
+				&& input.getDim1()>=1 && input.getDim2() >= 1 )
+			{
+				Hop hnew = null;
+				if( uhi.getDirection() == Direction.RowCol ) 
+					hnew = new LiteralOp(0.0);
+				else if( uhi.getDirection() == Direction.Col ) 
+					hnew = HopRewriteUtils.createDataGenOp(uhi, input, 0); //nrow(uhi)=1
+				else //if( uhi.getDirection() == Direction.Row ) 
+					hnew = HopRewriteUtils.createDataGenOp(input, uhi, 0); //ncol(uhi)=1
+				
+				//add new child to parent input
+				HopRewriteUtils.replaceChildReference(parent, hi, hnew, pos);
+				hi = hnew;
+				LOG.debug("Applied simplifyEmptyAggregate");
+			}
 		}
-		
 		return hi;
 	}
 	

http://git-wip-us.apache.org/repos/asf/systemml/blob/7e11deaa/src/main/java/org/apache/sysml/runtime/codegen/LibSpoofPrimitives.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/codegen/LibSpoofPrimitives.java b/src/main/java/org/apache/sysml/runtime/codegen/LibSpoofPrimitives.java
index 8d76e14..473d849 100644
--- a/src/main/java/org/apache/sysml/runtime/codegen/LibSpoofPrimitives.java
+++ b/src/main/java/org/apache/sysml/runtime/codegen/LibSpoofPrimitives.java
@@ -302,7 +302,7 @@ public class LibSpoofPrimitives
 	}
 	
 	public static double vectMin(double[] a, int ai, int len) { 
-		double val = Double.MAX_VALUE;
+		double val = Double.POSITIVE_INFINITY;
 		for( int i = ai; i < ai+len; i++ )
 			val = Math.min(a[i], val);
 		return val; 
@@ -314,7 +314,7 @@ public class LibSpoofPrimitives
 	}
 	
 	public static double vectMax(double[] a, int ai, int len) { 
-		double val = -Double.MAX_VALUE;
+		double val = Double.NEGATIVE_INFINITY;
 		for( int i = ai; i < ai+len; i++ )
 			val = Math.max(a[i], val);
 		return val; 

http://git-wip-us.apache.org/repos/asf/systemml/blob/7e11deaa/src/main/java/org/apache/sysml/runtime/codegen/SpoofCellwise.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/codegen/SpoofCellwise.java b/src/main/java/org/apache/sysml/runtime/codegen/SpoofCellwise.java
index 7c2ac35..5080e6c 100644
--- a/src/main/java/org/apache/sysml/runtime/codegen/SpoofCellwise.java
+++ b/src/main/java/org/apache/sysml/runtime/codegen/SpoofCellwise.java
@@ -487,7 +487,7 @@ public abstract class SpoofCellwise extends SpoofOperator implements Serializabl
 	{
 		double[] lc = c.valuesAt(0); //single block
 		
-		double initialVal = (_aggOp==AggOp.MIN) ? Double.MAX_VALUE : -Double.MAX_VALUE;
+		double initialVal = (_aggOp==AggOp.MIN) ? Double.POSITIVE_INFINITY : Double.NEGATIVE_INFINITY;
 		ValueFunction vfun = getAggFunction();
 		long lnnz = 0;
 		if( a == null && !sparseSafe ) { //empty
@@ -559,7 +559,7 @@ public abstract class SpoofCellwise extends SpoofOperator implements Serializabl
 	{
 		double[] lc = c.valuesAt(0); //single block
 		
-		double initialVal = (_aggOp==AggOp.MIN) ? Double.MAX_VALUE : -Double.MAX_VALUE;
+		double initialVal = (_aggOp==AggOp.MIN) ? Double.POSITIVE_INFINITY : Double.NEGATIVE_INFINITY;
 		ValueFunction vfun = getAggFunction();
 		Arrays.fill(lc, initialVal);
 		
@@ -622,7 +622,7 @@ public abstract class SpoofCellwise extends SpoofOperator implements Serializabl
 	{
 		//safe aggregation for min/max w/ handling of zero entries
 		//note: sparse safe with zero value as min/max handled outside
-		double ret = (_aggOp==AggOp.MIN) ? Double.MAX_VALUE : -Double.MAX_VALUE; 
+		double ret = (_aggOp==AggOp.MIN) ? Double.POSITIVE_INFINITY : Double.NEGATIVE_INFINITY;
 		ValueFunction vfun = getAggFunction();
 		
 		if( a == null && !sparseSafe ) {
@@ -764,7 +764,7 @@ public abstract class SpoofCellwise extends SpoofOperator implements Serializabl
 			MatrixBlock out, int m, int n, boolean sparseSafe, int rl, int ru)
 		throws DMLRuntimeException
 	{
-		double initialVal = (_aggOp==AggOp.MIN) ? Double.MAX_VALUE : -Double.MAX_VALUE;
+		double initialVal = (_aggOp==AggOp.MIN) ? Double.POSITIVE_INFINITY : Double.NEGATIVE_INFINITY;
 		ValueFunction vfun = getAggFunction();
 		
 		//note: sequential scan algorithm for both sparse-safe and -unsafe 
@@ -852,7 +852,7 @@ public abstract class SpoofCellwise extends SpoofOperator implements Serializabl
 			MatrixBlock out, int m, int n, boolean sparseSafe, int rl, int ru)
 		throws DMLRuntimeException
 	{
-		double initialVal = (_aggOp==AggOp.MIN) ? Double.MAX_VALUE : -Double.MAX_VALUE;
+		double initialVal = (_aggOp==AggOp.MIN) ? Double.POSITIVE_INFINITY : Double.NEGATIVE_INFINITY;
 		ValueFunction vfun = getAggFunction();
 		double[] c = out.getDenseBlockValues();
 		Arrays.fill(c, initialVal);
@@ -929,7 +929,7 @@ public abstract class SpoofCellwise extends SpoofOperator implements Serializabl
 			int m, int n, boolean sparseSafe, int rl, int ru)
 		throws DMLRuntimeException
 	{
-		double ret = (_aggOp==AggOp.MIN) ? Double.MAX_VALUE : -Double.MAX_VALUE;
+		double ret = (_aggOp==AggOp.MIN) ? Double.POSITIVE_INFINITY : Double.NEGATIVE_INFINITY;
 		ret = (sparseSafe && sblock.size() < (long)m*n) ? 0 : ret;
 		ValueFunction vfun = getAggFunction();
 		
@@ -1019,7 +1019,7 @@ public abstract class SpoofCellwise extends SpoofOperator implements Serializabl
 			double[] c, int m, int n, boolean sparseSafe, int rl, int ru)
 		throws DMLRuntimeException
 	{
-		Arrays.fill(c, rl, ru, (_aggOp==AggOp.MIN) ? Double.MAX_VALUE : -Double.MAX_VALUE);
+		Arrays.fill(c, rl, ru, (_aggOp==AggOp.MIN) ? Double.POSITIVE_INFINITY : Double.NEGATIVE_INFINITY);
 		ValueFunction vfun = getAggFunction();
 		long lnnz = 0;
 		Iterator<IJV> iter = a.getIterator(rl, ru, !sparseSafe);
@@ -1056,7 +1056,7 @@ public abstract class SpoofCellwise extends SpoofOperator implements Serializabl
 			double[] c, int m, int n, boolean sparseSafe, int rl, int ru)
 		throws DMLRuntimeException
 	{
-		Arrays.fill(c, rl, ru, (_aggOp==AggOp.MIN) ? Double.MAX_VALUE : -Double.MAX_VALUE);
+		Arrays.fill(c, rl, ru, (_aggOp==AggOp.MIN) ? Double.POSITIVE_INFINITY : Double.NEGATIVE_INFINITY);
 		ValueFunction vfun = getAggFunction();
 		long lnnz = 0;
 		Iterator<IJV> iter = a.getIterator(rl, ru, !sparseSafe);
@@ -1112,7 +1112,7 @@ public abstract class SpoofCellwise extends SpoofOperator implements Serializabl
 	{
 		//safe aggregation for min/max w/ handling of zero entries
 		//note: sparse safe with zero value as min/max handled outside
-		double ret = (_aggOp==AggOp.MIN) ? Double.MAX_VALUE : -Double.MAX_VALUE;
+		double ret = (_aggOp==AggOp.MIN) ? Double.POSITIVE_INFINITY : Double.NEGATIVE_INFINITY;
 		ValueFunction vfun = getAggFunction();
 		
 		Iterator<IJV> iter = a.getIterator(rl, ru, !sparseSafe);

http://git-wip-us.apache.org/repos/asf/systemml/blob/7e11deaa/src/main/java/org/apache/sysml/runtime/codegen/SpoofMultiAggregate.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/codegen/SpoofMultiAggregate.java b/src/main/java/org/apache/sysml/runtime/codegen/SpoofMultiAggregate.java
index 85c894a..72f7764 100644
--- a/src/main/java/org/apache/sysml/runtime/codegen/SpoofMultiAggregate.java
+++ b/src/main/java/org/apache/sysml/runtime/codegen/SpoofMultiAggregate.java
@@ -224,8 +224,8 @@ public abstract class SpoofMultiAggregate extends SpoofOperator implements Seria
 		switch( aggop ) {
 			case SUM:
 			case SUM_SQ: return 0; 
-			case MIN:    return Double.MAX_VALUE;
-			case MAX:    return -Double.MAX_VALUE;
+			case MIN:    return Double.POSITIVE_INFINITY;
+			case MAX:    return Double.NEGATIVE_INFINITY;
 		}
 		return 0;
 	}

http://git-wip-us.apache.org/repos/asf/systemml/blob/7e11deaa/src/main/java/org/apache/sysml/runtime/compress/ColGroupOffset.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/compress/ColGroupOffset.java b/src/main/java/org/apache/sysml/runtime/compress/ColGroupOffset.java
index 3603b9e..a312419 100644
--- a/src/main/java/org/apache/sysml/runtime/compress/ColGroupOffset.java
+++ b/src/main/java/org/apache/sysml/runtime/compress/ColGroupOffset.java
@@ -239,12 +239,11 @@ public abstract class ColGroupOffset extends ColGroupValue
 			LinearAlgebraUtils.vectMultiplyAdd(b[i], _values, c, off, 0, numVals);
 	}
 
-	protected final double mxxValues(int bitmapIx, Builtin builtin)
-	{
+	protected final double mxxValues(int bitmapIx, Builtin builtin) {
 		final int numCols = getNumCols();
 		final int valOff = bitmapIx * numCols;
-		
-		double val = Double.MAX_VALUE * ((builtin.getBuiltinCode()==BuiltinCode.MAX)?-1:1);
+		double val = (builtin.getBuiltinCode()==BuiltinCode.MAX) ?
+			Double.NEGATIVE_INFINITY : Double.POSITIVE_INFINITY;
 		for( int i = 0; i < numCols; i++ )
 			val = builtin.execute2(val, _values[valOff+i]);
 		

http://git-wip-us.apache.org/repos/asf/systemml/blob/7e11deaa/src/main/java/org/apache/sysml/runtime/compress/ColGroupValue.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/compress/ColGroupValue.java b/src/main/java/org/apache/sysml/runtime/compress/ColGroupValue.java
index 12bce84..a416c5e 100644
--- a/src/main/java/org/apache/sysml/runtime/compress/ColGroupValue.java
+++ b/src/main/java/org/apache/sysml/runtime/compress/ColGroupValue.java
@@ -266,7 +266,8 @@ public abstract class ColGroupValue extends ColGroup
 	protected void computeMxx(MatrixBlock result, Builtin builtin, boolean zeros) 
 	{
 		//init and 0-value handling
-		double val = Double.MAX_VALUE * ((builtin.getBuiltinCode()==BuiltinCode.MAX)?-1:1);
+		double val = (builtin.getBuiltinCode()==BuiltinCode.MAX) ?
+			Double.NEGATIVE_INFINITY : Double.POSITIVE_INFINITY;
 		if( zeros )
 			val = builtin.execute2(val, 0);
 		
@@ -296,10 +297,11 @@ public abstract class ColGroupValue extends ColGroup
 		
 		//init and 0-value handling
 		double[] vals = new double[numCols];
-		Arrays.fill(vals, Double.MAX_VALUE * ((builtin.getBuiltinCode()==BuiltinCode.MAX)?-1:1));
+		Arrays.fill(vals, (builtin.getBuiltinCode()==BuiltinCode.MAX) ?
+			Double.NEGATIVE_INFINITY : Double.POSITIVE_INFINITY);
 		if( zeros ) {
 			for( int j = 0; j < numCols; j++ )
-				vals[j] = builtin.execute2(vals[j], 0);		
+				vals[j] = builtin.execute2(vals[j], 0);
 		}
 		
 		//iterate over all values only

http://git-wip-us.apache.org/repos/asf/systemml/blob/7e11deaa/src/main/java/org/apache/sysml/runtime/compress/CompressedMatrixBlock.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/compress/CompressedMatrixBlock.java b/src/main/java/org/apache/sysml/runtime/compress/CompressedMatrixBlock.java
index d9cfc72..e5f8827 100644
--- a/src/main/java/org/apache/sysml/runtime/compress/CompressedMatrixBlock.java
+++ b/src/main/java/org/apache/sysml/runtime/compress/CompressedMatrixBlock.java
@@ -1185,7 +1185,8 @@ public class CompressedMatrixBlock extends MatrixBlock implements Externalizable
 		
 		//special handling init value for rowmins/rowmax
 		if( op.indexFn instanceof ReduceCol && op.aggOp.increOp.fn instanceof Builtin ) {
-			double val = Double.MAX_VALUE * ((((Builtin)op.aggOp.increOp.fn).getBuiltinCode()==BuiltinCode.MAX)?-1:1);
+			double val = (((Builtin)op.aggOp.increOp.fn).getBuiltinCode()==BuiltinCode.MAX) ?
+				Double.NEGATIVE_INFINITY : Double.POSITIVE_INFINITY;
 			ret.getDenseBlock().set(val);
 		}
 		

http://git-wip-us.apache.org/repos/asf/systemml/blob/7e11deaa/src/main/java/org/apache/sysml/runtime/instructions/InstructionUtils.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/instructions/InstructionUtils.java b/src/main/java/org/apache/sysml/runtime/instructions/InstructionUtils.java
index ff85aa7..172aa15 100644
--- a/src/main/java/org/apache/sysml/runtime/instructions/InstructionUtils.java
+++ b/src/main/java/org/apache/sysml/runtime/instructions/InstructionUtils.java
@@ -352,11 +352,11 @@ public class InstructionUtils
 			aggun = new AggregateUnaryOperator(agg, ReduceAll.getReduceAllFnObject(), numThreads);
 		} 
 		else if ( opcode.equalsIgnoreCase("uamax") ) {
-			AggregateOperator agg = new AggregateOperator(-Double.MAX_VALUE, Builtin.getBuiltinFnObject("max"));
+			AggregateOperator agg = new AggregateOperator(Double.NEGATIVE_INFINITY, Builtin.getBuiltinFnObject("max"));
 			aggun = new AggregateUnaryOperator(agg, ReduceAll.getReduceAllFnObject(), numThreads);
 		}
 		else if ( opcode.equalsIgnoreCase("uamin") ) {
-			AggregateOperator agg = new AggregateOperator(Double.MAX_VALUE, Builtin.getBuiltinFnObject("min"));
+			AggregateOperator agg = new AggregateOperator(Double.POSITIVE_INFINITY, Builtin.getBuiltinFnObject("min"));
 			aggun = new AggregateUnaryOperator(agg, ReduceAll.getReduceAllFnObject(), numThreads);
 		} 
 		else if ( opcode.equalsIgnoreCase("uatrace") ) {
@@ -368,27 +368,27 @@ public class InstructionUtils
 			aggun = new AggregateUnaryOperator(agg, ReduceDiag.getReduceDiagFnObject(), numThreads);
 		} 		
 		else if ( opcode.equalsIgnoreCase("uarmax") ) {
-			AggregateOperator agg = new AggregateOperator(-Double.MAX_VALUE, Builtin.getBuiltinFnObject("max"));
+			AggregateOperator agg = new AggregateOperator(Double.NEGATIVE_INFINITY, Builtin.getBuiltinFnObject("max"));
 			aggun = new AggregateUnaryOperator(agg, ReduceCol.getReduceColFnObject(), numThreads);
 		} 
 		else if (opcode.equalsIgnoreCase("uarimax") ) {
-			AggregateOperator agg = new AggregateOperator(-Double.MAX_VALUE, Builtin.getBuiltinFnObject("maxindex"), true, CorrectionLocationType.LASTCOLUMN);
+			AggregateOperator agg = new AggregateOperator(Double.NEGATIVE_INFINITY, Builtin.getBuiltinFnObject("maxindex"), true, CorrectionLocationType.LASTCOLUMN);
 			aggun = new AggregateUnaryOperator(agg, ReduceCol.getReduceColFnObject(), numThreads);
 		}
 		else if ( opcode.equalsIgnoreCase("uarmin") ) {
-			AggregateOperator agg = new AggregateOperator(Double.MAX_VALUE, Builtin.getBuiltinFnObject("min"));
+			AggregateOperator agg = new AggregateOperator(Double.POSITIVE_INFINITY, Builtin.getBuiltinFnObject("min"));
 			aggun = new AggregateUnaryOperator(agg, ReduceCol.getReduceColFnObject(), numThreads);
 		} 
 		else if (opcode.equalsIgnoreCase("uarimin") ) {
-			AggregateOperator agg = new AggregateOperator(Double.MAX_VALUE, Builtin.getBuiltinFnObject("minindex"), true, CorrectionLocationType.LASTCOLUMN);
+			AggregateOperator agg = new AggregateOperator(Double.POSITIVE_INFINITY, Builtin.getBuiltinFnObject("minindex"), true, CorrectionLocationType.LASTCOLUMN);
 			aggun = new AggregateUnaryOperator(agg, ReduceCol.getReduceColFnObject(), numThreads);
 		}
 		else if ( opcode.equalsIgnoreCase("uacmax") ) {
-			AggregateOperator agg = new AggregateOperator(-Double.MAX_VALUE, Builtin.getBuiltinFnObject("max"));
+			AggregateOperator agg = new AggregateOperator(Double.NEGATIVE_INFINITY, Builtin.getBuiltinFnObject("max"));
 			aggun = new AggregateUnaryOperator(agg, ReduceRow.getReduceRowFnObject(), numThreads);
 		} 
 		else if ( opcode.equalsIgnoreCase("uacmin") ) {
-			AggregateOperator agg = new AggregateOperator(Double.MAX_VALUE, Builtin.getBuiltinFnObject("min"));
+			AggregateOperator agg = new AggregateOperator(Double.POSITIVE_INFINITY, Builtin.getBuiltinFnObject("min"));
 			aggun = new AggregateUnaryOperator(agg, ReduceRow.getReduceRowFnObject(), numThreads);
 		}
 		
@@ -430,16 +430,16 @@ public class InstructionUtils
 			agg = new AggregateOperator(1, Multiply.getMultiplyFnObject());
 		}
 		else if (opcode.equalsIgnoreCase("arimax")){
-			agg = new AggregateOperator(-Double.MAX_VALUE, Builtin.getBuiltinFnObject("maxindex"), true, CorrectionLocationType.LASTCOLUMN);
+			agg = new AggregateOperator(Double.NEGATIVE_INFINITY, Builtin.getBuiltinFnObject("maxindex"), true, CorrectionLocationType.LASTCOLUMN);
 		}
 		else if ( opcode.equalsIgnoreCase("amax") ) {
-			agg = new AggregateOperator(-Double.MAX_VALUE, Builtin.getBuiltinFnObject("max"));
+			agg = new AggregateOperator(Double.NEGATIVE_INFINITY, Builtin.getBuiltinFnObject("max"));
 		}
 		else if ( opcode.equalsIgnoreCase("amin") ) {
-			agg = new AggregateOperator(Double.MAX_VALUE, Builtin.getBuiltinFnObject("min"));
+			agg = new AggregateOperator(Double.POSITIVE_INFINITY, Builtin.getBuiltinFnObject("min"));
 		}
 		else if (opcode.equalsIgnoreCase("arimin")){
-			agg = new AggregateOperator(Double.MAX_VALUE, Builtin.getBuiltinFnObject("minindex"), true, CorrectionLocationType.LASTCOLUMN);
+			agg = new AggregateOperator(Double.POSITIVE_INFINITY, Builtin.getBuiltinFnObject("minindex"), true, CorrectionLocationType.LASTCOLUMN);
 		}
 		else if ( opcode.equalsIgnoreCase("amean") ) {
 			boolean lcorrExists = (corrExists==null) ? true : Boolean.parseBoolean(corrExists);

http://git-wip-us.apache.org/repos/asf/systemml/blob/7e11deaa/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 da39471..4022201 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
@@ -695,9 +695,9 @@ public class SpoofSPInstruction extends SPInstruction {
 		if( aggop == AggOp.SUM || aggop == AggOp.SUM_SQ )
 			return new AggregateOperator(0, KahanPlus.getKahanPlusFnObject(), true, CorrectionLocationType.NONE);
 		else if( aggop == AggOp.MIN )
-			return new AggregateOperator(Double.MAX_VALUE, Builtin.getBuiltinFnObject(BuiltinCode.MIN), false, CorrectionLocationType.NONE);
+			return new AggregateOperator(Double.POSITIVE_INFINITY, Builtin.getBuiltinFnObject(BuiltinCode.MIN), false, CorrectionLocationType.NONE);
 		else if( aggop == AggOp.MAX )
-			return new AggregateOperator(-Double.MAX_VALUE, Builtin.getBuiltinFnObject(BuiltinCode.MAX), false, CorrectionLocationType.NONE);
+			return new AggregateOperator(Double.NEGATIVE_INFINITY, Builtin.getBuiltinFnObject(BuiltinCode.MAX), false, CorrectionLocationType.NONE);
 		return null;
 	}
 }

http://git-wip-us.apache.org/repos/asf/systemml/blob/7e11deaa/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixAgg.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixAgg.java b/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixAgg.java
index b56397e..ac8a185 100644
--- a/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixAgg.java
+++ b/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixAgg.java
@@ -1331,13 +1331,13 @@ public class LibMatrixAgg
 			}
 			case CUM_MIN:
 			case CUM_MAX: {
-				double init = Double.MAX_VALUE * ((optype==AggType.CUM_MAX)?-1:1);
+				double init = (optype==AggType.CUM_MAX) ? Double.NEGATIVE_INFINITY:Double.POSITIVE_INFINITY;
 				d_ucummxx(in.getDenseBlockValues(), null, out.getDenseBlockValues(), n, init, (Builtin)vFn, rl, ru);
 				break;
 			}
 			case MIN: 
 			case MAX: { //MAX/MIN
-				double init = Double.MAX_VALUE * ((optype==AggType.MAX)?-1:1);
+				double init = (optype==AggType.MAX) ? Double.NEGATIVE_INFINITY:Double.POSITIVE_INFINITY;
 				if( ixFn instanceof ReduceAll ) // MIN/MAX
 					d_uamxx(a, c, n, init, (Builtin)vFn, rl, ru);
 				else if( ixFn instanceof ReduceCol ) //ROWMIN/ROWMAX
@@ -1347,13 +1347,13 @@ public class LibMatrixAgg
 				break;
 			}
 			case MAX_INDEX: {
-				double init = -Double.MAX_VALUE;
+				double init = Double.NEGATIVE_INFINITY;
 				if( ixFn instanceof ReduceCol ) //ROWINDEXMAX
 					d_uarimxx(a, c, n, init, (Builtin)vFn, rl, ru);
 				break;
 			}
 			case MIN_INDEX: {
-				double init = Double.MAX_VALUE;
+				double init = Double.POSITIVE_INFINITY;
 				if( ixFn instanceof ReduceCol ) //ROWINDEXMIN
 					d_uarimin(a, c, n, init, (Builtin)vFn, rl, ru);
 				break;
@@ -1435,13 +1435,13 @@ public class LibMatrixAgg
 			}
 			case CUM_MIN:
 			case CUM_MAX: {
-				double init = Double.MAX_VALUE * ((optype==AggType.CUM_MAX)?-1:1);
+				double init = (optype==AggType.CUM_MAX) ? Double.NEGATIVE_INFINITY:Double.POSITIVE_INFINITY;
 				s_ucummxx(a, null, out.getDenseBlockValues(), n, init, (Builtin)vFn, rl, ru);
 				break;
 			}
 			case MIN:
 			case MAX: { //MAX/MIN
-				double init = Double.MAX_VALUE * ((optype==AggType.MAX)?-1:1);
+				double init = (optype==AggType.MAX) ? Double.NEGATIVE_INFINITY:Double.POSITIVE_INFINITY;
 				if( ixFn instanceof ReduceAll ) // MIN/MAX
 					s_uamxx(a, c, n, init, (Builtin)vFn, rl, ru);
 				else if( ixFn instanceof ReduceCol ) //ROWMIN/ROWMAX
@@ -1451,13 +1451,13 @@ public class LibMatrixAgg
 				break;
 			}
 			case MAX_INDEX: {
-				double init = -Double.MAX_VALUE;
+				double init = Double.NEGATIVE_INFINITY;
 				if( ixFn instanceof ReduceCol ) //ROWINDEXMAX
 					s_uarimxx(a, c, n, init, (Builtin)vFn, rl, ru);
 				break;
 			}
 			case MIN_INDEX: {
-				double init = Double.MAX_VALUE;
+				double init = Double.POSITIVE_INFINITY;
 				if( ixFn instanceof ReduceCol ) //ROWINDEXMAX
 					s_uarimin(a, c, n, init, (Builtin)vFn, rl, ru);
 				break;
@@ -1516,7 +1516,7 @@ public class LibMatrixAgg
 			}
 			case CUM_MIN:
 			case CUM_MAX: {
-				double init = Double.MAX_VALUE * ((optype==AggType.CUM_MAX)?-1:1);
+				double init = (optype==AggType.CUM_MAX)? Double.NEGATIVE_INFINITY:Double.POSITIVE_INFINITY;
 				d_ucummxx(a, agg, c, n, init, (Builtin)vFn, rl, ru);
 				break;
 			}
@@ -1548,7 +1548,7 @@ public class LibMatrixAgg
 			}
 			case CUM_MIN:
 			case CUM_MAX: {
-				double init = Double.MAX_VALUE * ((optype==AggType.CUM_MAX)?-1:1);
+				double init = (optype==AggType.CUM_MAX) ? Double.NEGATIVE_INFINITY:Double.POSITIVE_INFINITY;
 				s_ucummxx(a, agg, c, n, init, (Builtin)vFn, rl, ru);
 				break;
 			}
@@ -1560,7 +1560,25 @@ public class LibMatrixAgg
 	private static MatrixBlock aggregateUnaryMatrixEmpty(MatrixBlock in, MatrixBlock out, AggType optype, IndexFunction ixFn) 
 		throws DMLRuntimeException
 	{
-		//do nothing for pseudo sparse-safe operations
+		//handle all full aggregates over matrices with zero rows or columns
+		if( ixFn instanceof ReduceAll && (in.getNumRows() == 0 || in.getNumColumns() == 0) ) {
+			double val = Double.NaN;
+			switch( optype ) {
+				case KAHAN_SUM:
+				case KAHAN_SUM_SQ: val = 0; break;
+				case MIN:          val = Double.POSITIVE_INFINITY; break;
+				case MAX:          val = Double.NEGATIVE_INFINITY; break;
+				case MEAN:
+				case VAR:
+				case MIN_INDEX:
+				case MAX_INDEX:
+				default:           val = Double.NaN; break;
+			}
+			out.quickSetValue(0, 0, val);
+			return out;
+		}
+		
+		//handle pseudo sparse-safe operations over empty inputs 
 		if(optype==AggType.KAHAN_SUM || optype==AggType.KAHAN_SUM_SQ
 				|| optype==AggType.MIN || optype==AggType.MAX || optype==AggType.PROD
 				|| optype == AggType.CUM_KAHAN_SUM || optype == AggType.CUM_PROD

http://git-wip-us.apache.org/repos/asf/systemml/blob/7e11deaa/src/main/java/org/apache/sysml/runtime/matrix/data/MatrixBlock.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/matrix/data/MatrixBlock.java b/src/main/java/org/apache/sysml/runtime/matrix/data/MatrixBlock.java
index c59ec51..b10e153 100644
--- a/src/main/java/org/apache/sysml/runtime/matrix/data/MatrixBlock.java
+++ b/src/main/java/org/apache/sysml/runtime/matrix/data/MatrixBlock.java
@@ -798,7 +798,7 @@ public class MatrixBlock extends MatrixValue implements CacheBlock, Externalizab
 			return -1;
 		
 		//NOTE: usually this method is only applied on dense vectors and hence not really tuned yet.
-		double min = Double.MAX_VALUE;
+		double min = Double.POSITIVE_INFINITY;
 		for( int i=0; i<rlen; i++ )
 			for( int j=0; j<clen; j++ ){
 				double val = quickGetValue(i, j);
@@ -819,7 +819,7 @@ public class MatrixBlock extends MatrixValue implements CacheBlock, Externalizab
 		throws DMLRuntimeException
 	{
 		//construct operator
-		AggregateOperator aop = new AggregateOperator(Double.MAX_VALUE, Builtin.getBuiltinFnObject("min"));
+		AggregateOperator aop = new AggregateOperator(Double.POSITIVE_INFINITY, Builtin.getBuiltinFnObject("min"));
 		AggregateUnaryOperator auop = new AggregateUnaryOperator( aop, ReduceAll.getReduceAllFnObject());
 		
 		//execute operation
@@ -839,7 +839,7 @@ public class MatrixBlock extends MatrixValue implements CacheBlock, Externalizab
 		throws DMLRuntimeException
 	{
 		//construct operator
-		AggregateOperator aop = new AggregateOperator(-Double.MAX_VALUE, Builtin.getBuiltinFnObject("max"));
+		AggregateOperator aop = new AggregateOperator(Double.NEGATIVE_INFINITY, Builtin.getBuiltinFnObject("max"));
 		AggregateUnaryOperator auop = new AggregateUnaryOperator( aop, ReduceAll.getReduceAllFnObject());
 		
 		//execute operation

http://git-wip-us.apache.org/repos/asf/systemml/blob/7e11deaa/src/main/java/org/apache/sysml/runtime/transform/encode/EncoderBin.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/transform/encode/EncoderBin.java b/src/main/java/org/apache/sysml/runtime/transform/encode/EncoderBin.java
index e70a392..016adb4 100644
--- a/src/main/java/org/apache/sysml/runtime/transform/encode/EncoderBin.java
+++ b/src/main/java/org/apache/sysml/runtime/transform/encode/EncoderBin.java
@@ -79,9 +79,9 @@ public class EncoderBin extends Encoder
 			
 			// initialize internal transformation metadata
 			_min = new double[_colList.length];
-			Arrays.fill(_min, Double.MAX_VALUE);
+			Arrays.fill(_min, Double.POSITIVE_INFINITY);
 			_max = new double[_colList.length];
-			Arrays.fill(_max, -Double.MAX_VALUE);
+			Arrays.fill(_max, Double.NEGATIVE_INFINITY);
 		}
 	}
 

http://git-wip-us.apache.org/repos/asf/systemml/blob/7e11deaa/src/test/java/org/apache/sysml/test/integration/functions/misc/ZeroRowsColsMatrixTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/sysml/test/integration/functions/misc/ZeroRowsColsMatrixTest.java b/src/test/java/org/apache/sysml/test/integration/functions/misc/ZeroRowsColsMatrixTest.java
index 960eac5..7574fb6 100644
--- a/src/test/java/org/apache/sysml/test/integration/functions/misc/ZeroRowsColsMatrixTest.java
+++ b/src/test/java/org/apache/sysml/test/integration/functions/misc/ZeroRowsColsMatrixTest.java
@@ -143,35 +143,35 @@ public class ZeroRowsColsMatrixTest extends AutomatedTestBase
 		runEmptyMatrixTest(TEST_NAME3, true, ExecType.SPARK);
 	}
 	
-//	@Test
-//	public void testEmptyMatrixAggregatesNoRewritesCP() {
-//		runEmptyMatrixTest(TEST_NAME4, false, ExecType.CP);
-//	}
-//	
-//	@Test
-//	public void testEmptyMatrixAggregatesRewritesCP() {
-//		runEmptyMatrixTest(TEST_NAME4, true, ExecType.CP);
-//	}
-//	
-//	@Test
-//	public void testEmptyMatrixAggregatesNoRewritesMR() {
-//		runEmptyMatrixTest(TEST_NAME4, false, ExecType.MR);
-//	}
-//	
-//	@Test
-//	public void testEmptyMatrixAggregatesRewritesMR() {
-//		runEmptyMatrixTest(TEST_NAME4, true, ExecType.MR);
-//	}
-//	
-//	@Test
-//	public void testEmptyMatrixAggregatesNoRewritesSP() {
-//		runEmptyMatrixTest(TEST_NAME4, false, ExecType.SPARK);
-//	}
-//	
-//	@Test
-//	public void testEmptyMatrixAggregatesRewritesSP() {
-//		runEmptyMatrixTest(TEST_NAME4, true, ExecType.SPARK);
-//	}
+	@Test
+	public void testEmptyMatrixAggregatesNoRewritesCP() {
+		runEmptyMatrixTest(TEST_NAME4, false, ExecType.CP);
+	}
+	
+	@Test
+	public void testEmptyMatrixAggregatesRewritesCP() {
+		runEmptyMatrixTest(TEST_NAME4, true, ExecType.CP);
+	}
+	
+	@Test
+	public void testEmptyMatrixAggregatesNoRewritesMR() {
+		runEmptyMatrixTest(TEST_NAME4, false, ExecType.MR);
+	}
+	
+	@Test
+	public void testEmptyMatrixAggregatesRewritesMR() {
+		runEmptyMatrixTest(TEST_NAME4, true, ExecType.MR);
+	}
+	
+	@Test
+	public void testEmptyMatrixAggregatesNoRewritesSP() {
+		runEmptyMatrixTest(TEST_NAME4, false, ExecType.SPARK);
+	}
+	
+	@Test
+	public void testEmptyMatrixAggregatesRewritesSP() {
+		runEmptyMatrixTest(TEST_NAME4, true, ExecType.SPARK);
+	}
 	
 	private void runEmptyMatrixTest( String testname, boolean rewrites, ExecType et )
 	{

http://git-wip-us.apache.org/repos/asf/systemml/blob/7e11deaa/src/test/scripts/functions/misc/ZeroMatrix_Aggregates.R
----------------------------------------------------------------------
diff --git a/src/test/scripts/functions/misc/ZeroMatrix_Aggregates.R b/src/test/scripts/functions/misc/ZeroMatrix_Aggregates.R
index a7f00ba..bc105e6 100644
--- a/src/test/scripts/functions/misc/ZeroMatrix_Aggregates.R
+++ b/src/test/scripts/functions/misc/ZeroMatrix_Aggregates.R
@@ -26,9 +26,11 @@ library("Matrix")
 
 n = as.integer(args[1]);
 X = matrix(0, n, 0);
-R = rbind(rbind(
+R = rbind(rbind(rbind(rbind(
   as.matrix(sum(X)==0),
   as.matrix(min(X)==(1.0/0.0))),
-  as.matrix(max(X)==(-1.0/0.0)));
+  as.matrix(max(X)==(-1.0/0.0))),
+  as.matrix(is.nan(mean(X)))),
+  as.matrix(is.na(sd(X))));
 
-writeMM(as(R, "CsparseMatrix"), paste(args[2], "R", sep="")); 
+writeMM(as(R, "CsparseMatrix"), paste(args[2], "R", sep=""));

http://git-wip-us.apache.org/repos/asf/systemml/blob/7e11deaa/src/test/scripts/functions/misc/ZeroMatrix_Aggregates.dml
----------------------------------------------------------------------
diff --git a/src/test/scripts/functions/misc/ZeroMatrix_Aggregates.dml b/src/test/scripts/functions/misc/ZeroMatrix_Aggregates.dml
index 690c7b6..f26f516 100644
--- a/src/test/scripts/functions/misc/ZeroMatrix_Aggregates.dml
+++ b/src/test/scripts/functions/misc/ZeroMatrix_Aggregates.dml
@@ -21,11 +21,12 @@
 
 
 X = matrix(0, $1, 0);
-print(min(X))
-R = rbind(
-  as.matrix(sum(X)==0),
-  as.matrix(min(X)==(1.0/0.0)),
-  as.matrix(max(X)==(-1.0/0.0))
-);
+# nary rbind not applicable because not supported in MR
+R = rbind(rbind(rbind(rbind(
+  as.matrix(sum(X)==0),           # 0
+  as.matrix(min(X)==(1.0/0.0))),  # INF
+  as.matrix(max(X)==(-1.0/0.0))), # -INF
+  as.matrix(mean(X)!=mean(X))),   # NaN
+  as.matrix(sd(X)!=sd(X)));       # NaN
 
 write(R, $2);


[2/3] systemml git commit: [SYSTEMML-2139] Fix codegen with boolean literal inputs (e.g., ifelse)

Posted by mb...@apache.org.
[SYSTEMML-2139] Fix codegen with boolean literal inputs (e.g., ifelse)

This patch fixes the code generator to compile valid source code for
boolean literal inputs as used in ternary ifelse of binary lgoical
operations. Furthermore, this also includes the related tests.

Project: http://git-wip-us.apache.org/repos/asf/systemml/repo
Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/5983e96e
Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/5983e96e
Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/5983e96e

Branch: refs/heads/master
Commit: 5983e96ee213030b854800016aecfc5018ad45a1
Parents: e0b66b3
Author: Matthias Boehm <mb...@gmail.com>
Authored: Thu Feb 8 19:06:05 2018 -0800
Committer: Matthias Boehm <mb...@gmail.com>
Committed: Thu Feb 8 22:04:10 2018 -0800

----------------------------------------------------------------------
 .../sysml/hops/codegen/cplan/CNodeData.java     |  4 ++-
 .../hops/codegen/template/TemplateUtils.java    |  4 ++-
 .../functions/codegen/CellwiseTmplTest.java     | 30 +++++++++++++++-----
 .../scripts/functions/codegen/cellwisetmpl19.R  | 30 ++++++++++++++++++++
 .../functions/codegen/cellwisetmpl19.dml        | 28 ++++++++++++++++++
 5 files changed, 87 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/systemml/blob/5983e96e/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeData.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeData.java b/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeData.java
index 653b50b..11c5868 100644
--- a/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeData.java
+++ b/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeData.java
@@ -51,7 +51,7 @@ public class CNodeData extends CNode
 		_cols = node.getNumCols();
 		_dataType = node.getDataType();
 	}
-		
+	
 	@Override
 	public String getVarname() {
 		if( "NaN".equals(_name) )
@@ -60,6 +60,8 @@ public class CNodeData extends CNode
 			return "Double.POSITIVE_INFINITY";
 		else if( "-Infinity".equals(_name) )
 			return "Double.NEGATIVE_INFINITY";
+		else if( "true".equals(_name) || "false".equals(_name) )
+			return "true".equals(_name) ? "1d" : "0d";
 		else
 			return _name;
 	}

http://git-wip-us.apache.org/repos/asf/systemml/blob/5983e96e/src/main/java/org/apache/sysml/hops/codegen/template/TemplateUtils.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/hops/codegen/template/TemplateUtils.java b/src/main/java/org/apache/sysml/hops/codegen/template/TemplateUtils.java
index 8ac2d54..1f8151e 100644
--- a/src/main/java/org/apache/sysml/hops/codegen/template/TemplateUtils.java
+++ b/src/main/java/org/apache/sysml/hops/codegen/template/TemplateUtils.java
@@ -317,7 +317,9 @@ public class TemplateUtils
 				&& !TemplateUtils.isUnary(output, 
 					UnaryType.EXP, UnaryType.LOG, UnaryType.ROW_COUNTNNZS)) 
 			|| (output instanceof CNodeBinary
-				&& !TemplateUtils.isBinary(output, BinType.VECT_OUTERMULT_ADD))) 
+				&& !TemplateUtils.isBinary(output, BinType.VECT_OUTERMULT_ADD))
+			|| output instanceof CNodeTernary 
+				&& ((CNodeTernary)output).getType() == TernaryType.IFELSE)
 			&& hasOnlyDataNodeOrLookupInputs(output);
 	}
 	

http://git-wip-us.apache.org/repos/asf/systemml/blob/5983e96e/src/test/java/org/apache/sysml/test/integration/functions/codegen/CellwiseTmplTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/sysml/test/integration/functions/codegen/CellwiseTmplTest.java b/src/test/java/org/apache/sysml/test/integration/functions/codegen/CellwiseTmplTest.java
index 2f44f61..b32f06d 100644
--- a/src/test/java/org/apache/sysml/test/integration/functions/codegen/CellwiseTmplTest.java
+++ b/src/test/java/org/apache/sysml/test/integration/functions/codegen/CellwiseTmplTest.java
@@ -34,7 +34,7 @@ import org.apache.sysml.test.integration.TestConfiguration;
 import org.apache.sysml.test.utils.TestUtils;
 
 public class CellwiseTmplTest extends AutomatedTestBase 
-{	
+{
 	private static final String TEST_NAME = "cellwisetmpl";
 	private static final String TEST_NAME1 = TEST_NAME+1;
 	private static final String TEST_NAME2 = TEST_NAME+2;
@@ -54,7 +54,7 @@ public class CellwiseTmplTest extends AutomatedTestBase
 	private static final String TEST_NAME16 = TEST_NAME+16; //colSums(2*log(X));
 	private static final String TEST_NAME17 = TEST_NAME+17; //xor operation
 	private static final String TEST_NAME18 = TEST_NAME+18; //sum(ifelse(X,Y,Z))
-	
+	private static final String TEST_NAME19 = TEST_NAME+19; //sum(ifelse(true,Y,Z))+sum(ifelse(false,Y,Z))
 	
 	private static final String TEST_DIR = "functions/codegen/";
 	private static final String TEST_CLASS_DIR = TEST_DIR + CellwiseTmplTest.class.getSimpleName() + "/";
@@ -67,7 +67,7 @@ public class CellwiseTmplTest extends AutomatedTestBase
 	@Override
 	public void setUp() {
 		TestUtils.clearAssertionInformation();
-		for( int i=1; i<=18; i++ ) {
+		for( int i=1; i<=19; i++ ) {
 			addTestConfiguration( TEST_NAME+i, new TestConfiguration(
 					TEST_CLASS_DIR, TEST_NAME+i, new String[] {String.valueOf(i)}) );
 		}
@@ -320,9 +320,24 @@ public class CellwiseTmplTest extends AutomatedTestBase
 		testCodegenIntegration( TEST_NAME18, true, ExecType.SPARK );
 	}
 	
+	@Test
+	public void testCodegenCellwiseRewrite19() {
+		testCodegenIntegration( TEST_NAME19, true, ExecType.CP );
+	}
+
+	@Test
+	public void testCodegenCellwise19() {
+		testCodegenIntegration( TEST_NAME19, false, ExecType.CP );
+	}
+
+	@Test
+	public void testCodegenCellwiseRewrite19_sp() {
+		testCodegenIntegration( TEST_NAME19, true, ExecType.SPARK );
+	}
+	
 	
 	private void testCodegenIntegration( String testname, boolean rewrites, ExecType instType )
-	{			
+	{
 		boolean oldRewrites = OptimizerUtils.ALLOW_ALGEBRAIC_SIMPLIFICATION;
 		String oldTestConf = TEST_CONF;
 		RUNTIME_PLATFORM platformOld = rtplatform;
@@ -350,7 +365,7 @@ public class CellwiseTmplTest extends AutomatedTestBase
 			programArgs = new String[]{"-explain", "hops", "-stats", "-args", output("S") };
 			
 			fullRScriptName = HOME + testname + ".R";
-			rCmd = getRCmd(inputDir(), expectedDir());			
+			rCmd = getRCmd(inputDir(), expectedDir());
 
 			OptimizerUtils.ALLOW_ALGEBRAIC_SIMPLIFICATION = rewrites;
 
@@ -367,11 +382,12 @@ public class CellwiseTmplTest extends AutomatedTestBase
 			else {
 				//compare matrices 
 				HashMap<CellIndex, Double> dmlfile = readDMLMatrixFromHDFS("S");
-				HashMap<CellIndex, Double> rfile  = readRMatrixFromFS("S");	
+				HashMap<CellIndex, Double> rfile  = readRMatrixFromFS("S");
 				TestUtils.compareMatrices(dmlfile, rfile, eps, "Stat-DML", "Stat-R");
 			}
 			
-			if( !(rewrites && testname.equals(TEST_NAME2)) ) //sigmoid
+			if( !(rewrites && (testname.equals(TEST_NAME2)
+				|| testname.equals(TEST_NAME19))) ) //sigmoid
 				Assert.assertTrue(heavyHittersContainsSubString(
 						"spoofCell", "sp_spoofCell", "spoofMA", "sp_spoofMA"));
 			if( testname.equals(TEST_NAME7) ) //ensure matrix mult is fused

http://git-wip-us.apache.org/repos/asf/systemml/blob/5983e96e/src/test/scripts/functions/codegen/cellwisetmpl19.R
----------------------------------------------------------------------
diff --git a/src/test/scripts/functions/codegen/cellwisetmpl19.R b/src/test/scripts/functions/codegen/cellwisetmpl19.R
new file mode 100644
index 0000000..fb09020
--- /dev/null
+++ b/src/test/scripts/functions/codegen/cellwisetmpl19.R
@@ -0,0 +1,30 @@
+#-------------------------------------------------------------
+#
+# 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.
+#
+#-------------------------------------------------------------
+
+args<-commandArgs(TRUE)
+options(digits=22)
+library("Matrix")
+
+Y = as.numeric(matrix(seq(0, 199999), 1000, 200, byrow=TRUE));
+Z = as.numeric(matrix(seq(1000, 200999), 1000, 200, byrow=TRUE));
+R = as.matrix(sum(Y) + sum(Z));
+
+writeMM(as(R,"CsparseMatrix"), paste(args[2], "S", sep=""));

http://git-wip-us.apache.org/repos/asf/systemml/blob/5983e96e/src/test/scripts/functions/codegen/cellwisetmpl19.dml
----------------------------------------------------------------------
diff --git a/src/test/scripts/functions/codegen/cellwisetmpl19.dml b/src/test/scripts/functions/codegen/cellwisetmpl19.dml
new file mode 100644
index 0000000..c7733f8
--- /dev/null
+++ b/src/test/scripts/functions/codegen/cellwisetmpl19.dml
@@ -0,0 +1,28 @@
+#-------------------------------------------------------------
+#
+# 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.
+#
+#-------------------------------------------------------------
+
+Y = matrix(seq(0, 199999), 1000, 200);
+Z = matrix(seq(1000, 200999), 1000, 200);
+
+while(FALSE){}
+
+R = as.matrix(sum(ifelse(TRUE,Y,Z)) + sum(ifelse(FALSE,Y,Z)));
+write(R, $1)