This is an automated email from the ASF dual-hosted git repository.
mboehm7 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/systemds.git
The following commit(s) were added to refs/heads/main by this push:
new 8d36f955b4 [MINOR] Cleanup core matrix block (remove unused/untested
code)
8d36f955b4 is described below
commit 8d36f955b4f9f771581a08ff3fe395ced63f7907
Author: Matthias Boehm <mboe...@gmail.com>
AuthorDate: Sat Jul 27 19:19:41 2024 +0200
[MINOR] Cleanup core matrix block (remove unused/untested code)
---
.../runtime/compress/CompressedMatrixBlock.java | 9 +-
.../spark/MatrixIndexingSPInstruction.java | 8 +-
.../sysds/runtime/matrix/data/CM_N_COVCell.java | 2 +-
.../sysds/runtime/matrix/data/LibMatrixReorg.java | 3 +-
.../sysds/runtime/matrix/data/MatrixBlock.java | 282 ++++-----------------
.../sysds/runtime/matrix/data/MatrixCell.java | 2 +-
.../sysds/runtime/matrix/data/MatrixValue.java | 2 +-
.../builtin/part2/BuiltinRaGroupbyTest.java | 1 -
8 files changed, 61 insertions(+), 248 deletions(-)
diff --git
a/src/main/java/org/apache/sysds/runtime/compress/CompressedMatrixBlock.java
b/src/main/java/org/apache/sysds/runtime/compress/CompressedMatrixBlock.java
index 79951b4b4b..c00f52171c 100644
--- a/src/main/java/org/apache/sysds/runtime/compress/CompressedMatrixBlock.java
+++ b/src/main/java/org/apache/sysds/runtime/compress/CompressedMatrixBlock.java
@@ -764,10 +764,10 @@ public class CompressedMatrixBlock extends MatrixBlock {
}
@Override
- public MatrixBlock zeroOutOperations(MatrixValue result, IndexRange
range, boolean complementary) {
+ public MatrixBlock zeroOutOperations(MatrixValue result, IndexRange
range) {
printDecompressWarning("zeroOutOperations");
MatrixBlock tmp = getUncompressed();
- return tmp.zeroOutOperations(result, range, complementary);
+ return tmp.zeroOutOperations(result, range);
}
@Override
@@ -1212,11 +1212,6 @@ public class CompressedMatrixBlock extends MatrixBlock {
return false;
}
- @Override
- public void checkNaN() {
- throw new NotImplementedException();
- }
-
@Override
public void init(double[][] arr, int r, int c) {
throw new DMLCompressionException("Invalid to init on a
compressed MatrixBlock");
diff --git
a/src/main/java/org/apache/sysds/runtime/instructions/spark/MatrixIndexingSPInstruction.java
b/src/main/java/org/apache/sysds/runtime/instructions/spark/MatrixIndexingSPInstruction.java
index e97336a8a6..e3201051ca 100644
---
a/src/main/java/org/apache/sysds/runtime/instructions/spark/MatrixIndexingSPInstruction.java
+++
b/src/main/java/org/apache/sysds/runtime/instructions/spark/MatrixIndexingSPInstruction.java
@@ -158,7 +158,7 @@ public class MatrixIndexingSPInstruction extends
IndexingSPInstruction {
}
else { //general case
// zero-out lhs
- in1 = in1.mapToPair(new ZeroOutLHS(false,
ixrange, mcLeft));
+ in1 = in1.mapToPair(new ZeroOutLHS(ixrange,
mcLeft));
// slice rhs, shift and merge with lhs
in2 =
sec.getBinaryMatrixBlockRDDHandleForVariable( input2.getName() )
@@ -341,12 +341,10 @@ public class MatrixIndexingSPInstruction extends
IndexingSPInstruction {
{
private static final long serialVersionUID =
-3581795160948484261L;
- private boolean _complement = false;
private IndexRange _ixrange = null;
private int _blen = -1;
- public ZeroOutLHS(boolean complement, IndexRange range,
DataCharacteristics mcLeft) {
- _complement = complement;
+ public ZeroOutLHS(IndexRange range, DataCharacteristics mcLeft)
{
_ixrange = range;
_blen = mcLeft.getBlocksize();
_blen = mcLeft.getBlocksize();
@@ -365,7 +363,7 @@ public class MatrixIndexingSPInstruction extends
IndexingSPInstruction {
throw new Exception("Error while getting range
for zero-out");
}
- MatrixBlock zeroBlk = kv._2.zeroOutOperations(new
MatrixBlock(), range, _complement);
+ MatrixBlock zeroBlk = kv._2.zeroOutOperations(new
MatrixBlock(), range);
return new Tuple2<>(kv._1, zeroBlk);
}
}
diff --git
a/src/main/java/org/apache/sysds/runtime/matrix/data/CM_N_COVCell.java
b/src/main/java/org/apache/sysds/runtime/matrix/data/CM_N_COVCell.java
index aef6e78260..a77ca3349d 100644
--- a/src/main/java/org/apache/sysds/runtime/matrix/data/CM_N_COVCell.java
+++ b/src/main/java/org/apache/sysds/runtime/matrix/data/CM_N_COVCell.java
@@ -213,7 +213,7 @@ public class CM_N_COVCell extends MatrixValue
}
@Override
- public MatrixValue zeroOutOperations(MatrixValue result, IndexRange
range, boolean complementary) {
+ public MatrixValue zeroOutOperations(MatrixValue result, IndexRange
range) {
throw new DMLRuntimeException("operation not supported for
CM_N_COVCell");
}
diff --git
a/src/main/java/org/apache/sysds/runtime/matrix/data/LibMatrixReorg.java
b/src/main/java/org/apache/sysds/runtime/matrix/data/LibMatrixReorg.java
index fba185a5c4..1ebd73da4d 100644
--- a/src/main/java/org/apache/sysds/runtime/matrix/data/LibMatrixReorg.java
+++ b/src/main/java/org/apache/sysds/runtime/matrix/data/LibMatrixReorg.java
@@ -107,8 +107,7 @@ public class LibMatrixReorg {
// public interface //
/////////////////////////
- public static boolean isSupportedReorgOperator( ReorgOperator op )
- {
+ public static boolean isSupportedReorgOperator( ReorgOperator op ) {
return (getReorgType(op) != ReorgType.INVALID);
}
diff --git
a/src/main/java/org/apache/sysds/runtime/matrix/data/MatrixBlock.java
b/src/main/java/org/apache/sysds/runtime/matrix/data/MatrixBlock.java
index 054edf06a2..7c8efec94e 100644
--- a/src/main/java/org/apache/sysds/runtime/matrix/data/MatrixBlock.java
+++ b/src/main/java/org/apache/sysds/runtime/matrix/data/MatrixBlock.java
@@ -3280,53 +3280,10 @@ public class MatrixBlock extends MatrixValue implements
CacheBlock<MatrixBlock>,
}
else if(aggOp.correction==CorrectionLocationType.NONE) {
//e.g., ak+ kahan plus as used in sum, mapmult, mmcj
and tsmm
- if(aggOp.increOp.fn instanceof KahanPlus) {
- LibMatrixAgg.aggregateBinaryMatrix(newWithCor,
this, cor, deep);
- }
- else
- {
- if( newWithCor.isInSparseFormat() &&
aggOp.sparseSafe ) //SPARSE
- {
- SparseBlock b =
newWithCor.getSparseBlock();
- if( b==null ) //early abort on empty
block
- return;
- for( int r=0; r<Math.min(rlen,
b.numRows()); r++ )
- {
- if( !b.isEmpty(r) )
- {
- int bpos = b.pos(r);
- int blen = b.size(r);
- int[] bix =
b.indexes(r);
- double[] bvals =
b.values(r);
- for( int j=bpos;
j<bpos+blen; j++)
- {
- int c = bix[j];
- buffer._sum =
this.get(r, c);
-
buffer._correction = cor.get(r, c);
- buffer =
(KahanObject) aggOp.increOp.fn.execute(buffer, bvals[j]);
- set(r, c,
buffer._sum);
- cor.set(r, c,
buffer._correction);
- }
- }
- }
-
- }
- else //DENSE or SPARSE (!sparsesafe)
- {
- for(int r=0; r<rlen; r++)
- for(int c=0; c<clen; c++) {
- buffer._sum=this.get(r,
c);
-
buffer._correction=cor.get(r, c);
- buffer=(KahanObject)
aggOp.increOp.fn.execute(buffer, newWithCor.get(r, c));
- set(r, c, buffer._sum);
- cor.set(r, c,
buffer._correction);
- }
- }
+ if(!(aggOp.increOp.fn instanceof KahanPlus))
+ throw new DMLRuntimeException("Unsupported
incremental aggregation: "+aggOp.increOp.fn);
- //change representation if required
- //(note since ak+ on blocks is currently only
applied in MR, hence no need to account for this in mem estimates)
- examSparsity();
- }
+ LibMatrixAgg.aggregateBinaryMatrix(newWithCor, this,
cor, deep);
}
else if(aggOp.correction==CorrectionLocationType.LASTTWOROWS) {
double n, n2, mu2;
@@ -3452,23 +3409,10 @@ public class MatrixBlock extends MatrixValue implements
CacheBlock<MatrixBlock>,
if(aggOp.correction==CorrectionLocationType.LASTROW)
{
- if( aggOp.increOp.fn instanceof KahanPlus )
- {
- LibMatrixAgg.aggregateBinaryMatrix(newWithCor,
this, aggOp);
- }
- else
- {
- for(int r=0; r<rlen-1; r++)
- for(int c=0; c<clen; c++)
- {
- buffer._sum=this.get(r, c);
-
buffer._correction=this.get(r+1, c);
- buffer=(KahanObject)
aggOp.increOp.fn.execute(buffer, newWithCor.get(r, c),
-
newWithCor.get(r+1, c));
- set(r, c, buffer._sum);
- set(r+1, c, buffer._correction);
- }
- }
+ if( !(aggOp.increOp.fn instanceof KahanPlus) )
+ throw new DMLRuntimeException("Unsupported
incremental aggregation: "+aggOp.increOp.fn);
+
+ LibMatrixAgg.aggregateBinaryMatrix(newWithCor, this,
aggOp);
}
else if(aggOp.correction==CorrectionLocationType.LASTCOLUMN)
{
@@ -3511,20 +3455,10 @@ public class MatrixBlock extends MatrixValue implements
CacheBlock<MatrixBlock>,
}
else
{
- if(aggOp.increOp.fn instanceof KahanPlus) {
-
LibMatrixAgg.aggregateBinaryMatrix(newWithCor, this, aggOp);
- }
- else {
- for(int r=0; r<rlen; r++)
- for(int c=0; c<clen-1; c++)
- {
- buffer._sum=this.get(r,
c);
-
buffer._correction=this.get(r, c+1);
- buffer=(KahanObject)
aggOp.increOp.fn.execute(buffer, newWithCor.get(r, c), newWithCor.get(r, c+1));
- set(r, c, buffer._sum);
- set(r, c+1,
buffer._correction);
- }
- }
+ if(!(aggOp.increOp.fn instanceof KahanPlus))
+ throw new
DMLRuntimeException("Unsupported incremental aggregation: "+aggOp.increOp.fn);
+
+ LibMatrixAgg.aggregateBinaryMatrix(newWithCor,
this, aggOp);
}
}
else if(aggOp.correction==CorrectionLocationType.LASTTWOROWS)
@@ -3667,62 +3601,12 @@ public class MatrixBlock extends MatrixValue implements
CacheBlock<MatrixBlock>,
else
result.reset(tempCellIndex.row, tempCellIndex.column,
sps, nonZeros);
- if( LibMatrixReorg.isSupportedReorgOperator(op) )
- {
- //SPECIAL case (operators with special performance
requirements,
- //or size-dependent special behavior)
- //currently supported opcodes: r', rdiag, rsort, rev
- LibMatrixReorg.reorg(this, result, op);
- }
- else
- {
- //GENERIC case (any reorg operator)
- CellIndex temp = new CellIndex(0, 0);
- if(sparse && sparseBlock != null) {
- for(int r=0; r<Math.min(rlen,
sparseBlock.numRows()); r++) {
- if(sparseBlock.isEmpty(r)) continue;
- int apos = sparseBlock.pos(r);
- int alen = sparseBlock.size(r);
- int[] aix = sparseBlock.indexes(r);
- double[] avals = sparseBlock.values(r);
- for(int i=apos; i<apos+alen; i++) {
- tempCellIndex.set(r, aix[i]);
- op.fn.execute(tempCellIndex,
temp);
- result.appendValue(temp.row,
temp.column, avals[i]);
- }
- }
- }
- else if( !sparse && denseBlock != null ) {
- if( result.isInSparseFormat() ) {
//SPARSE<-DENSE
- DenseBlock a = getDenseBlock();
- for( int i=0; i<rlen; i++ ) {
- double[] avals = a.values(i);
- int aix = a.pos(i);
- for( int j=0; j<clen; j++ ) {
- temp.set(i, j);
- op.fn.execute(temp,
temp);
-
result.appendValue(temp.row, temp.column, avals[aix+j]);
- }
- }
- }
- else { //DENSE<-DENSE
- result.allocateDenseBlock();
- DenseBlock a = getDenseBlock();
- DenseBlock c = result.getDenseBlock();
- for( int i=0; i<rlen; i++ ) {
- double[] avals = a.values(i);
- int aix = a.pos(i);
- for( int j=0; j<clen; j++ ) {
- temp.set(i, j);
- op.fn.execute(temp,
temp);
- c.set(temp.row,
temp.column, avals[aix+j]);
- }
- }
- result.nonZeros = nonZeros;
- }
- }
- }
+ if( !LibMatrixReorg.isSupportedReorgOperator(op) )
+ throw new DMLRuntimeException("Unsupported reorg
operation: "+op);
+ // core reorg runtime operations
+ LibMatrixReorg.reorg(this, result, op);
+
return result;
}
@@ -4600,13 +4484,12 @@ public class MatrixBlock extends MatrixValue implements
CacheBlock<MatrixBlock>,
}
@Override
- public MatrixBlock zeroOutOperations(MatrixValue result, IndexRange
range, boolean complementary) {
+ public MatrixBlock zeroOutOperations(MatrixValue result, IndexRange
range) {
checkType(result);
double currentSparsity=(double)nonZeros/rlen/clen;
double
estimatedSps=currentSparsity*(range.rowEnd-range.rowStart+1)
*(range.colEnd-range.colStart+1)/rlen/clen;
- if(!complementary)
- estimatedSps=currentSparsity-estimatedSps;
+ estimatedSps = currentSparsity-estimatedSps;
boolean lsparse = evalSparseFormatInMemory(rlen, clen,
(long)(estimatedSps*rlen*clen));
@@ -4620,13 +4503,11 @@ public class MatrixBlock extends MatrixValue implements
CacheBlock<MatrixBlock>,
{
if(sparseBlock!=null)
{
- if(!complementary)//if zero out
- {
- for(int r=0;
r<Math.min((int)range.rowStart, sparseBlock.numRows()); r++)
- ((MatrixBlock)
result).appendRow(r, sparseBlock.get(r));
- for(int r=Math.min((int)range.rowEnd+1,
sparseBlock.numRows()); r<Math.min(rlen, sparseBlock.numRows()); r++)
- ((MatrixBlock)
result).appendRow(r, sparseBlock.get(r));
- }
+ for(int r=0; r<Math.min((int)range.rowStart,
sparseBlock.numRows()); r++)
+ ((MatrixBlock) result).appendRow(r,
sparseBlock.get(r));
+ for(int r=Math.min((int)range.rowEnd+1,
sparseBlock.numRows()); r<Math.min(rlen, sparseBlock.numRows()); r++)
+ ((MatrixBlock) result).appendRow(r,
sparseBlock.get(r));
+
for(int r=(int)range.rowStart;
r<=Math.min(range.rowEnd, sparseBlock.numRows()-1); r++)
{
if(sparseBlock.isEmpty(r))
@@ -4634,35 +4515,21 @@ public class MatrixBlock extends MatrixValue implements
CacheBlock<MatrixBlock>,
int[] cols=sparseBlock.indexes(r);
double[] values=sparseBlock.values(r);
- if(complementary)//if selection
+
+ int pos = sparseBlock.pos(r);
+ int len = sparseBlock.size(r);
+ int
start=sparseBlock.posFIndexGTE(r,(int)range.colStart);
+ if(start<0) start=pos+len;
+ int
end=sparseBlock.posFIndexGT(r,(int)range.colEnd);
+ if(end<0) end=pos+len;
+
+ for(int i=pos; i<start; i++)
{
- int
start=sparseBlock.posFIndexGTE(r,(int)range.colStart);
- if(start<0) continue;
- int
end=sparseBlock.posFIndexGT(r,(int)range.colEnd);
- if(end<0 || start>end)
- continue;
-
- for(int i=start; i<end; i++)
- {
- ((MatrixBlock)
result).appendValue(r, cols[i], values[i]);
- }
- }else
+ ((MatrixBlock)
result).appendValue(r, cols[i], values[i]);
+ }
+ for(int i=end; i<pos+len; i++)
{
- int pos = sparseBlock.pos(r);
- int len = sparseBlock.size(r);
- int
start=sparseBlock.posFIndexGTE(r,(int)range.colStart);
- if(start<0) start=pos+len;
- int
end=sparseBlock.posFIndexGT(r,(int)range.colEnd);
- if(end<0) end=pos+len;
-
- for(int i=pos; i<start; i++)
- {
- ((MatrixBlock)
result).appendValue(r, cols[i], values[i]);
- }
- for(int i=end; i<pos+len; i++)
- {
- ((MatrixBlock)
result).appendValue(r, cols[i], values[i]);
- }
+ ((MatrixBlock)
result).appendValue(r, cols[i], values[i]);
}
}
}
@@ -4671,37 +4538,25 @@ public class MatrixBlock extends MatrixValue implements
CacheBlock<MatrixBlock>,
if(denseBlock!=null)
{
double[] a = getDenseBlockValues();
- if(complementary)//if selection
- {
- int offset=((int)range.rowStart)*clen;
- for(int r=(int) range.rowStart;
r<=range.rowEnd; r++)
- {
- for(int c=(int) range.colStart;
c<=range.colEnd; c++)
- ((MatrixBlock)
result).appendValue(r, c, a[offset+c]);
- offset+=clen;
- }
- }else
+
+ int offset=0;
+ int r=0;
+ for(; r<(int)range.rowStart; r++)
+ for(int c=0; c<clen; c++, offset++)
+ ((MatrixBlock)
result).appendValue(r, c, a[offset]);
+
+ for(; r<=(int)range.rowEnd; r++)
{
- int offset=0;
- int r=0;
- for(; r<(int)range.rowStart; r++)
- for(int c=0; c<clen; c++,
offset++)
- ((MatrixBlock)
result).appendValue(r, c, a[offset]);
-
- for(; r<=(int)range.rowEnd; r++)
- {
- for(int c=0;
c<(int)range.colStart; c++)
- ((MatrixBlock)
result).appendValue(r, c, a[offset+c]);
- for(int c=(int)range.colEnd+1;
c<clen; c++)
- ((MatrixBlock)
result).appendValue(r, c, a[offset+c]);
- offset+=clen;
- }
-
- for(; r<rlen; r++)
- for(int c=0; c<clen; c++,
offset++)
- ((MatrixBlock)
result).appendValue(r, c, a[offset]);
+ for(int c=0; c<(int)range.colStart; c++)
+ ((MatrixBlock)
result).appendValue(r, c, a[offset+c]);
+ for(int c=(int)range.colEnd+1; c<clen;
c++)
+ ((MatrixBlock)
result).appendValue(r, c, a[offset+c]);
+ offset+=clen;
}
+ for(; r<rlen; r++)
+ for(int c=0; c<clen; c++, offset++)
+ ((MatrixBlock)
result).appendValue(r, c, a[offset]);
}
}
return (MatrixBlock)result;
@@ -5793,39 +5648,6 @@ public class MatrixBlock extends MatrixValue implements
CacheBlock<MatrixBlock>,
*/
public static boolean isThreadSafe(boolean sparse) {
return !sparse || DEFAULT_SPARSEBLOCK == SparseBlock.Type.MCSR;
//only MCSR thread-safe
- }
-
- /**
- * Checks for existing NaN values in the matrix block.
- * @throws DMLRuntimeException if the blocks contains at least one NaN.
- */
- public void checkNaN() {
- if( isEmptyBlock(false) )
- return;
- if( sparse ) {
- SparseBlock sblock = sparseBlock;
- for(int i=0; i<rlen; i++) {
- if( sblock.isEmpty(i) ) continue;
- int alen = sblock.size(i);
- int apos = sblock.pos(i);
- int[] aix = sblock.indexes(i);
- double[] avals = sblock.values(i);
- for(int k=apos; k<apos+alen; k++) {
- if( Double.isNaN(avals[k]) )
- throw new
DMLRuntimeException("NaN encountered at position ["+i+","+aix[k]+"].");
- }
- }
- }
- else {
- DenseBlock dblock = denseBlock;
- for(int i=0; i<rlen; i++) {
- int aix = dblock.pos(i);
- double[] avals = dblock.values(i);
- for(int j=0; j<clen; j++)
- if( Double.isNaN(avals[aix+j]) )
- throw new
DMLRuntimeException("NaN encountered at position ["+i+","+j+"].");
- }
- }
}
@Override
diff --git a/src/main/java/org/apache/sysds/runtime/matrix/data/MatrixCell.java
b/src/main/java/org/apache/sysds/runtime/matrix/data/MatrixCell.java
index f892766796..4782dba879 100644
--- a/src/main/java/org/apache/sysds/runtime/matrix/data/MatrixCell.java
+++ b/src/main/java/org/apache/sysds/runtime/matrix/data/MatrixCell.java
@@ -249,7 +249,7 @@ public class MatrixCell extends MatrixValue implements
Serializable
}
@Override
- public MatrixValue zeroOutOperations(MatrixValue result, IndexRange
range, boolean complementary) {
+ public MatrixValue zeroOutOperations(MatrixValue result, IndexRange
range) {
if(range.rowStart!=0 || range.rowEnd!=0 || range.colStart!=0 ||
range.colEnd!=0)
throw new DMLRuntimeException("wrong range: "+range+"
for matrixCell");
MatrixCell c3=checkType(result);
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 fc8ffd2728..a1a4254653 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
@@ -147,7 +147,7 @@ public abstract class MatrixValue implements
WritableComparable
public abstract void incrementalAggregate(AggregateOperator aggOp,
MatrixValue newWithCorrection);
- public abstract MatrixValue zeroOutOperations(MatrixValue result,
IndexRange range, boolean complementary);
+ public abstract MatrixValue zeroOutOperations(MatrixValue result,
IndexRange range);
/**
* Slice out up to 4 matrixBlocks that are separated by the row and col
Cuts.
diff --git
a/src/test/java/org/apache/sysds/test/functions/builtin/part2/BuiltinRaGroupbyTest.java
b/src/test/java/org/apache/sysds/test/functions/builtin/part2/BuiltinRaGroupbyTest.java
index 770f8c29e4..18ad933e77 100644
---
a/src/test/java/org/apache/sysds/test/functions/builtin/part2/BuiltinRaGroupbyTest.java
+++
b/src/test/java/org/apache/sysds/test/functions/builtin/part2/BuiltinRaGroupbyTest.java
@@ -26,7 +26,6 @@ import org.apache.sysds.test.TestConfiguration;
import org.apache.sysds.test.TestUtils;
import org.junit.Test;
-import java.util.Arrays;
import java.util.HashMap;
public class BuiltinRaGroupbyTest extends AutomatedTestBase
