New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SYSTEMDS-2550] Batch scaling and weighing of imbalanced workers #1141
[SYSTEMDS-2550] Batch scaling and weighing of imbalanced workers #1141
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It generally looks good, I added a few comments.
It could make sense to add documentation to each of the subclasses of DataPartitionFederatedScheme describing the partitioning scheme the class represents. This would make it easier to quickly understand the differences between the partitioning scheme classes, which would make it easier for you to present your work and it would be easier for future development/review of the classes.
src/main/java/org/apache/sysds/runtime/controlprogram/paramserv/FederatedPSControlThread.java
Show resolved
Hide resolved
src/main/java/org/apache/sysds/runtime/controlprogram/paramserv/FederatedPSControlThread.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/sysds/runtime/instructions/cp/ParamservBuiltinCPInstruction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/sysds/runtime/instructions/cp/ParamservBuiltinCPInstruction.java
Outdated
Show resolved
Hide resolved
…tored scaling to weighing
ece1f8d
to
87b1824
Compare
@sebwrede I have addressed all other proposed changes in the new commit. Thanks! |
LGTM! |
LGTM - thanks @tobiasrieger for the patch and @sebwrede for the review. I only modified some formatting issues, reduced the visibility of some members, change the string concatenation, and guarded the info logging (to avoid unnecessary string concatenation). |
This PR provides a number of updates to the Federated Parameter Server