From 7cf4b23873baa7ec63faca2432e8a9c633a2a1c5 Mon Sep 17 00:00:00 2001
From: baunsgaard <baunsgaard@tugraz.at>
Date: Wed, 24 Mar 2021 19:55:01 +0100
Subject: [PATCH 1/2] [MINOR] Fix native warning on sparse MM

---
 .../runtime/matrix/data/LibMatrixNative.java  | 20 +++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/src/main/java/org/apache/sysds/runtime/matrix/data/LibMatrixNative.java b/src/main/java/org/apache/sysds/runtime/matrix/data/LibMatrixNative.java
index ce7fbba65ba..f6f24c4ebad 100644
--- a/src/main/java/org/apache/sysds/runtime/matrix/data/LibMatrixNative.java
+++ b/src/main/java/org/apache/sysds/runtime/matrix/data/LibMatrixNative.java
@@ -79,11 +79,12 @@ public static void matrixMult(MatrixBlock m1, MatrixBlock m2, MatrixBlock ret, i
 			return;
 		}
 		
-		if( NativeHelper.isNativeLibraryLoaded()
-			&& !isMatMultMemoryBound(m1.rlen, m1.clen, m2.clen) 
+		boolean isValidForNative = !isMatMultMemoryBound(m1.rlen, m1.clen, m2.clen) 
 			&& !m1.isInSparseFormat() && !m2.isInSparseFormat()
-			&& m1.getDenseBlock().isContiguous() && m2.getDenseBlock().isContiguous()
-			&& 8L * ret.getLength() < Integer.MAX_VALUE ) //contiguous but not allocated
+			&& m1.getDenseBlock().isContiguous() && m2.getDenseBlock().isContiguous() //contiguous but not allocated
+			&& 8L * ret.getLength() < Integer.MAX_VALUE;
+
+		if( NativeHelper.isNativeLibraryLoaded() &&  isValidForNative ) 
 		{
 			ret.sparse = false;
 			ret.allocateDenseBlock();
@@ -114,11 +115,14 @@ public static void matrixMult(MatrixBlock m1, MatrixBlock m2, MatrixBlock ret, i
 			}
 			//else record failure and fallback to java
 			Statistics.incrementNativeFailuresCounter();
+			LOG.warn("matrixMult: Native mat mult failed. Falling back to java version ("
+				+ "loaded=" + NativeHelper.isNativeLibraryLoaded() 
+				+ ", sparse=" + (m1.isInSparseFormat() | m2.isInSparseFormat()) + ")");	
 		}
-		//fallback to default java implementation
-		LOG.warn("matrixMult: Native mat mult failed. Falling back to java version ("
-			+ "loaded=" + NativeHelper.isNativeLibraryLoaded() 
-			+ ", sparse=" + (m1.isInSparseFormat() | m2.isInSparseFormat()) + ")");
+
+		if(isValidForNative)
+			LOG.debug("Was valid for native MM but native lib was not loaded");
+		
 		if (k == 1)
 			LibMatrixMult.matrixMult(m1, m2, ret, !examSparsity);
 		else

From 2dfecc607e4ec55df40c3534132ba1aeb2b718ba Mon Sep 17 00:00:00 2001
From: baunsgaard <baunsgaard@tugraz.at>
Date: Wed, 24 Mar 2021 20:29:31 +0100
Subject: [PATCH 2/2] [MINOR] AggregateFinalResults uniform format

Aggregate of final results would use a generic aggregation if
either inputs were different format from each other.
This commit change the behavior to force uniform format.

Furthermore allocating the initial result in dense improve performance
slightly more with 1 second

The change improve performance of:
    uack+   59.179sec   ->  uack+  9.0sec
on CNN implementation using the parameterserver on MNIST.
---
 .../sysds/runtime/matrix/data/LibMatrixAgg.java   | 12 ++++++++++--
 .../sysds/runtime/matrix/data/MatrixValue.java    | 15 +++++++++++++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/apache/sysds/runtime/matrix/data/LibMatrixAgg.java b/src/main/java/org/apache/sysds/runtime/matrix/data/LibMatrixAgg.java
index 743dce92fe9..86bd412bcdc 100644
--- a/src/main/java/org/apache/sysds/runtime/matrix/data/LibMatrixAgg.java
+++ b/src/main/java/org/apache/sysds/runtime/matrix/data/LibMatrixAgg.java
@@ -274,7 +274,7 @@ public static void aggregateUnaryMatrix(MatrixBlock in, MatrixBlock out, Aggrega
 			pool.shutdown();
 			//aggregate partial results
 			if( !(uaop.indexFn instanceof ReduceCol) ) {
-				out.copy(((PartialAggTask)tasks.get(0)).getResult()); //for init
+				out.copy(((PartialAggTask)tasks.get(0)).getResult(), false); //for init
 				for( int i=1; i<tasks.size(); i++ )
 					aggregateFinalResult(uaop.aggOp, out, ((PartialAggTask)tasks.get(i)).getResult());
 			}
@@ -468,7 +468,7 @@ public static MatrixBlock aggregateTernary(MatrixBlock in1, MatrixBlock in2, Mat
 			List<Future<MatrixBlock>> rtasks = pool.invokeAll(tasks);	
 			pool.shutdown();
 			//aggregate partial results and error handling
-			ret.copy(rtasks.get(0).get()); //for init
+			ret.copy(rtasks.get(0).get(), false); //for init
 			for( int i=1; i<rtasks.size(); i++ )
 				aggregateFinalResult(op.aggOp, ret, rtasks.get(i).get());
 		}
@@ -674,6 +674,14 @@ private static void aggregateFinalResult( AggregateOperator aop, MatrixBlock out
 			laop = new AggregateOperator(0, KahanPlus.getKahanPlusFnObject(), aop.correction);
 		}
 
+		if( out.isInSparseFormat() != partout.isInSparseFormat() ){
+			if(partout.isInSparseFormat())
+				partout.sparseToDense();
+
+			if(out.isInSparseFormat())
+				out.sparseToDense();
+		}
+
 		//incremental aggregation of final results
 		if( laop.existsCorrection() )
 			out.incrementalAggregate(laop, partout);
diff --git a/src/main/java/org/apache/sysds/runtime/matrix/data/MatrixValue.java b/src/main/java/org/apache/sysds/runtime/matrix/data/MatrixValue.java
index d90e8447260..c9838d5edc4 100644
--- a/src/main/java/org/apache/sysds/runtime/matrix/data/MatrixValue.java
+++ b/src/main/java/org/apache/sysds/runtime/matrix/data/MatrixValue.java
@@ -96,7 +96,22 @@ public MatrixValue(MatrixValue that) {
 	public abstract void reset(int rl, int cl, boolean sp, long nnzs);
 	public abstract void reset(int rl, int cl, double v);
 
+	/**
+	 * Copy this MatrixValue into that MatrixValue.
+	 * 
+	 * If the MatrixValue is a MatrixBlock evaluate the sparsity of the original matrix,
+	 * and copy into either a sparse or a dense matrix.
+	 * 
+	 * @param that object to copy the values into.
+	 */
 	public abstract void copy(MatrixValue that);
+
+	/**
+	 * Copy this MatrixValue into that MatrixValue. But select sparse destination block depending on boolean parameter.
+	 * 
+	 * @param that object to copy the values into.
+	 * @param sp boolean specifying if output should be forced sparse or dense. (only applicable if the 'that' is a MatrixBlock)
+	 */
 	public abstract void copy(MatrixValue that, boolean sp);
 	
 	public abstract MatrixValue scalarOperations(ScalarOperator op, MatrixValue result);