Baunsgaard commented on a change in pull request #1062:
URL: https://github.com/apache/systemds/pull/1062#discussion_r492843396
##########
File path:
src/main/java/org/apache/sysds/runtime/instructions/InstructionUtils.java
##########
@@ -140,6 +141,19 @@ public static int checkNumFields( String[] parts, int
expected1, int expected2 )
return numFields;
}
+ public static int checkNumFields( String[] parts, int... expected ) {
+ int numParts = parts.length;
+ int numFields = numParts - 1; //account for opcode
+
+ if (Arrays.stream(expected).noneMatch((i) -> numFields == i))
+ throw new DMLRuntimeException(
+ "checkNumFields() -- expected number ("
+ +
Arrays.stream(expected).mapToObj(Integer::toString).reduce((a, b) -> a + ", " +
b).orElse("")
+ + ") != is not equal to actual number
(" + numFields + ").");
Review comment:
this looks overkill, maybe use a string builder
##########
File path:
src/main/java/org/apache/sysds/runtime/instructions/fed/AppendFEDInstruction.java
##########
@@ -33,71 +32,74 @@
import org.apache.sysds.runtime.meta.DataCharacteristics;
public class AppendFEDInstruction extends BinaryFEDInstruction {
- protected boolean _cbind; //otherwise rbind
-
- protected AppendFEDInstruction(Operator op, CPOperand in1, CPOperand
in2, CPOperand out,
- boolean cbind, String opcode, String istr) {
+ protected boolean _cbind; // otherwise rbind
+
+ protected AppendFEDInstruction(Operator op, CPOperand in1, CPOperand
in2, CPOperand out, boolean cbind,
+ String opcode, String istr) {
super(FEDType.Append, op, in1, in2, out, opcode, istr);
_cbind = cbind;
}
-
+
public static AppendFEDInstruction parseInstruction(String str) {
String[] parts =
InstructionUtils.getInstructionPartsWithValueType(str);
- InstructionUtils.checkNumFields(parts, 5, 4);
-
+ InstructionUtils.checkNumFields(parts, 6, 5, 4);
+
String opcode = parts[0];
CPOperand in1 = new CPOperand(parts[1]);
CPOperand in2 = new CPOperand(parts[2]);
CPOperand out = new CPOperand(parts[parts.length - 2]);
boolean cbind = Boolean.parseBoolean(parts[parts.length - 1]);
-
+
Operator op = new
ReorgOperator(OffsetColumnIndex.getOffsetColumnIndexFnObject(-1));
return new AppendFEDInstruction(op, in1, in2, out, cbind,
opcode, str);
}
-
+
@Override
public void processInstruction(ExecutionContext ec) {
- //get inputs
+ // get inputs
MatrixObject mo1 = ec.getMatrixObject(input1.getName());
MatrixObject mo2 = ec.getMatrixObject(input2.getName());
DataCharacteristics dc1 = mo1.getDataCharacteristics();
DataCharacteristics dc2 = mo1.getDataCharacteristics();
-
- //check input dimensions
- if (_cbind && mo1.getNumRows() != mo2.getNumRows()) {
- throw new DMLRuntimeException(
- "Append-cbind is not possible for federated
input matrices " + input1.getName() + " and "
- + input2.getName() + " with different number of
rows: " + mo1.getNumRows() + " vs "
- + mo2.getNumRows());
- }
- else if (!_cbind && mo1.getNumColumns() != mo2.getNumColumns())
{
- throw new DMLRuntimeException(
- "Append-rbind is not possible for federated
input matrices " + input1.getName() + " and "
- + input2.getName() + " with different number of
columns: " + mo1.getNumColumns()
- + " vs " + mo2.getNumColumns());
+
+ // check input dimensions
+ if(_cbind && mo1.getNumRows() != mo2.getNumRows()) {
+ throw new DMLRuntimeException("Append-cbind is not
possible for federated input matrices "
+ + input1.getName() + " and " + input2.getName()
+ " with different number of rows: " + mo1.getNumRows()
+ + " vs " + mo2.getNumRows());
Review comment:
again, maybe string builder.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
