[SYSTEMDS-2558,2554,2561] Fed frame recode transform (decode) support #1027
Conversation
kev-inn
force-pushed
the
fed_decode_frame
branch
from
cae7f4a to
45f769e
Compare
| return new FederatedResponse(ResponseType.ERROR, ex.getMessage()); | ||
| return new FederatedResponse(ResponseType.ERROR, new FederatedWorkerHandlerException( | ||
| "Exception of type " + ex.getClass() + " thrown when processing request", ex)); |
There was a problem hiding this 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
| 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); |
There was a problem hiding this 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.
There was a problem hiding this comment.
don't worry about the formatting - more severe was the above deletion of the recently added replace. ;-)
| 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); |
There was a problem hiding this 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).
kev-inn
force-pushed
the
fed_decode_frame
branch
from
7388543 to
5a9a1cd
Compare
kev-inn
changed the title
|
LGTM - thanks for the patch @kev-inn. Overall this is nice, I just made some minor modifications: (1) fixed the export in |
2 participants
Adds decode support for recode, pass-through and composite (containing only those two).