Skip to content

Commit

Permalink
[SYSTEMDS-3763] BinCell Compressed TestCover
Browse files Browse the repository at this point in the history
This commit improves the test coverage of the
compressed binary cell operation in this process
it unearthed a few edge cases that produced wrong
results in sparse inputs, if performing
overlapping constant minus vector, and if
using compressed left matrix binary op with empty
rows.

While making the PR I also include 100% coverage
of our ternary operator.

Overall this commit improve the code coverage by
.1 % in the project.

Closes #2083
  • Loading branch information
Baunsgaard committed Sep 8, 2024
1 parent b3517cb commit f920c59
Show file tree
Hide file tree
Showing 35 changed files with 1,865 additions and 404 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/javaTests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ jobs:
tests: [
"org.apache.sysds.test.applications.**",
"**.test.usertest.**",
"**.component.c**.**",
"**.component.c**.** -Dtest-threadCount=1 -Dtest-forkCount=1",
"**.component.e**.**,**.component.f**.**,**.component.m**.**",
"**.component.p**.**,**.component.r**.**,**.component.s**.**,**.component.t**.**,**.component.u**.**",
"**.functions.a**.**,**.functions.binary.matrix.**,**.functions.binary.scalar.**,**.functions.binary.tensor.**",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -479,14 +479,12 @@ public MatrixBlock scalarOperations(ScalarOperator sop, MatrixValue result) {
@Override
public MatrixBlock binaryOperations(BinaryOperator op, MatrixValue thatValue, MatrixValue result) {
MatrixBlock that = thatValue == null ? null : (MatrixBlock) thatValue;
MatrixBlock ret = result == null ? null : (MatrixBlock) result;
return CLALibBinaryCellOp.binaryOperationsRight(op, this, that, ret);
return CLALibBinaryCellOp.binaryOperationsRight(op, this, that);
}

public MatrixBlock binaryOperationsLeft(BinaryOperator op, MatrixValue thatValue, MatrixValue result) {
MatrixBlock that = thatValue == null ? null : (MatrixBlock) thatValue;
MatrixBlock ret = result == null ? null : (MatrixBlock) result;
return CLALibBinaryCellOp.binaryOperationsLeft(op, this, that, ret);
return CLALibBinaryCellOp.binaryOperationsLeft(op, this, that);
}

@Override
Expand Down Expand Up @@ -885,7 +883,7 @@ public void ctableOperations(Operator op, MatrixValue that, MatrixValue that2, C

@Override
public MatrixBlock ternaryOperations(TernaryOperator op, MatrixBlock m2, MatrixBlock m3, MatrixBlock ret) {
return CLALibTernaryOp.ternaryOperations(this, op, m2, m3, ret);
return CLALibTernaryOp.ternaryOperations(op, this, m2, m3);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import org.apache.sysds.runtime.functionobjects.Plus;
import org.apache.sysds.runtime.matrix.data.MatrixBlock;
import org.apache.sysds.runtime.matrix.operators.BinaryOperator;
import org.apache.sysds.runtime.matrix.operators.RightScalarOperator;
import org.apache.sysds.runtime.matrix.operators.ScalarOperator;
import org.apache.sysds.runtime.matrix.operators.UnaryOperator;

Expand Down Expand Up @@ -478,7 +479,11 @@ public AColGroup binaryRowOpRight(BinaryOperator op, double[] v, boolean isRowSa
final double[] reference = ColGroupUtils.binaryDefRowRight(op, v, _colIndexes);
return ColGroupDDCFOR.create(_colIndexes, _dict, _data, getCachedCounts(), reference);
}
final IDictionary ret = _dict.binOpRight(op, v, _colIndexes);
final IDictionary ret;
if(_colIndexes.size() == 1)
ret = _dict.applyScalarOp(new RightScalarOperator(op.fn, v[_colIndexes.get(0)]));
else
ret = _dict.binOpRight(op, v, _colIndexes);
return create(_colIndexes, ret, _data, getCachedCounts());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ public Dictionary binOpRight(BinaryOperator op, double[] v, IColIndex colIndexes
final int lenV = colIndexes.size();
for(int i = 0; i < len; i++)
retVals[i] = fn.execute(_values[i], v[colIndexes.get(i % lenV)]);

return create(retVals);
}

Expand Down
Loading

0 comments on commit f920c59

Please sign in to comment.