You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@systemds.apache.org by GitBox <gi...@apache.org> on 2021/03/30 08:12:36 UTC

[GitHub] [systemds] Baunsgaard commented on a change in pull request #1199: [SYSTEMDS-2604] Federated compile and federated output propagation

Baunsgaard commented on a change in pull request #1199:
URL: https://github.com/apache/systemds/pull/1199#discussion_r603873511



##########
File path: src/main/java/org/apache/sysds/runtime/instructions/FEDInstructionParser.java
##########
@@ -32,8 +36,28 @@
 	public static final HashMap<String, FEDType> String2FEDInstructionType;
 	static {
 		String2FEDInstructionType = new HashMap<>();
-		String2FEDInstructionType.put("fedinit", FEDType.Init);
-		String2FEDInstructionType.put("ba+*",    FEDType.AggregateBinary);
+		String2FEDInstructionType.put("fedinit"  , FEDType.Init);
+		String2FEDInstructionType.put("tsmm"     , FEDType.Tsmm);
+		String2FEDInstructionType.put("ba+*"     , FEDType.AggregateBinary);
+
+		String2FEDInstructionType.put("uak+"     , FEDType.AggregateUnary);
+		String2FEDInstructionType.put( "uark+"   , FEDType.AggregateUnary);
+		String2FEDInstructionType.put( "uack+"   , FEDType.AggregateUnary);
+		String2FEDInstructionType.put( "uasqk+"  , FEDType.AggregateUnary);
+		String2FEDInstructionType.put( "uarsqk+" , FEDType.AggregateUnary);
+		String2FEDInstructionType.put( "uacsqk+" , FEDType.AggregateUnary);

Review comment:
       inlining

##########
File path: src/main/java/org/apache/sysds/hops/ReorgOp.java
##########
@@ -76,6 +77,8 @@ public ReorgOp(String l, DataType dt, ValueType vt, ReOrgOp o, ArrayList<Hop> in
 			getInput().add(i, in);
 			in.getParent().add(this);
 		}
+
+		updateETFed();

Review comment:
       double updateETFed here maybe only one needed?

##########
File path: src/main/java/org/apache/sysds/runtime/instructions/fed/BinaryFEDInstruction.java
##########
@@ -42,11 +47,12 @@ public static BinaryFEDInstruction parseInstruction(String str) {
 		}
 
 		String[] parts = InstructionUtils.getInstructionPartsWithValueType(str);
-		InstructionUtils.checkNumFields(parts, 3, 4);
+		InstructionUtils.checkNumFields(parts, 3, 4, 5);
 		String opcode = parts[0];
 		CPOperand in1 = new CPOperand(parts[1]);
 		CPOperand in2 = new CPOperand(parts[2]);
 		CPOperand out = new CPOperand(parts[3]);
+		boolean federatedOutput = parts.length > 5 && Boolean.parseBoolean(parts[5]);

Review comment:
       does this work? should it not be index 4? or is that the number of threads?

##########
File path: src/main/java/org/apache/sysds/hops/BinaryOp.java
##########
@@ -496,7 +498,7 @@ else if (mbin == MMBinaryMethod.MR_BINARY_M) {
 				setOutputDimensions(binary);
 				setLineNumbers(binary);
 				setLops(binary);
-			}
+			} else throw new HopsException("Lop construction not implemented for ExecType " + et);

Review comment:
       split line




-- 
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:
users@infra.apache.org