[SYSTEMDS-2604] Federated compile and federated output propagation#1199
[SYSTEMDS-2604] Federated compile and federated output propagation#1199sebwrede wants to merge 7 commits intoapache:masterfrom
Conversation
This is the initial step for supporting compilation of FED instructions. The constructLops() method needs to be adapted to handle FED type hops for the other implementations of Hop similar to what is done in this commit for AggBinaryOp.
This commit edits the way ExecType is updated and adds a federatedOutput field in HOPs which can be used later to define a sequence of federated instructions with no GET calls between the federated instruction calls
This is only propagated for a single use case which involves Binary, AggBinary, and Transpose.
|
did you fix some bugs since it parses all tests? |
I removed an empty file, which made one test fail since it did not have an Apache license. This idea came from here: sagemath/cloud#114 (comment). |
okay, i will try to reinstall my r because i have tests that fail in GitHub actions and not locally... |
Baunsgaard
left a comment
There was a problem hiding this comment.
LGTM, only minor syntax comments
src/main/java/org/apache/sysds/runtime/instructions/FEDInstructionParser.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/sysds/runtime/instructions/fed/BinaryFEDInstruction.java
Show resolved
Hide resolved
|
LGTM. |
This PR adds a flag to compile the federated instructions during compile-time instead of simply converting the CP instructions to FED instructions during runtime. Later, the decision to compile to FED will be expanded so that it can base the decision on several scenarios, including the scenario where a chain of federated operations can be executed without sharing the data with the coordinator. Many of the FED instructions are currently implemented in a way that keeps the output federated, but this decision of keeping the output federated needs to be dependent on the flag so that we can have some kind of "FED planner" which compiles this information. |
Got it. Thanks for explaining in detail. Pushing the decision to the compiler seems to be a good idea. Regarding the chain of OPs, many today need partial output aggregation though I guess. |
Yes, this is true. There will be a lot of work with implementing the processing of the federated instructions. |
This PR adds compilation of federated instructions when
OptimizerUtils.FEDERATED_COMPILATIONis set to true.Additionally, a federated output flag is added and propagated through HOPs, LOPs, and instructions. The federated output flag will later be used to change the processing of federated instructions so that the output of federated instructions are not retrieved before all subsequent federated instructions have been executed.
The federated compilation and propagation of the federated output flag will be extended to other operations in future PRs.
Several design decisions have been made in this PR, for instance the decision to add the federated output flag as an additional operand in the instruction string. It could also have been added as a property of the output operand, but this would have consequences on how CPOperands are handled in the system. Comments on this or new ideas are appreciated.