kev-inn commented on a change in pull request #1027:
URL: https://github.com/apache/systemds/pull/1027#discussion_r473958834
##########
File path:
src/main/java/org/apache/sysds/runtime/controlprogram/federated/FederatedWorkerHandler.java
##########
@@ -268,7 +268,8 @@ private FederatedResponse execUDF(FederatedRequest request)
{
return udf.execute(ec, inputs);
}
catch(Exception ex) {
- return new FederatedResponse(ResponseType.ERROR,
ex.getMessage());
+ return new FederatedResponse(ResponseType.ERROR, new
FederatedWorkerHandlerException(
+ "Exception of type " + ex.getClass() + " thrown
when processing request", ex));
Review comment:
`ex.getMessage()` would sometimes not suffice and instead be confusing.
A `ArrayIndexOutOfBoundsException` would have for example just have the index
as a message, therefore our response would just contain an arbitrary number and
the problem would not be explained
##########
File path: src/main/java/org/apache/sysds/runtime/transform/encode/Encoder.java
##########
@@ -145,7 +145,7 @@ public Encoder subRangeEncoder(int colStart, int colEnd) {
*/
protected void mergeColumnInfo(Encoder other, int col) {
// update number of columns
- _clen = Math.max(_colList.length, col - 1 + other.getNumCols());
+ _clen = Math.max(_clen, col - 1 + other._clen);
Review comment:
This was my mistake in the last PR `_colList.length` is something
different to `_clen` and `.getNumCols()` will be wrong once we add dummycoding
(it gives us the cols after encoding).
##########
File path:
src/main/java/org/apache/sysds/runtime/instructions/fed/ParameterizedBuiltinFEDInstruction.java
##########
@@ -19,103 +19,217 @@
package org.apache.sysds.runtime.instructions.fed;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.LinkedHashMap;
+
+import org.apache.sysds.common.Types;
import org.apache.sysds.common.Types.DataType;
import org.apache.sysds.common.Types.ValueType;
+import org.apache.sysds.hops.OptimizerUtils;
import org.apache.sysds.lops.Lop;
import org.apache.sysds.runtime.DMLRuntimeException;
+import org.apache.sysds.runtime.controlprogram.caching.FrameObject;
import org.apache.sysds.runtime.controlprogram.caching.MatrixObject;
import org.apache.sysds.runtime.controlprogram.context.ExecutionContext;
import org.apache.sysds.runtime.controlprogram.federated.FederatedRequest;
-import org.apache.sysds.runtime.functionobjects.ParameterizedBuiltin;
-import org.apache.sysds.runtime.functionobjects.ValueFunction;
+import org.apache.sysds.runtime.controlprogram.federated.FederatedResponse;
+import org.apache.sysds.runtime.controlprogram.federated.FederatedUDF;
+import org.apache.sysds.runtime.controlprogram.federated.FederationMap;
import org.apache.sysds.runtime.controlprogram.federated.FederationUtils;
+import org.apache.sysds.runtime.functionobjects.ValueFunction;
import org.apache.sysds.runtime.instructions.InstructionUtils;
import org.apache.sysds.runtime.instructions.cp.CPOperand;
+import org.apache.sysds.runtime.instructions.cp.Data;
+import org.apache.sysds.runtime.matrix.data.FrameBlock;
+import org.apache.sysds.runtime.matrix.data.MatrixBlock;
import org.apache.sysds.runtime.matrix.operators.Operator;
-import org.apache.sysds.runtime.matrix.operators.SimpleOperator;
+import org.apache.sysds.runtime.meta.MatrixCharacteristics;
+import org.apache.sysds.runtime.meta.MetaDataFormat;
+import org.apache.sysds.runtime.privacy.PrivacyMonitor;
+import org.apache.sysds.runtime.transform.decode.Decoder;
+import org.apache.sysds.runtime.transform.decode.DecoderFactory;
public class ParameterizedBuiltinFEDInstruction extends
ComputationFEDInstruction {
-
protected final LinkedHashMap<String, String> params;
-
- protected ParameterizedBuiltinFEDInstruction(Operator op,
- LinkedHashMap<String, String> paramsMap, CPOperand out, String
opcode, String istr)
- {
+
+ protected ParameterizedBuiltinFEDInstruction(Operator op,
LinkedHashMap<String, String> paramsMap, CPOperand out,
+ String opcode, String istr) {
super(FEDType.ParameterizedBuiltin, op, null, null, out,
opcode, istr);
params = paramsMap;
}
-
- public HashMap<String,String> getParameterMap() {
- return params;
+
+ public HashMap<String, String> getParameterMap() {
+ return params;
}
-
+
public String getParam(String key) {
return getParameterMap().get(key);
}
-
+
public static LinkedHashMap<String, String>
constructParameterMap(String[] params) {
// process all elements in "params" except first(opcode) and
last(output)
- LinkedHashMap<String,String> paramMap = new LinkedHashMap<>();
-
+ LinkedHashMap<String, String> paramMap = new LinkedHashMap<>();
+
// all parameters are of form <name=value>
String[] parts;
- for ( int i=1; i <= params.length-2; i++ ) {
+ for(int i = 1; i <= params.length - 2; i++) {
parts = params[i].split(Lop.NAME_VALUE_SEPARATOR);
paramMap.put(parts[0], parts[1]);
}
-
+
return paramMap;
}
-
- public static ParameterizedBuiltinFEDInstruction parseInstruction (
String str ) {
+
+ public static ParameterizedBuiltinFEDInstruction
parseInstruction(String str) {
String[] parts =
InstructionUtils.getInstructionPartsWithValueType(str);
// first part is always the opcode
String opcode = parts[0];
// last part is always the output
- CPOperand out = new CPOperand( parts[parts.length-1] );
-
+ CPOperand out = new CPOperand(parts[parts.length - 1]);
+
// process remaining parts and build a hash map
- LinkedHashMap<String,String> paramsMap =
constructParameterMap(parts);
-
+ LinkedHashMap<String, String> paramsMap =
constructParameterMap(parts);
+
// determine the appropriate value function
ValueFunction func = null;
- if( opcode.equalsIgnoreCase("replace") ) {
- func =
ParameterizedBuiltin.getParameterizedBuiltinFnObject(opcode);
- return new ParameterizedBuiltinFEDInstruction(new
SimpleOperator(func), paramsMap, out, opcode, str);
Review comment:
I formatted this file (sorry reviewer), but I think since those files
are quite new we should start by abiding to our code style.
----------------------------------------------------------------
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]
