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 2017/11/08 21:21:05 UTC

[1/2] systemml git commit: [SYSTEMML-1990] New rewrites for rand, outer products, and cbind/rbind

Repository: systemml
Updated Branches:
  refs/heads/master 8a1f98e1b -> ec024661a


[SYSTEMML-1990] New rewrites for rand, outer products, and cbind/rbind

This patch generalizes two existing rewrites and introduces a new
rewrite for nary cbind/rbind:

(1) Generalized rand-binary fusion: So far we only fused binary
operations with literals into rand operations. For special cases of
multiply and add, we now also fuse binary operations with variable
scalar inputs.

(2) Generalized outer-product rewrites: Outer products for replication
with subsequent comparison operations are rewritten to binary outer
operations. This patch generalizes the detection of such patterns to
partial unknowns.

(3) New rbind/cbind folding: Exploiting the recently added nary
cbind/rbind operations, we now recursively fold subsequent cbind/rbind
operations with single consumers into nary cbind/rbind.


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

Branch: refs/heads/master
Commit: 578a98697e6219947b748337f9a121acf54afe53
Parents: 8a1f98e
Author: Matthias Boehm <mb...@gmail.com>
Authored: Mon Nov 6 13:14:14 2017 -0800
Committer: Matthias Boehm <mb...@gmail.com>
Committed: Wed Nov 8 13:22:13 2017 -0800

----------------------------------------------------------------------
 .../java/org/apache/sysml/hops/DataGenOp.java   |  15 +-
 .../sysml/hops/rewrite/HopRewriteUtils.java     |  21 +++
 .../RewriteAlgebraicSimplificationDynamic.java  |   5 +-
 .../RewriteAlgebraicSimplificationStatic.java   | 140 +++++++++++++++----
 .../java/org/apache/sysml/utils/Statistics.java |   3 +-
 .../test/integration/AutomatedTestBase.java     |  15 ++
 .../functions/misc/RewriteFoldRCBindTest.java   | 101 +++++++++++++
 .../functions/misc/RewriteFusedRandTest.java    |  62 ++++----
 .../scripts/functions/misc/RewriteFoldCBind.dml |  28 ++++
 .../scripts/functions/misc/RewriteFoldRBind.dml |  28 ++++
 .../scripts/functions/misc/RewriteFusedRand.dml |  29 ----
 .../functions/misc/RewriteFusedRandLit.dml      |  29 ++++
 .../functions/misc/RewriteFusedRandVar1.dml     |  28 ++++
 .../functions/misc/RewriteFusedRandVar2.dml     |  28 ++++
 .../functions/misc/ZPackageSuite.java           |   1 +
 15 files changed, 446 insertions(+), 87 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/systemml/blob/578a9869/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 69f35ed..500bbd9 100644
--- a/src/main/java/org/apache/sysml/hops/DataGenOp.java
+++ b/src/main/java/org/apache/sysml/hops/DataGenOp.java
@@ -45,7 +45,6 @@ import org.apache.sysml.runtime.util.UtilFunctions;
  */
 public class DataGenOp extends Hop implements MultiThreadedHop
 {
-	
 	public static final long UNSPECIFIED_SEED = -1;
 	
 	 // defines the specific data generation method
@@ -366,15 +365,21 @@ public class DataGenOp extends Hop implements MultiThreadedHop
 	}
 	
 
