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 2016/01/25 19:28:04 UTC

[3/3] incubator-systemml git commit: [SYSTEMML-382] Fix consistency sparse row deep-copy on row inserts

[SYSTEMML-382] Fix consistency sparse row deep-copy on row inserts

Especially with the new update in-place support we have to be careful
about shallow copies of deep rows, even if the current operation does
not modify the physical representation. This patch makes three
improvements. First, it fixes several issues introduces with the sparse
block runtime integration. Second, it extends the set row API by a flag
for deep copy requirements, which prevents unnecessary external deep
copies in case of CSR and COO. Third, it improves the performance of row
appends and specific binary-scalar operations by avoiding repeated
sparse row re-allocations. 

With this fix we can ensure result correctness again but might be too
conservative. During the rework of update in-place, we need to revisit
this and define a general policy of sparse row allocations. 

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

Branch: refs/heads/master
Commit: ed1511018656ade792385d9706a742bc0be85b4d
Parents: eb25f16
Author: Matthias Boehm <mb...@us.ibm.com>
Authored: Sun Jan 24 21:47:17 2016 -0800
Committer: Matthias Boehm <mb...@us.ibm.com>
Committed: Sun Jan 24 21:47:17 2016 -0800

----------------------------------------------------------------------
 .../runtime/matrix/data/LibMatrixBincell.java   | 10 +++---
 .../runtime/matrix/data/LibMatrixMult.java      |  4 +--
 .../runtime/matrix/data/LibMatrixReorg.java     |  4 +--
 .../sysml/runtime/matrix/data/MatrixBlock.java  | 38 +++++++++-----------
 .../sysml/runtime/matrix/data/SparseBlock.java  |  4 +--
 .../runtime/matrix/data/SparseBlockCOO.java     |  2 +-
 .../runtime/matrix/data/SparseBlockCSR.java     |  2 +-
 .../runtime/matrix/data/SparseBlockMCSR.java    |  4 +--
 8 files changed, 32 insertions(+), 36 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/ed151101/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixBincell.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixBincell.java b/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixBincell.java
index 2dfa7ba..593e201 100644
--- a/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixBincell.java
+++ b/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixBincell.java
@@ -966,7 +966,7 @@ public class LibMatrixBincell
 						double[] cvals = crow.values();
 						for(int j=0; j<alen; j++)
 							cvals[j] = (avals[apos+j] != 0) ? 1 : 0;
-						c.set(r, crow);
+						c.set(r, crow, false);
 						ret.nonZeros+=alen;
 					}
 					else //GENERAL CASE
@@ -975,7 +975,7 @@ public class LibMatrixBincell
 						if( op.fn instanceof Multiply || op.fn instanceof Multiply2 
 							|| op.fn instanceof Power2  )
 						{
-							c.allocate(r, alen, ret.clen);
+							c.allocate(r, alen);
 						}
 						
 						for(int j=apos; j<apos+alen; j++) {
@@ -1136,7 +1136,7 @@ public class LibMatrixBincell
 						
 						//temp
 						SparseRow thisRow = c.get(r);
-						c.set(r, new SparseRow(estimateSize, clen));
+						c.set(r, new SparseRow(estimateSize, clen), false);
 						
 						if(thisRow!=null)
 						{
@@ -1161,7 +1161,7 @@ public class LibMatrixBincell
 					if( !b.isEmpty(r) ) {
 						SparseRow tmp = new SparseRow( b.size(r), clen );
 						appendRightForSparseBinary(op, b.values(r), b.indexes(r), b.pos(r), b.size(r), 0, r, m1ret);
-						m1ret.sparseBlock.set(r, tmp);
+						m1ret.sparseBlock.set(r, tmp, false);
 					}
 				}				
 			}
@@ -1177,7 +1177,7 @@ public class LibMatrixBincell
 							for( int j=0; j<alen; j++ )
 								avals[j] = op.fn.execute(avals[j], 0);
 							tmp.compact(); //handle removed entries (e.g., mult, and)
-							c.set(r, tmp);
+							c.set(r, tmp, false);
 							
 							//NOTE: for left in-place, we cannot use append because it would create duplicates
 							//appendLeftForSparseBinary(op, arow.getValueContainer(), arow.getIndexContainer(), arow.size(), 0, r, m1ret);

http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/ed151101/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java b/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java
index f81ea44..7e6ec48 100644
--- a/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java
+++ b/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java
@@ -1512,7 +1512,7 @@ public class LibMatrixMult
 							if( !m2.sparseBlock.isEmpty(aix) ) {
 								ret.rlen=m;
 								ret.allocateSparseRowsBlock(false); //allocation on demand
-								ret.sparseBlock.set(i, new SparseRow(m2.sparseBlock.get(aix))); 
+								ret.sparseBlock.set(i, m2.sparseBlock.get(aix), true); 
 								ret.nonZeros += ret.sparseBlock.size(i);
 							}
 						}
@@ -2083,7 +2083,7 @@ public class LibMatrixMult
 				}
 		
 				//memcopy entire sparse row into target position
-				c.set(bpos, new SparseRow( b.get(i) ));
+				c.set(bpos, b.get(i), true);
 				lastblk = blk;
 			}
 		}

