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

Adding Serde to syft.js #43

Merged
merged 13 commits into from
Aug 1, 2019
Merged

Adding Serde to syft.js #43

merged 13 commits into from
Aug 1, 2019

Conversation

cereallarceny
Copy link
Member

In this PR we accomplished the following:

  1. Got a working example use-case put together with hot reloading.
  2. Wrote the entirety of the Serde parser in Javascript. We will need to add further types in the future and there are a few outstanding issues that we're tracking on PySyft: (Everything should simplify as a tuple in serde._simplify PySyft#2422, All strings should simplify without "b" and additional comma PySyft#2423, Serde is potentially simplifying information twice, creating a bad nested structure PySyft#2424)
  3. Created a translation layer between Javascript and Python (and back).
  4. Created the basic primitives for all types (PointerTensor, TorchTensor, TorchSize, Tuple (imported from another library), Range, Slice, and Plan.
  5. We have 100% test coverage across all files (except the index.js, which is currently in flux and going to change dramatically... so I didn't bother).

@cereallarceny cereallarceny self-assigned this Aug 1, 2019
@cereallarceny cereallarceny changed the title Comms Adding Serde to syft.js Aug 1, 2019
@codecov
Copy link

codecov bot commented Aug 1, 2019

Codecov Report

Merging #43 into master will increase coverage by 12.3%.
The diff coverage is 98.11%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #43      +/-   ##
=========================================
+ Coverage      75%   87.3%   +12.3%     
=========================================
  Files           3      12       +9     
  Lines         100     197      +97     
  Branches       16      31      +15     
=========================================
+ Hits           75     172      +97     
  Misses         20      20              
  Partials        5       5
Impacted Files Coverage Δ
src/index.js 69.51% <0%> (-4.36%) ⬇️
src/custom-types/slice.js 100% <100%> (ø)
src/custom-types/torch-size.js 100% <100%> (ø)
src/custom-types/torch-tensor.js 100% <100%> (ø)
src/custom-types/plan.js 100% <100%> (ø)
src/custom-types/pointer-tensor.js 100% <100%> (ø)
src/_constants.js 100% <100%> (ø)
src/serde.js 100% <100%> (ø)
src/logger.js 100% <100%> (+16.66%) ⬆️
src/custom-types/range.js 100% <100%> (ø)
... and 12 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 3c65980...21d32e1. Read the comment docs.

Copy link
Member

@iamtrask iamtrask left a comment

Choose a reason for hiding this comment

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

Absolutely fantastic work! Impressively fast turnaround as well!

@iamtrask iamtrask merged commit 0c7bbcd into master Aug 1, 2019
@cereallarceny cereallarceny deleted the comms branch August 1, 2019 15:37
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.

2 participants