-	public HashMap<String, Integer> getParamIndexMap()
-	{
+	public HashMap<String, Integer> getParamIndexMap() {
 		return _paramIndexMap;
 	}
 	
-	public int getParamIndex(String key)
-	{
+	public int getParamIndex(String key) {
 		return _paramIndexMap.get(key);
 	}
+	
+	public Hop getInput(String key) {
+		return getInput().get(getParamIndex(key));
+	}
+	
+	public void setInput(String key, Hop hop) {
+		getInput().set(getParamIndex(key), hop);
+	}
 
 	public boolean hasConstantValue() 
 	{

http://git-wip-us.apache.org/repos/asf/systemml/blob/578a9869/src/main/java/org/apache/sysml/hops/rewrite/HopRewriteUtils.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/hops/rewrite/HopRewriteUtils.java b/src/main/java/org/apache/sysml/hops/rewrite/HopRewriteUtils.java
index b6db466..15cc2cb 100644
--- a/src/main/java/org/apache/sysml/hops/rewrite/HopRewriteUtils.java
+++ b/src/main/java/org/apache/sysml/hops/rewrite/HopRewriteUtils.java
@@ -40,6 +40,7 @@ import org.apache.sysml.hops.Hop.Direction;
 import org.apache.sysml.hops.Hop.FileFormatTypes;
 import org.apache.sysml.hops.Hop.OpOp2;
 import org.apache.sysml.hops.Hop.OpOp3;
+import org.apache.sysml.hops.Hop.OpOpN;
 import org.apache.sysml.hops.Hop.ParamBuiltinOp;
 import org.apache.sysml.hops.Hop.ReOrgOp;
 import org.apache.sysml.hops.HopsException;
@@ -47,6 +48,7 @@ import org.apache.sysml.hops.IndexingOp;
 import org.apache.sysml.hops.LeftIndexingOp;
 import org.apache.sysml.hops.LiteralOp;
 import org.apache.sysml.hops.MemoTable;
+import org.apache.sysml.hops.NaryOp;
 import org.apache.sysml.hops.OptimizerUtils;
 import org.apache.sysml.hops.ParameterizedBuiltinOp;
 import org.apache.sysml.hops.ReorgOp;
@@ -603,6 +605,16 @@ public class HopRewriteUtils
 		return ix;
 	}
 	
+	public static NaryOp createNary(OpOpN op, Hop... inputs) throws HopsException {
+		Hop mainInput = inputs[0];
+		NaryOp nop = new NaryOp(mainInput.getName(), mainInput.getDataType(),
+			mainInput.getValueType(), op, inputs);
+		nop.setOutputBlocksizes(mainInput.getRowsInBlock(), mainInput.getColsInBlock());
+		copyLineNumbers(mainInput, nop);
+		nop.refreshSizeInformation();
+		return nop;
+	}
+	
 	public static Hop createValueHop( Hop hop, boolean row ) 
 		throws HopsException
 	{
@@ -957,6 +969,15 @@ public class HopRewriteUtils
 		return (hop instanceof AggUnaryOp && ((AggUnaryOp)hop).getOp()==AggOp.SUM_SQ);
 	}
 	
+	public static boolean isNary(Hop hop, OpOpN type) {
+		return hop instanceof NaryOp && ((NaryOp)hop).getOp()==type;
+	}
+	
+	public static boolean isNary(Hop hop, OpOpN... types) {
+		return ( hop instanceof NaryOp 
+			&& ArrayUtils.contains(types, ((NaryOp) hop).getOp()));
+	}
+	
 	public static boolean isNonZeroIndicator(Hop pred, Hop hop )
 	{
 		if( pred instanceof BinaryOp && ((BinaryOp)pred).getOp()==OpOp2.NOTEQUAL

http://git-wip-us.apache.org/repos/asf/systemml/blob/578a9869/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 eba06fc..0fa1aed 100644
--- a/src/main/java/org/apache/sysml/hops/rewrite/RewriteAlgebraicSimplificationDynamic.java
+++ b/src/main/java/org/apache/sysml/hops/rewrite/RewriteAlgebraicSimplificationDynamic.java
@@ -432,8 +432,9 @@ public class RewriteAlgebraicSimplificationDynamic extends HopRewriteRule
 			else if(HopRewriteUtils.isValidOuterBinaryOp(bop) 
 				&& HopRewriteUtils.isMatrixMultiply(left)
 				&& HopRewriteUtils.isDataGenOpWithConstantValue(left.getInput().get(1), 1)
-				&& left.getInput().get(0).getDim2() == 1 //column vector 
-				&& left.getDim1() != 1 && right.getDim1() == 1 ) //outer vector product 
+				&& (left.getInput().get(0).getDim2() == 1 //outer product
+					|| left.getInput().get(1).getDim1() == 1)
+				&& left.getDim1() != 1 && right.getDim1() == 1 ) //outer vector binary 
 			{
 				Hop hnew = HopRewriteUtils.createBinary(left.getInput().get(0), right, bop, true);
 				HopRewriteUtils.replaceChildReference(parent, hi, hnew, pos);

http://git-wip-us.apache.org/repos/asf/systemml/blob/578a9869/src/main/java/org/apache/sysml/hops/rewrite/RewriteAlgebraicSimplificationStatic.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/hops/rewrite/RewriteAlgebraicSimplificationStatic.java b/src/main/java/org/apache/sysml/hops/rewrite/RewriteAlgebraicSimplificationStatic.java
index 64a37d7..2d5d881 100644
--- a/src/main/java/org/apache/sysml/hops/rewrite/RewriteAlgebraicSimplificationStatic.java
+++ b/src/main/java/org/apache/sysml/hops/rewrite/RewriteAlgebraicSimplificationStatic.java
@@ -38,10 +38,12 @@ import org.apache.sysml.hops.Hop.AggOp;
 import org.apache.sysml.hops.Hop.DataGenMethod;
 import org.apache.sysml.hops.Hop.Direction;
 import org.apache.sysml.hops.Hop.OpOp3;
+import org.apache.sysml.hops.Hop.OpOpN;
 import org.apache.sysml.hops.Hop.ParamBuiltinOp;
 import org.apache.sysml.hops.Hop.ReOrgOp;
 import org.apache.sysml.hops.HopsException;
 import org.apache.sysml.hops.LiteralOp;
+import org.apache.sysml.hops.NaryOp;
 import org.apache.sysml.hops.OptimizerUtils;
 import org.apache.sysml.hops.Hop.OpOp2;
 import org.apache.sysml.hops.ParameterizedBuiltinOp;
@@ -147,23 +149,24 @@ public class RewriteAlgebraicSimplificationStatic extends HopRewriteRule
 			hi = removeUnnecessaryBinaryOperation(hop, hi, i);   //e.g., X*1 -> X (dep: should come after rm unnecessary vectorize)
 			hi = fuseDatagenAndBinaryOperation(hi);              //e.g., rand(min=-1,max=1)*7 -> rand(min=-7,max=7)
 			hi = fuseDatagenAndMinusOperation(hi);               //e.g., -(rand(min=-2,max=1)) -> rand(min=-1,max=2)
- 			hi = simplifyBinaryToUnaryOperation(hop, hi, i);     //e.g., X*X -> X^2 (pow2), X+X -> X*2, (X>0)-(X<0) -> sign(X)
- 			hi = canonicalizeMatrixMultScalarAdd(hi);            //e.g., eps+U%*%t(V) -> U%*%t(V)+eps, U%*%t(V)-eps -> U%*%t(V)+(-eps) 
- 			hi = simplifyReverseOperation(hop, hi, i);           //e.g., table(seq(1,nrow(X),1),seq(nrow(X),1,-1)) %*% X -> rev(X)
- 			if(OptimizerUtils.ALLOW_OPERATOR_FUSION)
- 				hi = simplifyMultiBinaryToBinaryOperation(hi);       //e.g., 1-X*Y -> X 1-* Y
- 			hi = simplifyDistributiveBinaryOperation(hop, hi, i);//e.g., (X-Y*X) -> (1-Y)*X
- 			hi = simplifyBushyBinaryOperation(hop, hi, i);       //e.g., (X*(Y*(Z%*%v))) -> (X*Y)*(Z%*%v)
- 			hi = simplifyUnaryAggReorgOperation(hop, hi, i);     //e.g., sum(t(X)) -> sum(X)
- 			hi = removeUnnecessaryAggregates(hi);                //e.g., sum(rowSums(X)) -> sum(X)
- 			hi = simplifyBinaryMatrixScalarOperation(hop, hi, i);//e.g., as.scalar(X*s) -> as.scalar(X)*s;
- 			hi = pushdownUnaryAggTransposeOperation(hop, hi, i); //e.g., colSums(t(X)) -> t(rowSums(X))
- 			hi = pushdownCSETransposeScalarOperation(hop, hi, i);//e.g., a=t(X), b=t(X^2) -> a=t(X), b=t(X)^2 for CSE t(X)
- 			hi = pushdownSumBinaryMult(hop, hi, i);              //e.g., sum(lamda*X) -> lamda*sum(X)
- 			hi = simplifyUnaryPPredOperation(hop, hi, i);        //e.g., abs(ppred()) -> ppred(), others: round, ceil, floor
- 			hi = simplifyTransposedAppend(hop, hi, i);           //e.g., t(cbind(t(A),t(B))) -> rbind(A,B);
- 			if(OptimizerUtils.ALLOW_OPERATOR_FUSION)
- 				hi = fuseBinarySubDAGToUnaryOperation(hop, hi, i);   //e.g., X*(1-X)-> sprop(X) || 1/(1+exp(-X)) -> sigmoid(X) || X*(X>0) -> selp(X)
+			hi = foldMultipleAppendOperations(hi);               //e.g., cbind(X,cbind(Y,Z)) -> cbind(X,Y,Z)
+			hi = simplifyBinaryToUnaryOperation(hop, hi, i);     //e.g., X*X -> X^2 (pow2), X+X -> X*2, (X>0)-(X<0) -> sign(X)
+			hi = canonicalizeMatrixMultScalarAdd(hi);            //e.g., eps+U%*%t(V) -> U%*%t(V)+eps, U%*%t(V)-eps -> U%*%t(V)+(-eps) 
+			hi = simplifyReverseOperation(hop, hi, i);           //e.g., table(seq(1,nrow(X),1),seq(nrow(X),1,-1)) %*% X -> rev(X)
+			if(OptimizerUtils.ALLOW_OPERATOR_FUSION)
+				hi = simplifyMultiBinaryToBinaryOperation(hi);       //e.g., 1-X*Y -> X 1-* Y
+			hi = simplifyDistributiveBinaryOperation(hop, hi, i);//e.g., (X-Y*X) -> (1-Y)*X
+			hi = simplifyBushyBinaryOperation(hop, hi, i);       //e.g., (X*(Y*(Z%*%v))) -> (X*Y)*(Z%*%v)
+			hi = simplifyUnaryAggReorgOperation(hop, hi, i);     //e.g., sum(t(X)) -> sum(X)
+			hi = removeUnnecessaryAggregates(hi);                //e.g., sum(rowSums(X)) -> sum(X)
+			hi = simplifyBinaryMatrixScalarOperation(hop, hi, i);//e.g., as.scalar(X*s) -> as.scalar(X)*s;
+			hi = pushdownUnaryAggTransposeOperation(hop, hi, i); //e.g., colSums(t(X)) -> t(rowSums(X))
+			hi = pushdownCSETransposeScalarOperation(hop, hi, i);//e.g., a=t(X), b=t(X^2) -> a=t(X), b=t(X)^2 for CSE t(X)
+			hi = pushdownSumBinaryMult(hop, hi, i);              //e.g., sum(lamda*X) -> lamda*sum(X)
+			hi = simplifyUnaryPPredOperation(hop, hi, i);        //e.g., abs(ppred()) -> ppred(), others: round, ceil, floor
+			hi = simplifyTransposedAppend(hop, hi, i);           //e.g., t(cbind(t(A),t(B))) -> rbind(A,B);
+			if(OptimizerUtils.ALLOW_OPERATOR_FUSION)
+				hi = fuseBinarySubDAGToUnaryOperation(hop, hi, i);   //e.g., X*(1-X)-> sprop(X) || 1/(1+exp(-X)) -> sigmoid(X) || X*(X>0) -> selp(X)
 			hi = simplifyTraceMatrixMult(hop, hi, i);            //e.g., trace(X%*%Y)->sum(X*t(Y));  
 			hi = simplifySlicedMatrixMult(hop, hi, i);           //e.g., (X%*%Y)[1,1] -> X[1,] %*% Y[,1];
 			hi = simplifyConstantSort(hop, hi, i);               //e.g., order(matrix())->matrix/seq; 
@@ -358,14 +361,13 @@ public class RewriteAlgebraicSimplificationStatic extends HopRewriteRule
 			//the creation of multiple datagen ops and thus potentially different results if seed not specified)
 			
 			//left input rand and hence output matrix double, right scalar literal
-			if( left instanceof DataGenOp && ((DataGenOp)left).getOp()==DataGenMethod.RAND &&
+			if( HopRewriteUtils.isDataGenOp(left, DataGenMethod.RAND) &&
 				right instanceof LiteralOp && left.getParent().size()==1 )
 			{
 				DataGenOp inputGen = (DataGenOp)left;
-				HashMap<String,Integer> params = inputGen.getParamIndexMap();
-				Hop pdf = left.getInput().get(params.get(DataExpression.RAND_PDF));
-				Hop min = left.getInput().get(params.get(DataExpression.RAND_MIN));
-				Hop max = left.getInput().get(params.get(DataExpression.RAND_MAX));
+				Hop pdf = inputGen.getInput(DataExpression.RAND_PDF);
+				Hop min = inputGen.getInput(DataExpression.RAND_MIN);
+				Hop max = inputGen.getInput(DataExpression.RAND_MAX);
 				double sval = ((LiteralOp)right).getDoubleValue();
 				
 				if( HopRewriteUtils.isBinary(bop, OpOp2.MULT, OpOp2.PLUS, OpOp2.MINUS, OpOp2.DIV)
@@ -396,10 +398,9 @@ public class RewriteAlgebraicSimplificationStatic extends HopRewriteRule
 				left instanceof LiteralOp && right.getParent().size()==1 )
 			{
 				DataGenOp inputGen = (DataGenOp)right;
-				HashMap<String,Integer> params = inputGen.getParamIndexMap();
-				Hop pdf = right.getInput().get(params.get(DataExpression.RAND_PDF));
-				Hop min = right.getInput().get(params.get(DataExpression.RAND_MIN));
-				Hop max = right.getInput().get(params.get(DataExpression.RAND_MAX));
+				Hop pdf = inputGen.getInput(DataExpression.RAND_PDF);
+				Hop min = inputGen.getInput(DataExpression.RAND_MIN);
+				Hop max = inputGen.getInput(DataExpression.RAND_MAX);
 				double sval = ((LiteralOp)left).getDoubleValue();
 				
 				if( (bop.getOp()==OpOp2.MULT || bop.getOp()==OpOp2.PLUS)
@@ -423,6 +424,44 @@ public class RewriteAlgebraicSimplificationStatic extends HopRewriteRule
 					LOG.debug("Applied fuseDatagenAndBinaryOperation2 (line "+bop.getBeginLine()+").");
 				}
 			}
+			//left input rand and hence output matrix double, right scalar variable
+			else if( HopRewriteUtils.isDataGenOp(left, DataGenMethod.RAND) 
+				&& right.getDataType().isScalar() && left.getParent().size()==1 )
+			{
+				DataGenOp gen = (DataGenOp)left;
+				Hop min = gen.getInput(DataExpression.RAND_MIN);
+				Hop max = gen.getInput(DataExpression.RAND_MAX);
+				
+				if( HopRewriteUtils.isBinary(bop, OpOp2.PLUS)
+					&& HopRewriteUtils.isLiteralOfValue(min, 0)
+					&& HopRewriteUtils.isLiteralOfValue(max, 0) )
+				{
+					gen.setInput(DataExpression.RAND_MIN, right);
+					gen.setInput(DataExpression.RAND_MAX, right);
+					//rewire all parents (avoid anomalies with replicated datagen)
+					List<Hop> parents = new ArrayList<>(bop.getParent());
+					for( Hop p : parents )
+						HopRewriteUtils.replaceChildReference(p, bop, gen);
+					hi = gen;
+					LOG.debug("Applied fuseDatagenAndBinaryOperation3a (line "+bop.getBeginLine()+").");
+				}
+				else if( HopRewriteUtils.isBinary(bop, OpOp2.MULT)
+					&& (HopRewriteUtils.isLiteralOfValue(min, 0)
+						|| HopRewriteUtils.isLiteralOfValue(min, 1))
+					&& HopRewriteUtils.isLiteralOfValue(max, 1) )
+				{
+					if( HopRewriteUtils.isLiteralOfValue(min, 1) )
+						gen.setInput(DataExpression.RAND_MIN, right);
+					gen.setInput(DataExpression.RAND_MAX, right);
+					//rewire all parents (avoid anomalies with replicated datagen)
+					List<Hop> parents = new ArrayList<>(bop.getParent());
+					for( Hop p : parents )
+						HopRewriteUtils.replaceChildReference(p, bop, gen);
+					hi = gen;
+					LOG.debug("Applied fuseDatagenAndBinaryOperation3b (line "+bop.getBeginLine()+").");
+				}
+			}
+			
 		}
 		
 		return hi;
@@ -478,6 +517,55 @@ public class RewriteAlgebraicSimplificationStatic extends HopRewriteRule
 		return hi;
 	}
 	
+	private static Hop foldMultipleAppendOperations(Hop hi) 
+		throws HopsException
+	{
+		if( hi.getDataType().isMatrix() //no string appends
+			&& (HopRewriteUtils.isBinary(hi, OpOp2.CBIND, OpOp2.RBIND) 
+			|| HopRewriteUtils.isNary(hi, OpOpN.CBIND, OpOpN.RBIND))
+			&& !OptimizerUtils.isHadoopExecutionMode() )
+		{
+			OpOp2 bop = (hi instanceof BinaryOp) ? ((BinaryOp)hi).getOp() :
+				OpOp2.valueOf(((NaryOp)hi).getOp().name());
+			OpOpN nop = (hi instanceof NaryOp) ? ((NaryOp)hi).getOp() :
+				OpOpN.valueOf(((BinaryOp)hi).getOp().name());
+			
+			boolean converged = false;
+			while( !converged ) {
+				//get first matching cbind or rbind
+				Hop first = hi.getInput().stream()
+					.filter(h -> HopRewriteUtils.isBinary(h, bop) || HopRewriteUtils.isNary(h, nop))
+					.findFirst().orElse(null);
+				
+				//replace current op with new nary cbind/rbind
+				if( first != null && first.getParent().size()==1 ) {
+					//construct new list of inputs (in original order)
+					ArrayList<Hop> linputs = new ArrayList<>();
+					for(Hop in : hi.getInput())
+						if( in == first )
+							linputs.addAll(first.getInput());
+						else
+							linputs.add(in);
+					Hop hnew = HopRewriteUtils.createNary(nop, linputs.toArray(new Hop[0]));
+					//clear dangling references
+					HopRewriteUtils.removeAllChildReferences(hi);
+					HopRewriteUtils.removeAllChildReferences(first);
+					//rewire all parents (avoid anomalies with refs to hi)
+					List<Hop> parents = new ArrayList<>(hi.getParent());
+					for( Hop p : parents )
+						HopRewriteUtils.replaceChildReference(p, hi, hnew);
+					hi = hnew;
+					LOG.debug("Applied foldMultipleAppendOperations (line "+hi.getBeginLine()+").");
+				}
+				else {
+					converged = true;
+				}
+			}
+		}
+		
+		return hi;
+	}
+	
 	/**
 	 * Handle simplification of binary operations (relies on previous common subexpression elimination).
 	 * At the same time this servers as a canonicalization for more complex rewrites. 

http://git-wip-us.apache.org/repos/asf/systemml/blob/578a9869/src/main/java/org/apache/sysml/utils/Statistics.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/utils/Statistics.java b/src/main/java/org/apache/sysml/utils/Statistics.java
index 555493c..5cc0650 100644
--- a/src/main/java/org/apache/sysml/utils/Statistics.java
+++ b/src/main/java/org/apache/sysml/utils/Statistics.java
@@ -545,7 +545,8 @@ public class Statistics
 	}
 	
 	public static long getCPHeavyHitterCount(String opcode) {
-		return _cpInstCounts.get(opcode);
+		Long tmp = _cpInstCounts.get(opcode);
+		return (tmp != null) ? tmp : 0;
 	}
 
 	/**

http://git-wip-us.apache.org/repos/asf/systemml/blob/578a9869/src/test/java/org/apache/sysml/test/integration/AutomatedTestBase.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/sysml/test/integration/AutomatedTestBase.java b/src/test/java/org/apache/sysml/test/integration/AutomatedTestBase.java
index d39e513..3bd78b0 100644
--- a/src/test/java/org/apache/sysml/test/integration/AutomatedTestBase.java
+++ b/src/test/java/org/apache/sysml/test/integration/AutomatedTestBase.java
@@ -1808,6 +1808,21 @@ public abstract class AutomatedTestBase
 		return writeInputFrame(name, data, false, schema, oi);
 	}
 
+	protected boolean heavyHittersContainsString(String... str) {
+		for( String opcode : Statistics.getCPHeavyHitterOpCodes())
+			for( String s : str )
+				if(opcode.equals(s))
+					return true;
+		return false;
+	}
+	
+	protected boolean heavyHittersContainsString(String str, int minCount) {
+		int count = 0;
+		for( String opcode : Statistics.getCPHeavyHitterOpCodes())
+			count += opcode.equals(str) ? 1 : 0;
+		return (count >= minCount);
+	}
+	
 	protected boolean heavyHittersContainsSubString(String... str) {
 		for( String opcode : Statistics.getCPHeavyHitterOpCodes())
 			for( String s : str )

http://git-wip-us.apache.org/repos/asf/systemml/blob/578a9869/src/test/java/org/apache/sysml/test/integration/functions/misc/RewriteFoldRCBindTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/sysml/test/integration/functions/misc/RewriteFoldRCBindTest.java b/src/test/java/org/apache/sysml/test/integration/functions/misc/RewriteFoldRCBindTest.java
new file mode 100644
index 0000000..83050bd
--- /dev/null
+++ b/src/test/java/org/apache/sysml/test/integration/functions/misc/RewriteFoldRCBindTest.java
@@ -0,0 +1,101 @@
+/*
+ * 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.
+ */
+
+package org.apache.sysml.test.integration.functions.misc;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.apache.sysml.hops.OptimizerUtils;
+import org.apache.sysml.runtime.matrix.data.MatrixValue.CellIndex;
+import org.apache.sysml.test.integration.AutomatedTestBase;
+import org.apache.sysml.test.integration.TestConfiguration;
+import org.apache.sysml.test.utils.TestUtils;
+import org.apache.sysml.utils.Statistics;
+
+public class RewriteFoldRCBindTest extends AutomatedTestBase 
+{
+	private static final String TEST_NAME1 = "RewriteFoldCBind";
+	private static final String TEST_NAME2 = "RewriteFoldRBind";
+	
+	private static final String TEST_DIR = "functions/misc/";
+	private static final String TEST_CLASS_DIR = TEST_DIR + RewriteFoldRCBindTest.class.getSimpleName() + "/";
+	
+	private static final int rows = 1932;
+	private static final int cols = 14;
+	
+	@Override
+	public void setUp() {
+		TestUtils.clearAssertionInformation();
+		addTestConfiguration( TEST_NAME1, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME1, new String[] { "R" }) );
+		addTestConfiguration( TEST_NAME2, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME2, new String[] { "R" }) );
+	}
+
+	@Test
+	public void testRewriteFoldCBindNoRewrite() {
+		testRewriteFoldRCBind( TEST_NAME1, false );
+	}
+	
+	@Test
+	public void testRewriteFoldCBindRewrite() {
+		testRewriteFoldRCBind( TEST_NAME1, true );
+	}
+	
+	@Test
+	public void testRewriteFoldRBindNoRewrite() {
+		testRewriteFoldRCBind( TEST_NAME2, false );
+	}
+	
+	@Test
+	public void testRewriteFoldRBindRewrite() {
+		testRewriteFoldRCBind( TEST_NAME2, true );
+	}
+
+	private void testRewriteFoldRCBind( String testname, boolean rewrites )
+	{	
+		boolean oldFlag = OptimizerUtils.ALLOW_ALGEBRAIC_SIMPLIFICATION;
+		
+		try {
+			TestConfiguration config = getTestConfiguration(testname);
+			loadTestConfiguration(config);
+			
+			String HOME = SCRIPT_DIR + TEST_DIR;
+			fullDMLScriptName = HOME + testname + ".dml";
+			programArgs = new String[]{ "-stats", "-args", String.valueOf(rows), 
+					String.valueOf(cols), output("R") };
+			OptimizerUtils.ALLOW_ALGEBRAIC_SIMPLIFICATION = rewrites;
+
+			//run performance tests
+			runTest(true, false, null, -1); 
+			
+			//compare matrices 
+			Double ret = readDMLMatrixFromHDFS("R").get(new CellIndex(1,1));
+			Assert.assertEquals("Wrong result", new Double(5*rows*cols), ret);
+			
+			//check for applied rewrites
+			if( rewrites ) {
+				Assert.assertTrue(!heavyHittersContainsString("append")
+					&& Statistics.getCPHeavyHitterCount("cbind") <= 1
+					&& Statistics.getCPHeavyHitterCount("rbind") <= 1);
+			}
+		}
+		finally {
+			OptimizerUtils.ALLOW_ALGEBRAIC_SIMPLIFICATION = oldFlag;
+		}
+	}
+}

http://git-wip-us.apache.org/repos/asf/systemml/blob/578a9869/src/test/java/org/apache/sysml/test/integration/functions/misc/RewriteFusedRandTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/sysml/test/integration/functions/misc/RewriteFusedRandTest.java b/src/test/java/org/apache/sysml/test/integration/functions/misc/RewriteFusedRandTest.java
index 1491aa6..d7fe902 100644
--- a/src/test/java/org/apache/sysml/test/integration/functions/misc/RewriteFusedRandTest.java
+++ b/src/test/java/org/apache/sysml/test/integration/functions/misc/RewriteFusedRandTest.java
@@ -19,8 +19,6 @@
 
 package org.apache.sysml.test.integration.functions.misc;
 
-import java.util.HashMap;
-
 import org.junit.Assert;
 import org.junit.Test;
 import org.apache.sysml.hops.OptimizerUtils;
@@ -29,13 +27,12 @@ import org.apache.sysml.test.integration.AutomatedTestBase;
 import org.apache.sysml.test.integration.TestConfiguration;
 import org.apache.sysml.test.utils.TestUtils;
 
-/**
- * 
- * 
- */
 public class RewriteFusedRandTest extends AutomatedTestBase 
-{	
-	private static final String TEST_NAME1 = "RewriteFusedRand";
+{
+	private static final String TEST_NAME1 = "RewriteFusedRandLit";
+	private static final String TEST_NAME2 = "RewriteFusedRandVar1";
+	private static final String TEST_NAME3 = "RewriteFusedRandVar2";
+	
 	private static final String TEST_DIR = "functions/misc/";
 	private static final String TEST_CLASS_DIR = TEST_DIR + RewriteFusedRandTest.class.getSimpleName() + "/";
 	
@@ -47,44 +44,50 @@ public class RewriteFusedRandTest extends AutomatedTestBase
 	public void setUp() {
 		TestUtils.clearAssertionInformation();
 		addTestConfiguration( TEST_NAME1, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME1, new String[] { "R" }) );
+		addTestConfiguration( TEST_NAME2, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME2, new String[] { "R" }) );
+		addTestConfiguration( TEST_NAME3, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME3, new String[] { "R" }) );
 	}
 
 	@Test
-	public void testRewriteFusedRandUniformNoRewrite()  {
+	public void testRewriteFusedRandUniformNoRewrite() {
 		testRewriteFusedRand( TEST_NAME1, "uniform", false );
 	}
 	
 	@Test
-	public void testRewriteFusedRandNormalNoRewrite()  {
+	public void testRewriteFusedRandNormalNoRewrite() {
 		testRewriteFusedRand( TEST_NAME1, "normal", false );
 	}
 	
 	@Test
-	public void testRewriteFusedRandPoissonNoRewrite()  {
+	public void testRewriteFusedRandPoissonNoRewrite() {
 		testRewriteFusedRand( TEST_NAME1, "poisson", false );
 	}
 	
 	@Test
-	public void testRewriteFusedRandUniform()  {
+	public void testRewriteFusedRandUniform() {
 		testRewriteFusedRand( TEST_NAME1, "uniform", true );
 	}
 	
 	@Test
-	public void testRewriteFusedRandNormal()  {
+	public void testRewriteFusedRandNormal() {
 		testRewriteFusedRand( TEST_NAME1, "normal", true );
 	}
 	
 	@Test
-	public void testRewriteFusedRandPoisson()  {
+	public void testRewriteFusedRandPoisson() {
 		testRewriteFusedRand( TEST_NAME1, "poisson", true );
 	}
 	
-	/**
-	 * 
-	 * @param condition
-	 * @param branchRemoval
-	 * @param IPA
-	 */
+	@Test
+	public void testRewriteFusedZerosPlusVar() {
+		testRewriteFusedRand( TEST_NAME2, "uniform", true );
+	}
+	
+	@Test
+	public void testRewriteFusedOnesMultVar() {
+		testRewriteFusedRand( TEST_NAME3, "uniform", true );
+	}
+	
 	private void testRewriteFusedRand( String testname, String pdf, boolean rewrites )
 	{	
 		boolean oldFlag = OptimizerUtils.ALLOW_ALGEBRAIC_SIMPLIFICATION;
@@ -95,7 +98,7 @@ public class RewriteFusedRandTest extends AutomatedTestBase
 			
 			String HOME = SCRIPT_DIR + TEST_DIR;
 			fullDMLScriptName = HOME + testname + ".dml";
-			programArgs = new String[]{ "-args", String.valueOf(rows), 
+			programArgs = new String[]{ "-stats", "-args", String.valueOf(rows), 
 					String.valueOf(cols), pdf, String.valueOf(seed), output("R") };
 			OptimizerUtils.ALLOW_ALGEBRAIC_SIMPLIFICATION = rewrites;
 
@@ -103,11 +106,22 @@ public class RewriteFusedRandTest extends AutomatedTestBase
 			runTest(true, false, null, -1); 
 			
 			//compare matrices 
-			HashMap<CellIndex, Double> dmlfile = readDMLMatrixFromHDFS("R");
-			Assert.assertEquals("Wrong result, expected: "+rows, new Double(rows), dmlfile.get(new CellIndex(1,1)));
+			Double ret = readDMLMatrixFromHDFS("R").get(new CellIndex(1,1));
+			if( testname.equals(TEST_NAME1) )
+				Assert.assertEquals("Wrong result", new Double(rows), ret);
+			else if( testname.equals(TEST_NAME2) )
+				Assert.assertEquals("Wrong result", new Double(Math.pow(rows*cols, 2)), ret);
+			else if( testname.equals(TEST_NAME3) )
+				Assert.assertEquals("Wrong result", new Double(Math.pow(rows*cols, 2)), ret);
+			
+			//check for applied rewrites
+			if( rewrites && pdf.equals("uniform") ) {
+				Assert.assertTrue(!heavyHittersContainsString("+")
+					&& !heavyHittersContainsString("*"));
+			}
 		}
 		finally {
 			OptimizerUtils.ALLOW_ALGEBRAIC_SIMPLIFICATION = oldFlag;
 		}
 	}	
-}
\ No newline at end of file
+}

http://git-wip-us.apache.org/repos/asf/systemml/blob/578a9869/src/test/scripts/functions/misc/RewriteFoldCBind.dml
----------------------------------------------------------------------
diff --git a/src/test/scripts/functions/misc/RewriteFoldCBind.dml b/src/test/scripts/functions/misc/RewriteFoldCBind.dml
new file mode 100644
index 0000000..47733fe
--- /dev/null
+++ b/src/test/scripts/functions/misc/RewriteFoldCBind.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.
+#
+#-------------------------------------------------------------
+
+X = matrix(1, $1, $2)
+while(FALSE){}
+Y = cbind(cbind(X,X),cbind(X,X,X))
+while(FALSE){}
+R = as.matrix(sum(Y))
+
+write(R, $3);

http://git-wip-us.apache.org/repos/asf/systemml/blob/578a9869/src/test/scripts/functions/misc/RewriteFoldRBind.dml
----------------------------------------------------------------------
diff --git a/src/test/scripts/functions/misc/RewriteFoldRBind.dml b/src/test/scripts/functions/misc/RewriteFoldRBind.dml
new file mode 100644
index 0000000..c489f7c
--- /dev/null
+++ b/src/test/scripts/functions/misc/RewriteFoldRBind.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.
+#
+#-------------------------------------------------------------
+
+X = matrix(1, $1, $2)
+while(FALSE){}
+Y = rbind(rbind(X,X),rbind(X,X,X))
+while(FALSE){}
+R = as.matrix(sum(Y))
+
+write(R, $3);

http://git-wip-us.apache.org/repos/asf/systemml/blob/578a9869/src/test/scripts/functions/misc/RewriteFusedRand.dml
----------------------------------------------------------------------
diff --git a/src/test/scripts/functions/misc/RewriteFusedRand.dml b/src/test/scripts/functions/misc/RewriteFusedRand.dml
deleted file mode 100644
index ab00f04..0000000
--- a/src/test/scripts/functions/misc/RewriteFusedRand.dml
+++ /dev/null
@@ -1,29 +0,0 @@
-#-------------------------------------------------------------
-#
-# 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.
-#
-#-------------------------------------------------------------
-
-X1 = rand(rows=$1, cols=$2, pdf=$3, seed=$4) * 7;
-
-while(FALSE){} #prevent cse
-
-X2 = rand(rows=$1, cols=$2, pdf=$3, seed=$4) * 7;
-
-R = as.matrix(sum(rowSums(X1)==rowSums(X2)));
-write(R, $5);
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/systemml/blob/578a9869/src/test/scripts/functions/misc/RewriteFusedRandLit.dml
----------------------------------------------------------------------
diff --git a/src/test/scripts/functions/misc/RewriteFusedRandLit.dml b/src/test/scripts/functions/misc/RewriteFusedRandLit.dml
new file mode 100644
index 0000000..ab00f04
--- /dev/null
+++ b/src/test/scripts/functions/misc/RewriteFusedRandLit.dml
@@ -0,0 +1,29 @@
+#-------------------------------------------------------------
+#
+# 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.
+#
+#-------------------------------------------------------------
+
+X1 = rand(rows=$1, cols=$2, pdf=$3, seed=$4) * 7;
+
+while(FALSE){} #prevent cse
+
+X2 = rand(rows=$1, cols=$2, pdf=$3, seed=$4) * 7;
+
+R = as.matrix(sum(rowSums(X1)==rowSums(X2)));
+write(R, $5);
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/systemml/blob/578a9869/src/test/scripts/functions/misc/RewriteFusedRandVar1.dml
----------------------------------------------------------------------
diff --git a/src/test/scripts/functions/misc/RewriteFusedRandVar1.dml b/src/test/scripts/functions/misc/RewriteFusedRandVar1.dml
new file mode 100644
index 0000000..f37557f
--- /dev/null
+++ b/src/test/scripts/functions/misc/RewriteFusedRandVar1.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.
+#
+#-------------------------------------------------------------
+
+X = matrix(1, $1, $2)
+while(FALSE){}
+Y = matrix(0, $1, $2) + sum(X);
+while(FALSE){}
+R = as.matrix(sum(Y))
+
+write(R, $5);

http://git-wip-us.apache.org/repos/asf/systemml/blob/578a9869/src/test/scripts/functions/misc/RewriteFusedRandVar2.dml
----------------------------------------------------------------------
diff --git a/src/test/scripts/functions/misc/RewriteFusedRandVar2.dml b/src/test/scripts/functions/misc/RewriteFusedRandVar2.dml
new file mode 100644
index 0000000..d98ef08
--- /dev/null
+++ b/src/test/scripts/functions/misc/RewriteFusedRandVar2.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.
+#
+#-------------------------------------------------------------
+
+X = matrix(1, $1, $2)
+while(FALSE){}
+Y = matrix(1, $1, $2) * sum(X);
+while(FALSE){}
+R = as.matrix(sum(Y))
+
+write(R, $5);

http://git-wip-us.apache.org/repos/asf/systemml/blob/578a9869/src/test_suites/java/org/apache/sysml/test/integration/functions/misc/ZPackageSuite.java
----------------------------------------------------------------------
diff --git a/src/test_suites/java/org/apache/sysml/test/integration/functions/misc/ZPackageSuite.java b/src/test_suites/java/org/apache/sysml/test/integration/functions/misc/ZPackageSuite.java
index a453cbd..ae4f820 100644
--- a/src/test_suites/java/org/apache/sysml/test/integration/functions/misc/ZPackageSuite.java
+++ b/src/test_suites/java/org/apache/sysml/test/integration/functions/misc/ZPackageSuite.java
@@ -56,6 +56,7 @@ import org.junit.runners.Suite;
 	RewriteCTableToRExpandTest.class,
 	RewriteElementwiseMultChainOptimizationTest.class,
 	RewriteEliminateAggregatesTest.class,
+	RewriteFoldRCBindTest.class,
 	RewriteFuseBinaryOpChainTest.class,
 	RewriteFusedRandTest.class,
 	RewriteLoopVectorization.class,


[2/2] systemml git commit: [SYSTEMML-2007] New order builtin w/ multiple order-by columns

Posted by mb...@apache.org.
[SYSTEMML-2007] New order builtin w/ multiple order-by columns

This patch generalizes the compiler and runtime of the existing order
builtin function to support multiple order-by columns. So far we only
support a single scalar column parameter - now users can either provide
a scalar or matrix argument. This avoids emulating this functionality
via multiple order calls in reverse order of order-by columns, which
works due to stable sort. 

This patch modifies the compiler and CP runtime, without degrading
performance for the case of a single order-by column. The spark runtime
will follow in a subsequent patch, as it requires an second sort runtime
to avoid unnecessary overhead in case of a single order-by column.

On a scenario of a 1M x 100 dense matrix, with varying number of
distinct values, and 3 order-by columns, this patch improved performance
(compared to the emulation via 3 order calls) as follows:

1M x 10, distinct=1000: 2,271ms -> 940ms
1M x 10, distinct=100: 2,125ms -> 966ms
1M x 10, distinct=10: 2.057ms -> 958ms


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

Branch: refs/heads/master
Commit: ec024661a42cdc243addb6feacac026d60313f67
Parents: 578a986
Author: Matthias Boehm <mb...@gmail.com>
Authored: Tue Nov 7 20:02:09 2017 -0800
Committer: Matthias Boehm <mb...@gmail.com>
Committed: Wed Nov 8 13:22:15 2017 -0800

----------------------------------------------------------------------
 .../java/org/apache/sysml/hops/ReorgOp.java     |  22 +--
 .../ParameterizedBuiltinFunctionExpression.java |  11 +-
 .../runtime/functionobjects/SortIndex.java      |  28 ++-
 .../instructions/cp/ReorgCPInstruction.java     |  13 +-
 .../instructions/spark/ReorgSPInstruction.java  |  26 ++-
 .../instructions/spark/utils/RDDSortUtils.java  |   2 +-
 .../runtime/matrix/data/LibMatrixReorg.java     | 122 ++++++++-----
 .../sysml/runtime/matrix/data/MatrixBlock.java  |   4 +-
 .../sysml/runtime/util/DataConverter.java       |  55 +++---
 .../reorg/MultipleOrderByColsTest.java          | 179 +++++++++++++++++++
 src/test/scripts/functions/reorg/OrderMultiBy.R |  42 +++++
 .../scripts/functions/reorg/OrderMultiBy.dml    |  33 ++++
 12 files changed, 428 insertions(+), 109 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/systemml/blob/ec024661/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 ef83253..4b55c9b 100644
--- a/src/main/java/org/apache/sysml/hops/ReorgOp.java
+++ b/src/main/java/org/apache/sysml/hops/ReorgOp.java
@@ -51,11 +51,8 @@ import org.apache.sysml.runtime.matrix.MatrixCharacteristics;
 
 public class ReorgOp extends Hop implements MultiThreadedHop
 {
-	
 	public static boolean FORCE_DIST_SORT_INDEXES = false;
 	
-	public boolean bSortSPRewriteApplicable = false;
-	
 	private ReOrgOp op;
 	private int _maxNumThreads = -1; //-1 for unlimited
 	
@@ -371,18 +368,21 @@ public class ReorgOp extends Hop implements MultiThreadedHop
 						setLops( mmult.constructLops() );
 						
 						//cleanups
-						HopRewriteUtils.removeChildReference(table, input);		
+						HopRewriteUtils.removeChildReference(table, input);
 					}
 				}
-				else //CP or Spark
+				else if( et==ExecType.SPARK ) {
+					boolean sortRewrite = !FORCE_DIST_SORT_INDEXES && isSortSPRewriteApplicable();
+					Lop transform1 = constructCPOrSparkSortLop(input, by, desc, ixret, et, sortRewrite);
+					setOutputDimensions(transform1);
+					setLineNumbers(transform1);
+					setLops(transform1);
+				}
+				else //CP
 				{
-					if( et==ExecType.SPARK && !FORCE_DIST_SORT_INDEXES)
-						bSortSPRewriteApplicable = isSortSPRewriteApplicable();
-					
-					Lop transform1 = constructCPOrSparkSortLop(input, by, desc, ixret, et, bSortSPRewriteApplicable);
+					Lop transform1 = constructCPOrSparkSortLop(input, by, desc, ixret, et, false);
 					setOutputDimensions(transform1);
 					setLineNumbers(transform1);
-					
 					setLops(transform1);
 				}
 				break;
@@ -402,7 +402,7 @@ public class ReorgOp extends Hop implements MultiThreadedHop
 		throws HopsException, LopsException
 	{
 		Transform transform1 = new Transform( input.constructLops(), HopsTransf2Lops.get(ReOrgOp.SORT), 
-				     input.getDataType(), input.getValueType(), et, bSortIndInMem);
+				input.getDataType(), input.getValueType(), et, bSortIndInMem);
 		
 		for( Hop c : new Hop[]{by,desc,ixret} ) {
 			Lop ltmp = c.constructLops();

http://git-wip-us.apache.org/repos/asf/systemml/blob/ec024661/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 ad95552..b365abb 100644
--- a/src/main/java/org/apache/sysml/parser/ParameterizedBuiltinFunctionExpression.java
+++ b/src/main/java/org/apache/sysml/parser/ParameterizedBuiltinFunctionExpression.java
@@ -444,15 +444,16 @@ public class ParameterizedBuiltinFunctionExpression extends DataIdentifier
 			orderby = new IntIdentifier(1);
 			addVarParam("by", orderby);
 		}
-		else if( orderby !=null && orderby.getOutput().getDataType() != DataType.SCALAR ){				
-			raiseValidateError("Orderby column 'by' is of type '"+orderby.getOutput().getDataType()+"'. Please, specify a scalar order by column index.", conditional, LanguageErrorCodes.INVALID_PARAMETERS);
-		}	
+		else if( orderby !=null && !(orderby.getOutput().getDataType().isScalar() 
+			|| orderby.getOutput().getDataType().isMatrix()) ) {
+			raiseValidateError("Orderby column 'by' is of type '"+orderby.getOutput().getDataType()+"'. Please, use a scalar or row vector to specify column indexes.", conditional, LanguageErrorCodes.INVALID_PARAMETERS);
+		}
 		
 		Expression decreasing = getVarParam("decreasing"); //[OPTIONAL] DECREASING
 		if( decreasing == null ) { //default: ascending
 			addVarParam("decreasing", new BooleanIdentifier(false));
 		}
-		else if( decreasing!=null && decreasing.getOutput().getDataType() != DataType.SCALAR ){				
+		else if( decreasing!=null && decreasing.getOutput().getDataType() != DataType.SCALAR ){
 			raiseValidateError("Ordering 'decreasing' is of type '"+decreasing.getOutput().getDataType()+"', '"+decreasing.getOutput().getValueType()+"'. Please, specify 'decreasing' as a scalar boolean.", conditional, LanguageErrorCodes.INVALID_PARAMETERS);
 		}
 		
@@ -461,7 +462,7 @@ public class ParameterizedBuiltinFunctionExpression extends DataIdentifier
 			indexreturn = new BooleanIdentifier(false);
 			addVarParam("index.return", indexreturn);
 		}
-		else if( indexreturn!=null && indexreturn.getOutput().getDataType() != DataType.SCALAR ){				
+		else if( indexreturn!=null && indexreturn.getOutput().getDataType() != DataType.SCALAR ){
 			raiseValidateError("Return type 'index.return' is of type '"+indexreturn.getOutput().getDataType()+"', '"+indexreturn.getOutput().getValueType()+"'. Please, specify 'indexreturn' as a scalar boolean.", conditional, LanguageErrorCodes.INVALID_PARAMETERS);
 		}
 		long dim2 = ( indexreturn instanceof BooleanIdentifier ) ? 

http://git-wip-us.apache.org/repos/asf/systemml/blob/ec024661/src/main/java/org/apache/sysml/runtime/functionobjects/SortIndex.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/functionobjects/SortIndex.java b/src/main/java/org/apache/sysml/runtime/functionobjects/SortIndex.java
index c72735a..0d6d481 100644
--- a/src/main/java/org/apache/sysml/runtime/functionobjects/SortIndex.java
+++ b/src/main/java/org/apache/sysml/runtime/functionobjects/SortIndex.java
@@ -34,26 +34,22 @@ public class SortIndex extends IndexFunction
 {
 	private static final long serialVersionUID = -8446389232078905200L;
 
-	private int     _col        = -1;
-	private boolean _decreasing = false;
-	private boolean _ixreturn   = false;
+	private final int[] _cols;
+	private final boolean _decreasing;
+	private final boolean _ixreturn;
 	
-	private SortIndex() {
-		// nothing to do here
+	public SortIndex(int col, boolean decreasing, boolean indexreturn) {
+		this(new int[]{col}, decreasing, indexreturn);
 	}
-
-	public static SortIndex getSortIndexFnObject(int col, boolean decreasing, boolean indexreturn) 
-	{
-		SortIndex ix = new SortIndex();
-		ix._col = col;
-		ix._decreasing = decreasing;
-		ix._ixreturn = indexreturn;
-		
-		return ix;
+	
+	public SortIndex(int[] cols, boolean decreasing, boolean indexreturn) {
+		_cols = cols;
+		_decreasing = decreasing;
+		_ixreturn = indexreturn;
 	}
 
-	public int getCol() {
-		return _col;
+	public int[] getCols() {
+		return _cols;
 	}
 	
 	public boolean getDecreasing() {

http://git-wip-us.apache.org/repos/asf/systemml/blob/ec024661/src/main/java/org/apache/sysml/runtime/instructions/cp/ReorgCPInstruction.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/instructions/cp/ReorgCPInstruction.java b/src/main/java/org/apache/sysml/runtime/instructions/cp/ReorgCPInstruction.java
index 3788d68..96b4644 100644
--- a/src/main/java/org/apache/sysml/runtime/instructions/cp/ReorgCPInstruction.java
+++ b/src/main/java/org/apache/sysml/runtime/instructions/cp/ReorgCPInstruction.java
@@ -31,6 +31,7 @@ import org.apache.sysml.runtime.instructions.InstructionUtils;
 import org.apache.sysml.runtime.matrix.data.MatrixBlock;
 import org.apache.sysml.runtime.matrix.operators.Operator;
 import org.apache.sysml.runtime.matrix.operators.ReorgOperator;
+import org.apache.sysml.runtime.util.DataConverter;
 
 public class ReorgCPInstruction extends UnaryCPInstruction {
 	// sort-specific attributes (to enable variable attributes)
@@ -115,8 +116,8 @@ public class ReorgCPInstruction extends UnaryCPInstruction {
 			CPOperand col = new CPOperand(parts[2]);
 			CPOperand desc = new CPOperand(parts[3]);
 			CPOperand ixret = new CPOperand(parts[4]);
-			return new ReorgCPInstruction(new ReorgOperator(SortIndex.getSortIndexFnObject(1,false,false)), 
-						in, out, col, desc, ixret, opcode, str);
+			return new ReorgCPInstruction(new ReorgOperator(new SortIndex(1,false,false)), 
+				in, out, col, desc, ixret, opcode, str);
 		}
 		else {
 			throw new DMLRuntimeException("Unknown opcode while parsing a ReorgInstruction: " + str);
@@ -132,18 +133,20 @@ public class ReorgCPInstruction extends UnaryCPInstruction {
 		ReorgOperator r_op = (ReorgOperator) _optr;
 		if( r_op.fn instanceof SortIndex ) {
 			//additional attributes for sort
-			int col = (int)ec.getScalarInput(_col.getName(), _col.getValueType(), _col.isLiteral()).getLongValue();
+			int[] cols = _col.getDataType().isMatrix() ? DataConverter.convertToIntVector(ec.getMatrixInput(_col.getName())) :
+				new int[]{(int)ec.getScalarInput(_col.getName(), _col.getValueType(), _col.isLiteral()).getLongValue()};
 			boolean desc = ec.getScalarInput(_desc.getName(), _desc.getValueType(), _desc.isLiteral()).getBooleanValue();
 			boolean ixret = ec.getScalarInput(_ixret.getName(), _ixret.getValueType(), _ixret.isLiteral()).getBooleanValue();
-			r_op = r_op.setFn(SortIndex.getSortIndexFnObject(col, desc, ixret));
+			r_op = r_op.setFn(new SortIndex(cols, desc, ixret));
 		}
 		
 		//execute operation
 		MatrixBlock soresBlock = (MatrixBlock) (matBlock.reorgOperations(r_op, new MatrixBlock(), 0, 0, 0));
 		
 		//release inputs/outputs
+		if( r_op.fn instanceof SortIndex && _col.getDataType().isMatrix() )
+			ec.releaseMatrixInput(_col.getName());
 		ec.releaseMatrixInput(input1.getName(), getExtendedOpcode());
 		ec.setMatrixOutput(output.getName(), soresBlock, getExtendedOpcode());
 	}
-	
 }

http://git-wip-us.apache.org/repos/asf/systemml/blob/ec024661/src/main/java/org/apache/sysml/runtime/instructions/spark/ReorgSPInstruction.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/instructions/spark/ReorgSPInstruction.java b/src/main/java/org/apache/sysml/runtime/instructions/spark/ReorgSPInstruction.java
index f8b92ac..c742a0b 100644
--- a/src/main/java/org/apache/sysml/runtime/instructions/spark/ReorgSPInstruction.java
+++ b/src/main/java/org/apache/sysml/runtime/instructions/spark/ReorgSPInstruction.java
@@ -52,6 +52,7 @@ import org.apache.sysml.runtime.matrix.data.MatrixIndexes;
 import org.apache.sysml.runtime.matrix.mapred.IndexedMatrixValue;
 import org.apache.sysml.runtime.matrix.operators.Operator;
 import org.apache.sysml.runtime.matrix.operators.ReorgOperator;
+import org.apache.sysml.runtime.util.DataConverter;
 import org.apache.sysml.runtime.util.UtilFunctions;
 
 public class ReorgSPInstruction extends UnarySPInstruction {
@@ -103,13 +104,13 @@ public class ReorgSPInstruction extends UnarySPInstruction {
 			CPOperand col = new CPOperand(parts[2]);
 			CPOperand desc = new CPOperand(parts[3]);
 			CPOperand ixret = new CPOperand(parts[4]);
-			boolean bSortIndInMem = false;		
+			boolean bSortIndInMem = false;
 			
 			if(parts.length > 5)
 				bSortIndInMem = Boolean.parseBoolean(parts[6]);
-						
-			return new ReorgSPInstruction(new ReorgOperator(SortIndex.getSortIndexFnObject(1,false,false)), 
-					                      in, col, desc, ixret, out, opcode, bSortIndInMem, str);
+			
+			return new ReorgSPInstruction(new ReorgOperator(new SortIndex(1,false,false)),
+				in, col, desc, ixret, out, opcode, bSortIndInMem, str);
 		}
 		else {
 			throw new DMLRuntimeException("Unknown opcode while parsing a ReorgInstruction: " + str);
@@ -156,16 +157,23 @@ public class ReorgSPInstruction extends UnarySPInstruction {
 			// Sort by column 'col' in ascending/descending order and return either index/value
 			
 			//get parameters
-			long col = ec.getScalarInput(_col.getName(), _col.getValueType(), _col.isLiteral()).getLongValue();
+			long[] cols = _col.getDataType().isMatrix() ? DataConverter.convertToLongVector(ec.getMatrixInput(_col.getName())) :
+				new long[]{ec.getScalarInput(_col.getName(), _col.getValueType(), _col.isLiteral()).getLongValue()};
 			boolean desc = ec.getScalarInput(_desc.getName(), _desc.getValueType(), _desc.isLiteral()).getBooleanValue();
 			boolean ixret = ec.getScalarInput(_ixret.getName(), _ixret.getValueType(), _ixret.isLiteral()).getBooleanValue();
 			boolean singleCol = (mcIn.getCols() == 1);
 			
+			//error handling unsupported operations
+			//TODO additional spark sort runtime with multiple order columns
+			if( cols.length > 1 ) 
+				LOG.warn("Unsupported sort with multiple order-by columns. Falling back first sort column.");
+			long col = cols[0];
+			
 			// extract column (if necessary) and sort 
 			out = in1;
 			if( !singleCol ){
 				out = out.filter(new IsBlockInRange(1, mcIn.getRows(), col, col, mcIn))
-						 .mapValues(new ExtractColumn((int)UtilFunctions.computeCellInBlock(col, mcIn.getColsPerBlock())));
+					.mapValues(new ExtractColumn((int)UtilFunctions.computeCellInBlock(col, mcIn.getColsPerBlock())));
 			}
 			
 			//actual index/data sort operation
@@ -177,8 +185,8 @@ public class ReorgSPInstruction extends UnarySPInstruction {
 			}
 			else { //sort multi-column matrix
 				if (! _bSortIndInMem)
-				        out = RDDSortUtils.sortDataByVal(out, in1, !desc, mcIn.getRows(), mcIn.getCols(), mcIn.getRowsPerBlock(), mcIn.getColsPerBlock());
-				else					
+					out = RDDSortUtils.sortDataByVal(out, in1, !desc, mcIn.getRows(), mcIn.getCols(), mcIn.getRowsPerBlock(), mcIn.getColsPerBlock());
+				else
 					out = RDDSortUtils.sortDataByValMemSort(out, in1, !desc, mcIn.getRows(), mcIn.getCols(), mcIn.getRowsPerBlock(), mcIn.getColsPerBlock(), sec, (ReorgOperator) _optr);
 			}
 		}
@@ -187,6 +195,8 @@ public class ReorgSPInstruction extends UnarySPInstruction {
 		}
 		
 		//store output rdd handle
+		if( opcode.equalsIgnoreCase("rsort") && _col.getDataType().isMatrix() )
+			sec.releaseMatrixInput(_col.getName());
 		updateReorgMatrixCharacteristics(sec);
 		sec.setRDDHandleForVariable(output.getName(), out);
 		sec.addLineageRDD(output.getName(), input1.getName());

http://git-wip-us.apache.org/repos/asf/systemml/blob/ec024661/src/main/java/org/apache/sysml/runtime/instructions/spark/utils/RDDSortUtils.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/instructions/spark/utils/RDDSortUtils.java b/src/main/java/org/apache/sysml/runtime/instructions/spark/utils/RDDSortUtils.java
index 2fddce3..bf63f0d 100644
--- a/src/main/java/org/apache/sysml/runtime/instructions/spark/utils/RDDSortUtils.java
+++ b/src/main/java/org/apache/sysml/runtime/instructions/spark/utils/RDDSortUtils.java
@@ -175,7 +175,7 @@ public class RDDSortUtils
 				.toMatrixBlock(val, (int)rlen, 1, brlen, bclen, -1);
 
 		//in-memory sort operation (w/ index return: source index in target position)
-		ReorgOperator lrop = new ReorgOperator(SortIndex.getSortIndexFnObject(1, !asc, true));	
+		ReorgOperator lrop = new ReorgOperator(new SortIndex(1, !asc, true));
 		MatrixBlock sortedIx = (MatrixBlock) inMatBlock
 				.reorgOperations(lrop, new MatrixBlock(), -1, -1, -1);
 		

http://git-wip-us.apache.org/repos/asf/systemml/blob/ec024661/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 28c3bf6..b86dc96 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
@@ -114,7 +114,7 @@ public class LibMatrixReorg
 				return diag(in, out); 
 			case SORT:      
 				SortIndex ix = (SortIndex) op.fn;
-				return sort(in, out, ix.getCol(), ix.getDecreasing(), ix.getIndexReturn());
+				return sort(in, out, ix.getCols(), ix.getDecreasing(), ix.getIndexReturn());
 			
 			default:        
 				throw new DMLRuntimeException("Unsupported reorg operator: "+op.fn);
@@ -317,7 +317,7 @@ public class LibMatrixReorg
 		return out;
 	}
 
-	public static MatrixBlock sort(MatrixBlock in, MatrixBlock out, int by, boolean desc, boolean ixret) 
+	public static MatrixBlock sort(MatrixBlock in, MatrixBlock out, int[] by, boolean desc, boolean ixret) 
 		throws DMLRuntimeException
 	{
 		//meta data gathering and preparation
@@ -328,8 +328,9 @@ public class LibMatrixReorg
 		out.nonZeros = ixret ? rlen : in.nonZeros;
 		
 		//step 1: error handling
-		if( by <= 0 || clen < by )
-			throw new DMLRuntimeException("Sort configuration issue: non-existing orderby column: "+by+" ("+rlen+"x"+clen+" input).");
+		if( !isValidSortByList(by, clen) )
+			throw new DMLRuntimeException("Sort configuration issue: invalid orderby columns: "
+				+ Arrays.toString(by)+" ("+rlen+"x"+clen+" input).");
 		
 		//step 2: empty block / special case handling
 		if( !ixret ) //SORT DATA
@@ -350,8 +351,9 @@ public class LibMatrixReorg
 		{
 			if( in.isEmptyBlock(false) ) { //EMPTY INPUT BLOCK
 				out.allocateDenseBlock(false);
-				for( int i=0; i<rlen; i++ ) //seq(1,n)
-					out.setValueDenseUnsafe(i, 0, i+1);
+				double[] c = out.getDenseBlock();
+				for( int i=0; i<rlen; i++ )
+					c[i] = i+1; //seq(1,n)
 				return out;
 			}
 		}
@@ -363,12 +365,16 @@ public class LibMatrixReorg
 		double[] values = new double[rlen];
 		for( int i=0; i<rlen; i++ ) {
 			vix[i] = i;
-			values[i] = in.quickGetValue(i, by-1);
+			values[i] = in.quickGetValue(i, by[0]-1);
 		}
 		
 		//sort index vector on extracted data (unstable)
 		SortUtils.sortByValue(0, rlen, values, vix);
-
+		
+		//sort by secondary columns if required (in-place)
+		if( by.length > 1 )
+			sortBySecondary(0, rlen, values, vix, in, by, 1);
+		
 		//flip order if descending requested (note that this needs to happen
 		//before we ensure stable outputs, hence we also flip values)
 		if(desc) {
@@ -377,42 +383,25 @@ public class LibMatrixReorg
 		}
 		
 		//final pass to ensure stable output
-		for( int i=0; i<rlen-1; i++ ) {
-			double tmp = values[i];
-			//determine run of equal values
-			int len = 0;
-			while( i+len+1<rlen && tmp==values[i+len+1] )
-				len++;
-			//unstable sort of run indexes (equal value guaranteed)
-			if( len>0 ) {
-				Arrays.sort(vix, i, i+len+1);
-				i += len; //skip processed run
-			}
-		}
+		sortIndexesStable(0, rlen, values, vix, in, by, 1);
 
 		//step 4: create output matrix (guaranteed non-empty, see step 2)
-		if( !ixret )
-		{
+		if( !ixret ) {
 			//copy input data in sorted order into result
-			if( !sparse ) //DENSE
-			{
+			if( !sparse ) { //DENSE
 				out.allocateDenseBlock(false);
-				for( int i=0; i<rlen; i++ ) {
+				for( int i=0; i<rlen; i++ )
 					System.arraycopy(in.denseBlock, vix[i]*clen, out.denseBlock, i*clen, clen);
-				}
 			}
-			else //SPARSE
-			{
+			else { //SPARSE
 				out.allocateSparseRowsBlock(false);
 				for( int i=0; i<rlen; i++ )
-					if( !in.sparseBlock.isEmpty(vix[i]) ) {
+					if( !in.sparseBlock.isEmpty(vix[i]) )
 						out.sparseBlock.set(i, in.sparseBlock.get(vix[i]),
 							!SHALLOW_COPY_REORG); //row remains unchanged
-					}
 			}
 		}
-		else
-		{
+		else {
 			//copy sorted index vector into result
 			out.allocateDenseBlock(false);
 			for( int i=0; i<rlen; i++ )
@@ -2031,11 +2020,9 @@ public class LibMatrixReorg
 	 * 
 	 * @param m1 matrix
 	 */
-	private static void sortReverseDense( MatrixBlock m1 )
-	{
+	private static void sortReverseDense( MatrixBlock m1 ) {
 		int rlen = m1.rlen;
 		double[] a = m1.denseBlock;
-		
 		for( int i=0; i<rlen/2; i++ ) {
 			double tmp = a[i];
 			a[i] = a[rlen - i -1];
@@ -2043,10 +2030,8 @@ public class LibMatrixReorg
 		}
 	}
 
-	private static void sortReverseDense( int[] a )
-	{
+	private static void sortReverseDense( int[] a ) {
 		int rlen = a.length;
-		
 		for( int i=0; i<rlen/2; i++ ) {
 			int tmp = a[i];
 			a[i] = a[rlen - i -1];
@@ -2054,16 +2039,71 @@ public class LibMatrixReorg
 		}
 	}
 
-	private static void sortReverseDense( double[] a )
-	{
+	private static void sortReverseDense( double[] a ) {
 		int rlen = a.length;
-		
 		for( int i=0; i<rlen/2; i++ ) {
 			double tmp = a[i];
 			a[i] = a[rlen - i -1];
 			a[rlen - i - 1] = tmp;
 		}
 	}
+	
+	private static void sortBySecondary(int rl, int ru, double[] values, int[] vix, MatrixBlock in, int[] by, int off) {
+		//find runs of equal values in current offset and index range
+		//replace value by next column, sort, and recurse until single value
+		for( int i=rl; i<ru-1; i++ ) {
+			double tmp = values[i];
+			//determine run of equal values
+			int len = 0;
+			while( i+len+1<ru && tmp==values[i+len+1] )
+				len++;
+			//temp value replacement and recursive sort
+			if( len > 0 ) {
+				double old = values[i];
+				//extract values of next column
+				for(int j=i; j<i+len+1; j++)
+					values[j] = in.quickGetValue(vix[j], by[off]-1);
+				//sort values, incl recursive decent
+				SortUtils.sortByValue(i, i+len+1, values, vix);
+				if( off+1 < by.length )
+					sortBySecondary(i, i+len+1, values, vix, in, by, off+1);
+				//reset values of previous level
+				Arrays.fill(values, i, i+len+1, old);
+				i += len; //skip processed run
+			}
+		}
+	}
+	
+	private static void sortIndexesStable(int rl, int ru, double[] values, int[] vix, MatrixBlock in, int[] by, int off) {
+		for( int i=rl; i<ru-1; i++ ) {
+			double tmp = values[i];
+			//determine run of equal values
+			int len = 0;
+			while( i+len+1<ru && tmp==values[i+len+1] )
+				len++;
+			//temp value replacement and recursive decent
+			if( len > 0 ) {
+				if( off < by.length ) {
+					//extract values of next column
+					for(int j=i; j<i+len+1; j++)
+						values[j] = in.quickGetValue(vix[j], by[off]-1);
+					sortIndexesStable(i, i+len+1, values, vix, in, by, off+1);
+				}
+				else //unstable sort of run indexes (equal value guaranteed)
+					Arrays.sort(vix, i, i+len+1);
+				i += len; //skip processed run
+			}
+		}
+	}
+	
+	private static boolean isValidSortByList(int[] by, int clen) {
+		if( by == null || by.length==0 || by.length>clen )
+			return false;
+		for(int i=0; i<by.length; i++)
+			if( by[i] <= 0 || clen < by[i])
+				return false;
+		return true;
+	}
 
 	@SuppressWarnings("unused")
 	private static void countAgg( int[] c, int[] ai, final int len ) 

http://git-wip-us.apache.org/repos/asf/systemml/blob/ec024661/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 88efde2..924d6c5 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
@@ -3462,7 +3462,7 @@ public class MatrixBlock extends MatrixValue implements CacheBlock, Externalizab
 		{
 			//SPECIAL case (operators with special performance requirements, 
 			//or size-dependent special behavior)
-			//currently supported opcodes: r', rdiag, rsort
+			//currently supported opcodes: r', rdiag, rsort, rev
 			LibMatrixReorg.reorg(this, result, op);
 		}
 		else 
@@ -4755,7 +4755,7 @@ public class MatrixBlock extends MatrixValue implements CacheBlock, Externalizab
 		tdw.quickSetValue(0, 1, zero_wt); //num zeros in input
 		
 		// Sort td and tw based on values inside td (ascending sort), incl copy into result
-		SortIndex sfn = SortIndex.getSortIndexFnObject(1, false, false);
+		SortIndex sfn = new SortIndex(1, false, false);
 		ReorgOperator rop = new ReorgOperator(sfn);
 		LibMatrixReorg.reorg(tdw, (MatrixBlock)result, rop);
 		

http://git-wip-us.apache.org/repos/asf/systemml/blob/ec024661/src/main/java/org/apache/sysml/runtime/util/DataConverter.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/util/DataConverter.java b/src/main/java/org/apache/sysml/runtime/util/DataConverter.java
index 5a91564..af69e81 100644
--- a/src/main/java/org/apache/sysml/runtime/util/DataConverter.java
+++ b/src/main/java/org/apache/sysml/runtime/util/DataConverter.java
@@ -278,32 +278,47 @@ public class DataConverter
 		return ret;
 	}
 
-	public static int[] convertToIntVector( MatrixBlock mb)
-	{
+	public static int[] convertToIntVector( MatrixBlock mb) {
 		int rows = mb.getNumRows();
 		int cols = mb.getNumColumns();
 		int[] ret = new int[rows*cols]; //0-initialized
-		
-		
-		if( mb.getNonZeros() > 0 )
-		{
-			if( mb.isInSparseFormat() )
-			{
-				Iterator<IJV> iter = mb.getSparseBlockIterator();
-				while( iter.hasNext() ) {
-					IJV cell = iter.next();
-					ret[cell.getI()*cols+cell.getJ()] = (int)cell.getV();
-				}
+		if( mb.isEmptyBlock(false) ) 
+			return ret;
+		if( mb.isInSparseFormat() ) {
+			Iterator<IJV> iter = mb.getSparseBlockIterator();
+			while( iter.hasNext() ) {
+				IJV cell = iter.next();
+				ret[cell.getI()*cols+cell.getJ()] = (int)cell.getV();
 			}
-			else
-			{
-				//memcopy row major representation if at least 1 non-zero
-				for( int i=0, cix=0; i<rows; i++ )
-					for( int j=0; j<cols; j++, cix++ )
-						ret[cix] = (int)(mb.getValueDenseUnsafe(i, j));
+		}
+		else {
+			//memcopy row major representation if at least 1 non-zero
+			for( int i=0, cix=0; i<rows; i++ )
+				for( int j=0; j<cols; j++, cix++ )
+					ret[cix] = (int)(mb.getValueDenseUnsafe(i, j));
+		}
+		return ret;
+	}
+	
+	public static long[] convertToLongVector( MatrixBlock mb) {
+		int rows = mb.getNumRows();
+		int cols = mb.getNumColumns();
+		long[] ret = new long[rows*cols]; //0-initialized
+		if( mb.isEmptyBlock(false) ) 
+			return ret;
+		if( mb.isInSparseFormat() ) {
+			Iterator<IJV> iter = mb.getSparseBlockIterator();
+			while( iter.hasNext() ) {
+				IJV cell = iter.next();
+				ret[cell.getI()*cols+cell.getJ()] = (int)cell.getV();
 			}
 		}
-		
+		else {
+			//memcopy row major representation if at least 1 non-zero
+			for( int i=0, cix=0; i<rows; i++ )
+				for( int j=0; j<cols; j++, cix++ )
+					ret[cix] = (int)(mb.getValueDenseUnsafe(i, j));
+		}
 		return ret;
 	}
 

http://git-wip-us.apache.org/repos/asf/systemml/blob/ec024661/src/test/java/org/apache/sysml/test/integration/functions/reorg/MultipleOrderByColsTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/sysml/test/integration/functions/reorg/MultipleOrderByColsTest.java b/src/test/java/org/apache/sysml/test/integration/functions/reorg/MultipleOrderByColsTest.java
new file mode 100644
index 0000000..67c6487
--- /dev/null
+++ b/src/test/java/org/apache/sysml/test/integration/functions/reorg/MultipleOrderByColsTest.java
@@ -0,0 +1,179 @@
+/*
+ * 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.
+ */
+
+package org.apache.sysml.test.integration.functions.reorg;
+
+import java.util.HashMap;
+
+import org.junit.Test;
+
+import org.apache.sysml.api.DMLScript;
+import org.apache.sysml.api.DMLScript.RUNTIME_PLATFORM;
+import org.apache.sysml.lops.LopProperties.ExecType;
+import org.apache.sysml.runtime.matrix.data.MatrixValue.CellIndex;
+import org.apache.sysml.test.integration.AutomatedTestBase;
+import org.apache.sysml.test.integration.TestConfiguration;
+import org.apache.sysml.test.utils.TestUtils;
+
+public class MultipleOrderByColsTest extends AutomatedTestBase 
+{
+	private final static String TEST_NAME1 = "OrderMultiBy";
+	
+	private final static String TEST_DIR = "functions/reorg/";
+	private static final String TEST_CLASS_DIR = TEST_DIR + MultipleOrderByColsTest.class.getSimpleName() + "/";
+	private final static double eps = 1e-10;
+	
+	private final static int rows = 1017;
+	private final static int cols = 736;
+	private final static double sparsity1 = 0.7;
+	private final static double sparsity2 = 0.07;
+
+	@Override
+	public void setUp() {
+		TestUtils.clearAssertionInformation();
+		addTestConfiguration(TEST_NAME1, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME1,new String[]{"B"}));
+	}
+	
+	@Test
+	public void testOrderDenseAscDataCP() {
+		runOrderTest(TEST_NAME1, false, false, false, ExecType.CP);
+	}
+	
+	@Test
+	public void testOrderDenseAscIxCP() {
+		runOrderTest(TEST_NAME1, false, false, true, ExecType.CP);
+	}
+	
+	@Test
+	public void testOrderDenseDescDataCP() {
+		runOrderTest(TEST_NAME1, false, true, false, ExecType.CP);
+	}
+	
+	@Test
+	public void testOrderDenseDescIxCP() {
+		runOrderTest(TEST_NAME1, false, true, true, ExecType.CP);
+	}
+	
+	@Test
+	public void testOrderSparseAscDataCP() {
+		runOrderTest(TEST_NAME1, true, false, false, ExecType.CP);
+	}
+	
+	@Test
+	public void testOrderSparseAscIxCP() {
+		runOrderTest(TEST_NAME1, true, false, true, ExecType.CP);
+	}
+	
+	@Test
+	public void testOrderSparseDescDataCP() {
+		runOrderTest(TEST_NAME1, true, true, false, ExecType.CP);
+	}
+	
+	@Test
+	public void testOrderSparseDescIxCP() {
+		runOrderTest(TEST_NAME1, true, true, true, ExecType.CP);
+	}
+
+//TODO enable together with additional spark sort runtime
+//	@Test
+//	public void testOrderDenseAscDataSP() {
+//		runOrderTest(TEST_NAME1, false, false, false, ExecType.SPARK);
+//	}
+//	
+//	@Test
+//	public void testOrderDenseAscIxSP() {
+//		runOrderTest(TEST_NAME1, false, false, true, ExecType.SPARK);
+//	}
+//	
+//	@Test
+//	public void testOrderDenseDescDataSP() {
+//		runOrderTest(TEST_NAME1, false, true, false, ExecType.SPARK);
+//	}
+//	
+//	@Test
+//	public void testOrderDenseDescIxSP() {
+//		runOrderTest(TEST_NAME1, false, true, true, ExecType.SPARK);
+//	}
+//	
+//	@Test
+//	public void testOrderSparseAscDataSP() {
+//		runOrderTest(TEST_NAME1, true, false, false, ExecType.SPARK);
+//	}
+//	
+//	@Test
+//	public void testOrderSparseAscIxSP() {
+//		runOrderTest(TEST_NAME1, true, false, true, ExecType.SPARK);
+//	}
+//	
+//	@Test
+//	public void testOrderSparseDescDataSP() {
+//		runOrderTest(TEST_NAME1, true, true, false, ExecType.SPARK);
+//	}
+//	
+//	@Test
+//	public void testOrderSparseDescIxSP() {
+//		runOrderTest(TEST_NAME1, true, true, true, ExecType.SPARK);
+//	}
+	
+	private void runOrderTest( String testname, boolean sparse, boolean desc, boolean ixret, ExecType et)
+	{
+		RUNTIME_PLATFORM platformOld = rtplatform;
+		switch( et ){
+			case MR: rtplatform = RUNTIME_PLATFORM.HADOOP; break;
+			case SPARK: rtplatform = RUNTIME_PLATFORM.SPARK; break;
+			default: rtplatform = RUNTIME_PLATFORM.HYBRID; break;
+		}
+	
+		boolean sparkConfigOld = DMLScript.USE_LOCAL_SPARK_CONFIG;
+		if( rtplatform == RUNTIME_PLATFORM.SPARK )
+			DMLScript.USE_LOCAL_SPARK_CONFIG = true;
+		
+		try
+		{
+			String TEST_NAME = testname;
+			TestConfiguration config = getTestConfiguration(TEST_NAME);
+			loadTestConfiguration(config);
+			
+			String HOME = SCRIPT_DIR + TEST_DIR;
+			fullDMLScriptName = HOME + TEST_NAME + ".dml";
+			programArgs = new String[]{"-explain","-args", input("A"), 
+				String.valueOf(desc).toUpperCase(), String.valueOf(ixret).toUpperCase(), output("B") };
+			
+			fullRScriptName = HOME + TEST_NAME + ".R";
+			rCmd = "Rscript" + " " + fullRScriptName + " " + inputDir() + " " +
+				String.valueOf(desc).toUpperCase()+" "+String.valueOf(ixret).toUpperCase()+" "+expectedDir();
+			
+			double sparsity = (sparse) ? sparsity2 : sparsity1; //with rounding for duplicates
+			double[][] A = TestUtils.round(getRandomMatrix(rows, cols, -10, 10, sparsity, 7));
+			writeInputMatrixWithMTD("A", A, true);
+			
+			runTest(true, false, null, -1);
+			runRScript(true);
+			
+			//compare matrices
+			HashMap<CellIndex, Double> dmlfile = readDMLMatrixFromHDFS("B");
+			HashMap<CellIndex, Double> rfile  = readRMatrixFromFS("B");
+			TestUtils.compareMatrices(dmlfile, rfile, eps, "Stat-DML", "Stat-R");
+		}
+		finally {
+			rtplatform = platformOld;
+			DMLScript.USE_LOCAL_SPARK_CONFIG = sparkConfigOld;
+		}
+	}
+}

http://git-wip-us.apache.org/repos/asf/systemml/blob/ec024661/src/test/scripts/functions/reorg/OrderMultiBy.R
----------------------------------------------------------------------
diff --git a/src/test/scripts/functions/reorg/OrderMultiBy.R b/src/test/scripts/functions/reorg/OrderMultiBy.R
new file mode 100644
index 0000000..374dad0
--- /dev/null
+++ b/src/test/scripts/functions/reorg/OrderMultiBy.R
@@ -0,0 +1,42 @@
+#-------------------------------------------------------------
+#
+# 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")
+
+A = readMM(paste(args[1], "A.mtx", sep=""))
+desc = as.logical(args[2]);
+ixret = as.logical(args[3]);
+
+col1 = A[,3];
+col2 = A[,7];
+col3 = A[,14];
+
+
+if( ixret ) {
+  B = order(col1, col2, col3, decreasing=desc);
+} else {
+  B = A[order(col1, col2, col3, decreasing=desc),];
+}
+
+writeMM(as(B,"CsparseMatrix"), paste(args[4], "B", sep=""))

http://git-wip-us.apache.org/repos/asf/systemml/blob/ec024661/src/test/scripts/functions/reorg/OrderMultiBy.dml
----------------------------------------------------------------------
diff --git a/src/test/scripts/functions/reorg/OrderMultiBy.dml b/src/test/scripts/functions/reorg/OrderMultiBy.dml
new file mode 100644
index 0000000..f6d2246
--- /dev/null
+++ b/src/test/scripts/functions/reorg/OrderMultiBy.dml
@@ -0,0 +1,33 @@
+#-------------------------------------------------------------
+#
+# 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.
+#
+#-------------------------------------------------------------
+
+
+A = read($1);
+
+ix = matrix("3 7 14", rows=1, cols=3)
+
+#B = order(target=A, by=14, decreasing=$2, index.return=$3);
+#B = order(target=B, by=7, decreasing=$2, index.return=$3);
+#B = order(target=B, by=3, decreasing=$2, index.return=$3);
+
+B = order(target=A, by=ix, decreasing=$2, index.return=$3);
+
+write(B, $4, format="text");  
\ No newline at end of file