mboehm7 commented on pull request #919: URL: https://github.com/apache/systemml/pull/919#issuecomment-636521576
LGTM - thanks @sebwrede. That's a very good start on runtime propagation. There were a couple of minor things I changed during the merge though: First, I moved individual checks for privacy constraints into a separate `PrivacyMonitor` in the privacy package. A central place and lean integration points, will later allow us to for example keep a trace of how privacy constraints propagated and which exchanges we checked (and potentially give out to a user as a explanation on how the privacy constraints were respected). Second, I cleaned up remaining warnings (imports, non-static methods), moved the `PrivacyException` into the privacy package too, and cleaned up the `VariableCPInstruction` (e.g., redundant method for obtaining the variable opcode, and switch statements). Finally, when looking over the tests it seems that the error handling in the communication with federated workers is still very preliminary (e.g., worker throws privacy exception and sends the message, master fails with class cast exception on trying to convert the message into a matrix). Could you maybe clean this up (to propagate the permission denied to the user at the master) in your next iteration on runtime propagation? Thanks. ---------------------------------------------------------------- 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: us...@infra.apache.org