http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/ed151101/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 bdfe7c8..388a632 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
@@ -366,7 +366,7 @@ public class LibMatrixReorg
 				for( int i=0; i<rlen; i++ ) {
 					int ix = vix[i];
 					if( !in.sparseBlock.isEmpty(ix) ) {
-						out.sparseBlock.set(i, in.sparseBlock.get(ix));
+						out.sparseBlock.set(i, in.sparseBlock.get(ix), true);
 					}
 				}
 			}
@@ -1052,7 +1052,7 @@ public class LibMatrixReorg
 		//copy all rows into target positions
 		for( int i=0; i<m; i++ ) {
 			if( !a.isEmpty(i) ) {
-				c.set(m-1-i, a.get(i));	
+				c.set(m-1-i, a.get(i), true);	
 			}
 		}
 	}

http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/ed151101/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 9a7bfcb..23fdb85 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
@@ -792,30 +792,26 @@ public class MatrixBlock extends MatrixValue implements Externalizable
 		}
 	}
 	
-	public void appendRow(int r, SparseRow values)
+	/**
+	 * 
+	 * @param r
+	 * @param row
+	 */
+	public void appendRow(int r, SparseRow row)
 	{
-		if(values==null)
+		if(row == null)
 			return;
-		if(sparse)
-		{
+		
+		if(sparse) {
 			//allocation on demand
 			allocateSparseRowsBlock(false);
-			sparseBlock.allocate(r, values.size(), -1);
-			
-			//TODO perf sparse block
-			int[] cols=values.indexes();
-			double[] vals=values.values();
-			for(int i=0; i<values.size(); i++)
-				sparseBlock.append(r, cols[i], vals[i]);
-			
-			nonZeros+=values.size();
-			
+			sparseBlock.set(r, row, true);
+			nonZeros += row.size();
 		}
-		else
-		{
-			int[] cols=values.indexes();
-			double[] vals=values.values();
-			for(int i=0; i<values.size(); i++)
+		else {
+			int[] cols = row.indexes();
+			double[] vals = row.values();
+			for(int i=0; i<row.size(); i++)
 				quickSetValue(r, cols[i], vals[i]);
 		}
 	}
@@ -1358,7 +1354,7 @@ public class MatrixBlock extends MatrixValue implements Externalizable
 		for(int i=0; i<Math.min(that.sparseBlock.numRows(), rlen); i++)
 		{
 			if(!that.sparseBlock.isEmpty(i)) {
-				sparseBlock.set(i, new SparseRow(that.sparseBlock.get(i)));				
+				sparseBlock.set(i, that.sparseBlock.get(i), true);				
 			}
 			else if(!this.sparseBlock.isEmpty(i)) {
 				this.sparseBlock.reset(i,estimatedNNzsPerRow, clen);
@@ -1797,7 +1793,7 @@ public class MatrixBlock extends MatrixValue implements Externalizable
 				{
 					if( a.isEmpty(i) ) {
 						//copy entire sparse row (no sort required)
-						a.set(i, new SparseRow(b.get(i))); 
+						a.set(i, b.get(i), true); 
 					}
 					else
 					{

http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/ed151101/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlock.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlock.java b/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlock.java
index 02e5f73..10d25bd 100644
--- a/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlock.java
+++ b/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlock.java
@@ -288,9 +288,9 @@ public abstract class SparseBlock implements Serializable
 	 * 
 	 * @param r  row index starting at 0
 	 * @param row
-	 * @return
+	 * @param deep  indicator to create deep copy of sparse row
 	 */
-	public abstract void set(int r, SparseRow row);
+	public abstract void set(int r, SparseRow row, boolean deep);
 	
 	/**
 	 * Append a value to the end of the physical representation. This should 

http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/ed151101/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlockCOO.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlockCOO.java b/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlockCOO.java
index 7173946..19407e1 100644
--- a/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlockCOO.java
+++ b/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlockCOO.java
@@ -303,7 +303,7 @@ public class SparseBlockCOO extends SparseBlock
 	}
 	
 	@Override
-	public void set(int r, SparseRow row) {
+	public void set(int r, SparseRow row, boolean deep) {
 		int pos = pos(r);
 		int alen = row.size();
 		int[] aix = row.indexes();

http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/ed151101/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlockCSR.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlockCSR.java b/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlockCSR.java
index 7a447bd..fce7102 100644
--- a/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlockCSR.java
+++ b/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlockCSR.java
@@ -288,7 +288,7 @@ public class SparseBlockCSR extends SparseBlock
 	}
 
 	@Override
-	public void set(int r, SparseRow row) {
+	public void set(int r, SparseRow row, boolean deep) {
 		int pos = pos(r);
 		int len = size(r);		
 		int alen = row.size();

http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/ed151101/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlockMCSR.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlockMCSR.java b/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlockMCSR.java
index dfddf47..637f011 100644
--- a/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlockMCSR.java
+++ b/src/main/java/org/apache/sysml/runtime/matrix/data/SparseBlockMCSR.java
@@ -231,8 +231,8 @@ public class SparseBlockMCSR extends SparseBlock
 	}
 
 	@Override
-	public void set(int r, SparseRow row) {
-		_rows[r] = row;
+	public void set(int r, SparseRow row, boolean deep) {
+		_rows[r] = deep ? new SparseRow(row) : row;
 	}
 	
 	@Override