Skip to content

Commit cebf845

Browse files
mboehm7mboehm7
committed
[MINOR] Misc code cleanups for improve test code coverage
1 parent 1590d17 commit cebf845

28 files changed

+142
-859
lines changed

‎src/main/java/org/apache/sysds/hops/AggBinaryOp.java‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,6 @@ public AggBinaryOp(String l, DataType dt, ValueType vt, OpOp2 innOp,
111111
refreshSizeInformation();
112112
}
113113

114-
@Override
115-
public void checkArity() {
116-
HopsException.check(_input.size() == 2, this, "should have arity 2 but has arity %d", _input.size());
117-
}
118-
119114
public void setHasLeftPMInput(boolean flag) {
120115
_hasLeftPMInput = flag;
121116
}
@@ -387,34 +382,32 @@ protected ExecType optFindExecType(boolean transitive)
387382
{
388383
checkAndSetForcedPlatform();
389384

390-
if( _etypeForced != null )
391-
{
392-
_etype = _etypeForced;
385+
if( _etypeForced != null ) {
386+
setExecType(_etypeForced);
393387
}
394388
else
395389
{
396-
if ( OptimizerUtils.isMemoryBasedOptLevel() )
397-
{
398-
_etype = findExecTypeByMemEstimate();
390+
if ( OptimizerUtils.isMemoryBasedOptLevel() ) {
391+
setExecType(findExecTypeByMemEstimate());
399392
}
400393
// choose CP if the dimensions of both inputs are below Hops.CPThreshold
401394
// OR if it is vector-vector inner product
402395
else if ( (getInput().get(0).areDimsBelowThreshold() && getInput().get(1).areDimsBelowThreshold())
403396
|| (getInput().get(0).isVector() && getInput().get(1).isVector() && !isOuterProduct()) )
404397
{
405-
_etype = ExecType.CP;
398+
setExecType(ExecType.CP);
406399
}
407400
else
408401
{
409-
_etype = ExecType.SPARK;
402+
setExecType(ExecType.SPARK);
410403
}
411404

412405
//check for valid CP mmchain, send invalid memory requirements to remote
413406
if( _etype == ExecType.CP
414407
&& checkMapMultChain() != ChainType.NONE
415408
&& OptimizerUtils.getLocalMemBudget() <
416409
getInput().get(0).getInput().get(0).getOutputMemEstimate() ) {
417-
_etype = ExecType.SPARK;
410+
setExecType(ExecType.SPARK);
418411
}
419412

420413
//check for valid CP dimensions and matrix size
@@ -429,7 +422,7 @@ && checkMapMultChain() != ChainType.NONE
429422
|| ( !mmtsj.isRight() && isApplicableForTransitiveSparkExecType(false))) )
430423
{
431424
//pull binary aggregate into spark
432-
_etype = ExecType.SPARK;
425+
setExecType(ExecType.SPARK);
433426
}
434427

435428
//mark for recompile (forever)

‎src/main/java/org/apache/sysds/hops/AggUnaryOp.java‎

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -63,28 +63,19 @@ public AggUnaryOp(String l, DataType dt, ValueType vt, AggOp o, Direction idx, H
6363
inp.getParent().add(this);
6464
}
6565

66-
@Override
67-
public void checkArity() {
68-
HopsException.check(_input.size() == 1, this, "should have arity 1 but has arity %d", _input.size());
69-
}
70-
71-
public AggOp getOp()
72-
{
66+
public AggOp getOp() {
7367
return _op;
7468
}
7569

76-
public void setOp(AggOp op)
77-
{
70+
public void setOp(AggOp op) {
7871
_op = op;
7972
}
8073

81-
public Direction getDirection()
82-
{
74+
public Direction getDirection() {
8375
return _direction;
8476
}
8577

86-
public void setDirection(Direction direction)
87-
{
78+
public void setDirection(Direction direction) {
8879
_direction = direction;
8980
}
9081

@@ -370,9 +361,8 @@ protected ExecType optFindExecType(boolean transitive) {
370361
ExecType REMOTE = ExecType.SPARK;
371362

372363
//forced / memory-based / threshold-based decision
373-
if( _etypeForced != null )
374-
{
375-
_etype = _etypeForced;
364+
if( _etypeForced != null ) {
365+
setExecType(_etypeForced);
376366
}
377367
else
378368
{
@@ -381,10 +371,10 @@ protected ExecType optFindExecType(boolean transitive) {
381371
}
382372
// Choose CP, if the input dimensions are below threshold or if the input is a vector
383373
else if(getInput().get(0).areDimsBelowThreshold() || getInput().get(0).isVector()) {
384-
_etype = ExecType.CP;
374+
setExecType(ExecType.CP);
385375
}
386376
else {
387-
_etype = REMOTE;
377+
setExecType(REMOTE);
388378
}
389379

390380
//check for valid CP dimensions and matrix size
@@ -402,15 +392,15 @@ else if(getInput().get(0).areDimsBelowThreshold() || getInput().get(0).isVector(
402392
&& (onlyOneParent() || allParentsSpark() || inputDoesNotRequireAggregation() ))
403393
{
404394
//pull unary aggregate into spark
405-
_etype = ExecType.SPARK;
395+
setExecType(ExecType.SPARK);
406396
}
407397

408398
//ensure cp exec type for single-node operations
409-
if( _op == AggOp.UNIQUE ) {
410-
_etype = ExecType.CP;
411-
} else {
399+
if( _op == AggOp.UNIQUE )
400+
setExecType(ExecType.CP);
401+
else
412402
setRequiresRecompileIfNecessary();
413-
}
403+
414404
return _etype;
415405
}
416406

‎src/main/java/org/apache/sysds/hops/BinaryOp.java‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,6 @@ public BinaryOp(String l, DataType dt, ValueType vt, OpOp2 o,
110110
refreshSizeInformation();
111111
}
112112

113-
@Override
114-
public void checkArity() {
115-
HopsException.check(_input.size() == 2, this, "should have arity 2 but has arity %d", _input.size());
116-
}
117-
118113
public OpOp2 getOp() {
119114
return op;
120115
}
@@ -756,13 +751,12 @@ protected ExecType optFindExecType(boolean transitive) {
756751
DataType dt2 = getInput().get(1).getDataType();
757752

758753
if( _etypeForced != null ) {
759-
_etype = _etypeForced;
754+
setExecType(_etypeForced);
760755
}
761756
else
762757
{
763-
if ( OptimizerUtils.isMemoryBasedOptLevel() )
764-
{
765-
_etype = findExecTypeByMemEstimate();
758+
if ( OptimizerUtils.isMemoryBasedOptLevel() ) {
759+
setExecType(findExecTypeByMemEstimate());
766760
}
767761
else
768762
{
@@ -773,29 +767,29 @@ protected ExecType optFindExecType(boolean transitive) {
773767
if ( (getInput().get(0).areDimsBelowThreshold() && getInput().get(1).areDimsBelowThreshold())
774768
|| (getInput().get(0).isVector() && getInput().get(1).isVector()))
775769
{
776-
_etype = ExecType.CP;
770+
setExecType(ExecType.CP);
777771
}
778772
}
779773
else if ( dt1 == DataType.MATRIX && dt2 == DataType.SCALAR ) {
780774
if ( getInput().get(0).areDimsBelowThreshold() || getInput().get(0).isVector() )
781775
{
782-
_etype = ExecType.CP;
776+
setExecType(ExecType.CP);
783777
}
784778
}
785779
else if ( dt1 == DataType.SCALAR && dt2 == DataType.MATRIX ) {
786780
if ( getInput().get(1).areDimsBelowThreshold() || getInput().get(1).isVector() )
787781
{
788-
_etype = ExecType.CP;
782+
setExecType(ExecType.CP);
789783
}
790784
}
791785
else
792786
{
793-
_etype = ExecType.CP;
787+
setExecType(ExecType.CP);
794788
}
795789

796790
//if no CP condition applied
797791
if( _etype == null )
798-
_etype = ExecType.SPARK;
792+
setExecType(ExecType.SPARK);
799793
}
800794

801795
//check for valid CP dimensions and matrix size

‎src/main/java/org/apache/sysds/hops/DataGenOp.java‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -140,13 +140,6 @@ public DataGenOp(OpOpDG mthd, DataIdentifier id)
140140
refreshSizeInformation();
141141
}
142142

143-
@Override
144-
public void checkArity() {
145-
int sz = _input.size();
146-
int pz = _paramIndexMap.size();
147-
HopsException.check(sz == pz, this, "has %d inputs but %d parameters", sz, pz);
148-
}
149-
150143
@Override
151144
public String getOpString() {
152145
return "dg(" + _op.toString().toLowerCase() +")";

‎src/main/java/org/apache/sysds/hops/DataOp.java‎

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -200,33 +200,6 @@ public DataOp(String l, DataType dt, ValueType vt,
200200
setFileFormat(FileFormat.BINARY);
201201
}
202202

203-
/** Check for N (READ) or N+1 (WRITE) inputs. */
204-
@Override
205-
public void checkArity() {
206-
int sz = _input.size();
207-
int pz = _paramIndexMap.size();
208-
switch (_op) {
209-
case PERSISTENTREAD:
210-
case TRANSIENTREAD:
211-
case SQLREAD:
212-
HopsException.check(sz == pz, this,
213-
"in %s operator type has %d inputs and %d parameters", _op.name(), sz, pz);
214-
break;
215-
case PERSISTENTWRITE:
216-
case TRANSIENTWRITE:
217-
case FUNCTIONOUTPUT:
218-
HopsException.check(sz == pz + 1, this,
219-
"in %s operator type has %d inputs and %d parameters (expect 1 more input for write operator type)",
220-
_op.name(), sz, pz);
221-
break;
222-
223-
case FEDERATED:
224-
//TODO
225-
default:
226-
//do nothing
227-
}
228-
}
229-
230203
public OpOpData getOp() {
231204
return _op;
232205
}

‎src/main/java/org/apache/sysds/hops/DnnOp.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,6 @@ public DnnOp(String l, DataType dt, ValueType vt, OpOpDnn o, ArrayList<Hop> inp)
8484
refreshSizeInformation();
8585
}
8686

87-
@Override
88-
public void checkArity() {
89-
HopsException.check(_input.size() >= 1, this, "should have at least one input but has %d inputs", _input.size());
90-
}
91-
9287
public OpOpDnn getOp() {
9388
return op;
9489
}

‎src/main/java/org/apache/sysds/hops/FunctionOp.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,6 @@ public FunctionOp(FunctionType type, String fnamespace, String fname, String[] i
9191
}
9292
}
9393

94-
/** FunctionOps may have any number of inputs. */
95-
@Override
96-
public void checkArity() {}
97-
9894
public String getFunctionKey() {
9995
return DMLProgram.constructFunctionKey(
10096
getFunctionNamespace(), getFunctionName());
@@ -403,10 +399,11 @@ protected ExecType optFindExecType(boolean transitive)
403399
if ( getFunctionType() == FunctionType.MULTIRETURN_BUILTIN ) {
404400
boolean isBuiltinFunction = isBuiltinFunction();
405401
// check if there is sufficient memory to execute this function
402+
double mem = getMemEstimate(); // obtain memory estimates for all (e.g., used in parfor)
406403
if(isBuiltinFunction && getFunctionName().equalsIgnoreCase("transformencode") ) {
407404
_etype = ((_etypeForced==ExecType.SPARK
408-
|| (getMemEstimate() >= OptimizerUtils.getLocalMemBudget()
409-
&& OptimizerUtils.isSparkExecutionMode())) ? ExecType.SPARK : ExecType.CP);
405+
|| (mem >= OptimizerUtils.getLocalMemBudget() && OptimizerUtils.isSparkExecutionMode())) ?
406+
ExecType.SPARK : ExecType.CP);
410407
}
411408
else if(isBuiltinFunction && (getFunctionName().equalsIgnoreCase("lstm") || getFunctionName().equalsIgnoreCase("lstm_backward"))) {
412409
_etype = DMLScript.USE_ACCELERATOR ? ExecType.GPU : ExecType.CP;

‎src/main/java/org/apache/sysds/hops/Hop.java‎

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -181,19 +181,7 @@ public long getHopID() {
181181
return _ID;
182182
}
183183

184-
/**
185-
* Check whether this Hop has a correct number of inputs.
186-
*
187-
* (Some Hops can have a variable number of inputs, such as DataOp, DataGenOp, ParameterizedBuiltinOp,
188-
* ReorgOp, TernaryOp, QuaternaryOp, MultipleOp, DnnOp, and SpoofFusedOp.)
189-
*
190-
* Parameterized Hops (such as DataOp) can check that the number of parameters matches the number of inputs.
191-
*
192-
*/
193-
public abstract void checkArity();
194-
195-
public ExecType getExecType()
196-
{
184+
public ExecType getExecType() {
197185
return _etype;
198186
}
199187

@@ -307,18 +295,6 @@ public boolean hasValidCPDimsAndSize() {
307295
invalid |= !OptimizerUtils.isValidCPDimensions(in._dc.getRows(), in._dc.getCols());
308296
return !invalid;
309297
}
310-
311-
public boolean hasMatrixInputWithDifferentBlocksizes() {
312-
for( Hop c : getInput() ) {
313-
if( c.getDataType()==DataType.MATRIX
314-
&& getBlocksize() != c.getBlocksize() )
315-
{
316-
return true;
317-
}
318-
}
319-
320-
return false;
321-
}
322298

323299
public void setRequiresReblock(boolean flag) {
324300
_requiresReblock = flag;

‎src/main/java/org/apache/sysds/hops/IndexingOp.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,6 @@ public IndexingOp(String l, DataType dt, ValueType vt, Hop inpMatrix, Hop inpRow
7474
setColLowerEqualsUpper(passedColsLEU);
7575
}
7676

77-
@Override
78-
public void checkArity() {
79-
HopsException.check(_input.size() == 5, this, "should have 5 inputs but has %d inputs", _input.size());
80-
}
81-
8277
public boolean isRowLowerEqualsUpper(){
8378
return _rowLowerEqualsUpper;
8479
}

‎src/main/java/org/apache/sysds/hops/LeftIndexingOp.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,6 @@ public LeftIndexingOp(String l, DataType dt, ValueType vt, Hop inpMatrixLeft, Ho
7373
setColLowerEqualsUpper(passedColsLEU);
7474
}
7575

76-
@Override
77-
public void checkArity() {
78-
HopsException.check(_input.size() == 6, this, "should have 6 inputs but has %d inputs", 6);
79-
}
80-
8176
public boolean isRowLowerEqualsUpper(){
8277
return _rowLowerEqualsUpper;
8378
}

0 commit comments

Comments
 (0)