Skip to content
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

Update protobuf types and Plan, temporarily rollback Threepio usage #114

Merged
merged 4 commits into from
Apr 15, 2020

Conversation

vvmnnnkv
Copy link
Member

Changes:

  • Make protobuf and Plan up to date with PySyft and syft-proto
  • Add unit test that runs MNIST example plan
  • Temporarily restored legacy translation because Threepio doesn't have all commands we need for MNIST example (examples: sub, truediv, gt); Additionally, even if we fallback to legacy only on commands that Threepio doesn't know, there's still a problem, the plan execution result is incorrect (needs further investigation).

@codecov
Copy link

codecov bot commented Apr 12, 2020

Codecov Report

Merging #114 into master will decrease coverage by 0.18%.
The diff coverage is 89.39%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #114      +/-   ##
==========================================
- Coverage   72.16%   71.97%   -0.19%     
==========================================
  Files          22       24       +2     
  Lines         740      760      +20     
  Branches       93       96       +3     
==========================================
+ Hits          534      547      +13     
- Misses        185      191       +6     
- Partials       21       22       +1     
Impacted Files Coverage Δ
src/protobuf/mapping.js 100.00% <ø> (ø)
src/syft-model.js 0.00% <ø> (ø)
src/types/message.js 100.00% <ø> (+5.08%) ⬆️
src/_errors.js 53.84% <33.33%> (-0.70%) ⬇️
src/types/placeholder.js 94.44% <88.88%> (-5.56%) ⬇️
src/types/state.js 90.00% <90.00%> (ø)
src/types/role.js 90.47% <90.47%> (ø)
src/types/computation-action.js 90.90% <90.90%> (ø)
src/types/plan.js 90.00% <100.00%> (-2.31%) ⬇️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d070f6...f41a93d. Read the comment docs.


import { Threepio, Command } from '@openmined/threepio';
const threepio = new Threepio('torch', 'tfjs', tf);
*/
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Threepio is temporarily commented out.

}
const cmd = new Command(functionName, resolvedArgs, this.kwargs);
const translation = threepio.translate(cmd);
return translation.executeRoutine();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Threepio is temporarily commented out.

@cereallarceny cereallarceny merged commit f52a359 into master Apr 15, 2020
@delete-merged-branch delete-merged-branch bot deleted the vvm/protobuf-and-threepio branch April 15, 2020 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants