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.

Sign up for GitHub

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

Jump to bottom

Added security_type attribute to protocol #222

Merged
merged 2 commits into from
Jun 7, 2021
Merged

Added security_type attribute to protocol #222

merged 2 commits into from
Jun 7, 2021

Conversation

rasswanth-s
Copy link
Contributor

Description

This PR adds security_type attribute to the MPC protocols,given that each protocol has different security guarantees.It fixes #219 .
Corresponding PR in PySyft for serialization and de serialization of Protocol class

Affected Dependencies

List any dependencies that are required for this change.

How has this been tested?

Added tests for the same.

Checklist

@rasswanth-s rasswanth-s mentioned this pull request Jun 6, 2021
Merged
4 tasks
@rasswanth-s rasswanth-s added this to the Falcon milestone Jun 6, 2021
@rasswanth-s rasswanth-s added the feature Add a new functionality label Jun 6, 2021
kamathhrishi
kamathhrishi requested changes Jun 6, 2021
View reviewed changes
Copy link
Contributor

@kamathhrishi kamathhrishi left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Seems good like a good start
You will have to modify the notebook examples according to the latest protocol interface. Also, we have to post an issue in PySyft noting that the examples of Duet should be modified in accordance to new protocol interface.

Also I have added other comments.

@rasswanth-s
Copy link
Contributor Author

Seems good like a good start
You will have to modify the notebook examples according to the latest protocol interface. Also, we have to post an issue in PySyft noting that the examples of Duet should be modified in accordance to new protocol interface.

Also I have added other comments.

@kamathhrishi ,I saw the log of notebooks test Hrishi.I think the errors were due to the serialization part.I think It would be resolved once we merge the PySyft PR

LaRiffle
LaRiffle approved these changes Jun 6, 2021
View reviewed changes
Copy link
Contributor

@LaRiffle LaRiffle left a comment

Choose a reason for hiding this comment

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

Some small remarks, but this is a great addition towards more clarity about what we support or not! 🙌

src/sympc/protocol/falcon/falcon.py Outdated Show resolved Hide resolved
src/sympc/protocol/fss/fss.py Outdated Show resolved Hide resolved
tests/sympc/tensor/mpc_tensor_test.py Outdated Show resolved Hide resolved
@kamathhrishi kamathhrishi self-requested a review June 7, 2021 06:51
kamathhrishi
kamathhrishi approved these changes Jun 7, 2021
View reviewed changes
Copy link
Contributor

@kamathhrishi kamathhrishi left a comment

Choose a reason for hiding this comment

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

Looks good to me

@rasswanth-s rasswanth-s merged commit a96797d into OpenMined:main Jun 7, 2021
@rasswanth-s rasswanth-s deleted the security_type branch June 7, 2021 14:57
@rasswanth-s rasswanth-s self-assigned this Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kamathhrishi kamathhrishi kamathhrishi approved these changes

@LaRiffle LaRiffle LaRiffle approved these changes

@gmuraru gmuraru Awaiting requested review from gmuraru gmuraru is a code owner

@aanurraj aanurraj Awaiting requested review from aanurraj aanurraj is a code owner

Assignees

@rasswanth-s rasswanth-s

Labels
feature Add a new functionality
Projects
None yet
Milestone
Falcon
Development

Successfully merging this pull request may close these issues.

Addition of security_type attribute to the Protocols.
3 participants
@rasswanth-s @kamathhrishi @LaRiffle