sebwrede commented on a change in pull request #1371:
URL: https://github.com/apache/systemds/pull/1371#discussion_r700247966
##########
File path:
src/main/java/org/apache/sysds/runtime/instructions/fed/CtableFEDInstruction.java
##########
@@ -116,119 +120,154 @@ public void processInstruction(ExecutionContext ec) {
mo1 = ec.getMatrixObject(input3);
}
- long dim1 = Collections.max(Arrays.asList(dims1),
Long::compare);
- boolean fedOutput = dim1 % mo1.getFedMapping().getSize() == 0
&& dims1.length == Arrays.stream(dims1).distinct().count();
+ // static non-partitioned output dimension (same for all
federated partitions)
+ long staticDim = Collections.max(Arrays.asList(dims1),
Long::compare);
+ boolean fedOutput = isFedOutput(mo1.getFedMapping(), mo2);
Review comment:
I think you are right. The only potential problem is that the current
processRequest implementation assumes that mo1 is federated and mo2 is
broadcast. This is only a problem if this same logic for checking which input
is federated is not a part of the federated planner, which sets the _fedOut
field. This is just something I will have to look at later when I extend the
federated planner to include this instruction.
The current implementation is sufficient for now.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
