search

georgymh / decentralized-ml

Interoperable and decentralized machine learning.
Apache License 2.0
9 stars 5 forks source link

Averaging #29

Closed kiddyboots216 closed 5 years ago

kiddyboots216 commented 5 years ago

Adding averaging jobtype to Runner. Also added a mock Communication Jobtype just to get to the Averaging test. Please do me an approve friendo. No am many lines code change. (P.S. the integration test for the Communication Manager looks horrific, I know, but just imagine how hard it was to make the test!)

kiddyboots216 commented 5 years ago

This PR is not dependent on how the Gateway communicates with the rest of the service. COMM skeleton code is left in because it's been consistently useful in testing and I don't think it's a good use of time to split up stuff, spend even more time writing temporary tests, and have large leaps of logic. If you think it's that important then we can do it, I guess, it's just time wasted.

georgymh commented 5 years ago

Ok, since it's not dependent on the Blockchain code, let's remove the code related to it (NEW_INFO, the code I removed from the optimizers in PR #26, etc).

About the COMM code, it sounds like there's less boilerplate/temp code to be written in this PR if the COMM PR is submitted first, so why don't we get the COMM PR out first and then we come back to this one? It will make reviewing both/ approving both easier for both of us.

kiddyboots216 commented 5 years ago

This PR is ready to be merged. @neeleshdodda44 Please review.

kiddyboots216 commented 5 years ago

FYI the reason JOB_COMM is in here at all is because @georgymh you pushed a buggy commit that causes errors in Runner threads to take down the main thread, meaning that a few of these tests won't pass (since the Communication Manager/Optimizer automatically transition and send a communication job, which takes down the main thread with an invalid job assertion error) unless the communication job skeleton code is left in.