ywcb00 commented on a change in pull request #1371:
URL: https://github.com/apache/systemds/pull/1371#discussion_r700225238
##########
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:
We could just replace the boolean _fedOutput_ of the instruction with
the __fedOut_ field, right? Then, in principle, the __fedOut_ field is used to
decide in which branch (federated/local) the output is created.
However, this change should not affect the _setFedOutput_ method at all, but
it would bring with it an advantage for the case, that in the future even
partially aggregated federated output is supported. :thinking:
--
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]
