[SYSTEMML-2038] Fix sparse tsmm, rowIndexMax, cumsum over CSR inputs The recent change to use the memory-efficient CSR format for all dense-sparse conversions revealed a couple of hidden correctness issues with sparse tsmm (transpose-self-matrix multiply), rowIndexMax, and cumsum block operations over CSR blocks.
Project: http://git-wip-us.apache.org/repos/asf/systemml/repo Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/fdc24bb7 Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/fdc24bb7 Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/fdc24bb7 Branch: refs/heads/master Commit: fdc24bb7d46858f2300efd7bee302c3ce6a6365d Parents: 5d4ad5b Author: Matthias Boehm <[email protected]> Authored: Thu Dec 7 02:05:58 2017 -0800 Committer: Matthias Boehm <[email protected]> Committed: Thu Dec 7 13:40:37 2017 -0800 ---------------------------------------------------------------------- .../gpu/BuiltinUnaryGPUInstruction.java | 19 +---------- .../sysml/runtime/matrix/data/LibMatrixAgg.java | 2 +- .../runtime/matrix/data/LibMatrixMult.java | 34 +++++++++----------- .../sysml/runtime/matrix/data/MatrixBlock.java | 3 +- .../runtime/matrix/data/SparseBlockCSR.java | 4 +-- 5 files changed, 22 insertions(+), 40 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/systemml/blob/fdc24bb7/src/main/java/org/apache/sysml/runtime/instructions/gpu/BuiltinUnaryGPUInstruction.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/runtime/instructions/gpu/BuiltinUnaryGPUInstruction.java b/src/main/java/org/apache/sysml/runtime/instructions/gpu/BuiltinUnaryGPUInstruction.java index cd78164..93a8f35 100644 --- a/src/main/java/org/apache/sysml/runtime/instructions/gpu/BuiltinUnaryGPUInstruction.java +++ b/src/main/java/org/apache/sysml/runtime/instructions/gpu/BuiltinUnaryGPUInstruction.java @@ -23,12 +23,9 @@ package org.apache.sysml.runtime.instructions.gpu; import org.apache.sysml.parser.Expression.DataType; import org.apache.sysml.parser.Expression.ValueType; import org.apache.sysml.runtime.DMLRuntimeException; -import org.apache.sysml.runtime.functionobjects.Builtin; -import org.apache.sysml.runtime.functionobjects.ValueFunction; import org.apache.sysml.runtime.instructions.InstructionUtils; import org.apache.sysml.runtime.instructions.cp.CPOperand; import org.apache.sysml.runtime.matrix.operators.Operator; -import org.apache.sysml.runtime.matrix.operators.UnaryOperator; public abstract class BuiltinUnaryGPUInstruction extends GPUInstruction { int _arity; @@ -56,21 +53,10 @@ public abstract class BuiltinUnaryGPUInstruction extends GPUInstruction { String[] parts = InstructionUtils.getInstructionPartsWithValueType(str); String opcode = null; - ValueFunction func = null; //print or stop or cumulative aggregates - if( parts.length==4 ) - { - opcode = parts[0]; - in.split(parts[1]); - out.split(parts[2]); - func = Builtin.getBuiltinFnObject(opcode); - + if( parts.length==4 ) { throw new DMLRuntimeException("The instruction is not supported on GPU:" + str); -// if( Arrays.asList(new String[]{"ucumk+","ucum*","ucummin","ucummax"}).contains(opcode) ) -// return new MatrixBuiltinCPInstruction(new UnaryOperator(func,Integer.parseInt(parts[3])), in, out, opcode, str); -// else -// return new ScalarBuiltinCPInstruction(new SimpleOperator(func), in, out, opcode, str); } else //2+1, general case { @@ -78,12 +64,9 @@ public abstract class BuiltinUnaryGPUInstruction extends GPUInstruction { opcode = parts[0]; in.split(parts[1]); out.split(parts[2]); - // func = Builtin.getBuiltinFnObject(opcode); - // new UnaryOperator(func) if(in.getDataType() == DataType.SCALAR) throw new DMLRuntimeException("The instruction is not supported on GPU:" + str); -// return new ScalarBuiltinCPInstruction(new SimpleOperator(func), in, out, opcode, str); else if(in.getDataType() == DataType.MATRIX) return new MatrixBuiltinGPUInstruction(null, in, out, opcode, str); } http://git-wip-us.apache.org/repos/asf/systemml/blob/fdc24bb7/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixAgg.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixAgg.java b/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixAgg.java index f1ed00d..ae5e768 100644 --- a/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixAgg.java +++ b/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixAgg.java @@ -2634,7 +2634,7 @@ public class LibMatrixAgg if(alen < n && (builtin.execute2( 0, c[cix+1] ) == 1)) { int ix = n-1; //find last 0 value - for( int j=alen-1; j>=0; j--, ix-- ) + for( int j=apos+alen-1; j>=apos; j--, ix-- ) if( aix[j]!=ix ) break; c[cix+0] = ix + 1; //max index (last) http://git-wip-us.apache.org/repos/asf/systemml/blob/fdc24bb7/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 9421bdb..e2af752 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 @@ -1807,7 +1807,7 @@ public class LibMatrixMult throws DMLRuntimeException { //2) transpose self matrix multiply sparse - // (compute only upper-triangular matrix due to symmetry) + // (compute only upper-triangular matrix due to symmetry) SparseBlock a = m1.sparseBlock; double[] c = ret.denseBlock; int m = m1.rlen; @@ -1820,25 +1820,23 @@ public class LibMatrixMult if( LOW_LEVEL_OPTIMIZATION ) { int arlen = a.numRows(); - for( int r=0; r<arlen; r++ ) - if( !a.isEmpty(r) ) - { - int apos = a.pos(r); - int alen = a.size(r); - int[] aix = a.indexes(r); - double[] avals = a.values(r); - int rlix = (rl==0) ? 0 : a.posFIndexGTE(r, rl); - rlix = (rlix>=0) ? apos+rlix : apos+alen; - - for(int i = rlix; i < apos+alen && aix[i]<ru; i++) - { - double val = avals[i]; - if( val != 0 ) { - int ix2 = aix[i]*n; - vectMultiplyAdd(val, avals, c, aix, i, ix2, alen-i); - } + for( int r=0; r<arlen; r++ ) { + if( a.isEmpty(r) ) continue; + int apos = a.pos(r); + int alen = a.size(r); + int[] aix = a.indexes(r); + double[] avals = a.values(r); + int rlix = (rl==0) ? 0 : a.posFIndexGTE(r, rl); + rlix = (rlix>=0) ? apos+rlix : apos+alen; + int len = apos + alen; + for(int i = rlix; i < len && aix[i]<ru; i++) { + double val = avals[i]; + if( val != 0 ) { + int ix2 = aix[i]*n; + vectMultiplyAdd(val, avals, c, aix, i, ix2, len-i); } } + } } else { http://git-wip-us.apache.org/repos/asf/systemml/blob/fdc24bb7/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 2f69e5d..4e82891 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 @@ -29,6 +29,7 @@ import java.io.ObjectOutput; import java.io.ObjectOutputStream; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; import java.util.stream.LongStream; @@ -553,7 +554,7 @@ public class MatrixBlock extends MatrixValue implements CacheBlock, Externalizab //check for existing sparse block: return empty list if( sparseBlock==null ) - return new ArrayList<IJV>().iterator(); + return Collections.EMPTY_LIST.iterator(); //get iterator over sparse block return sparseBlock.getIterator(rl, ru); http://git-wip-us.apache.org/repos/asf/systemml/blob/fdc24bb7/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 228a806..c3496ec 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 @@ -456,14 +456,14 @@ public class SparseBlockCSR extends SparseBlock @Override public void set(int r, SparseRow row, boolean deep) { int pos = pos(r); - int len = size(r); + int len = size(r); int alen = row.size(); int[] aix = row.indexes(); double[] avals = row.values(); //delete existing values if necessary if( len > 0 ) //incl size update - deleteIndexRange(r, aix[0], aix[alen-1]+1); + deleteIndexRange(r, aix[pos], aix[pos+len-1]+1); //prepare free space (allocate and shift) int lsize = _size+alen;
