mboehm7 edited a comment on pull request #1237: URL: https://github.com/apache/systemds/pull/1237#issuecomment-841874157
Thanks for the patch and the detailed discussion @sebwrede and @sebwrede. I started merging it but need to finalize this tomorrow (so please no new commits on this PR). As a high-level summary: * The modification of the FederatedRange is actually unnecessary as we predominately iterate over the entry set - now we simply convert this map to a list of pairs and avoid any workarounds for comparison. * In anticipation of @OlgaOvcharenko PR (currently WIP) for handling broadcasts and sliced broadcasts as federated data as well, we should clearly separate the partitioning type (ROW, COL, NONE, MIXED) and the replication type (NONE, FULL, PARTIAL) where PARTIAL might be OVERLAP. * Currently the federatedOutput flags are conditionally compiled into the instructions. We should add a three-valued logic here (true, false, none) where true/false forced the instruction to act a certain way, while none preserves the current heuristic runtime semantics. * We need to fix (i.e., remove) all the stateful string concatenation -- 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]
