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

Fix notebooks in Syft Core #74

Open
gmuraru opened this issue Mar 7, 2021 · 7 comments
Open

Fix notebooks in Syft Core #74

gmuraru opened this issue Mar 7, 2021 · 7 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@gmuraru
Copy link
Member

gmuraru commented Mar 7, 2021

Description

There is a problem with the Syft Notebooks for SyMPC (the request name parameter is no longer needed).

How to Reproduce

  1. Install SyMPC and Syft
  2. Run the notebooks from Syft

Expected Behavior

The notebooks should run smoothly.

@gmuraru gmuraru added the good first issue Good for newcomers label Mar 7, 2021
@gmuraru gmuraru added this to the Code Quality milestone Mar 19, 2021
@archity
Copy link

archity commented Jul 10, 2021

The link to the "Syft Notebooks for SyMPC" seems to be broken. If the issue is still open, I'd like to contribute to this issue.

@rasswanth-s
Copy link
Contributor

Sure @archity ,I have also opened a corresponding issue in Pysyft yesterday,you could also comment there, OpenMined/PySyft#5771

@archity
Copy link

archity commented Jul 10, 2021

I think I wasn't clear (or maybe I haven't understood your comment), but the link "Syft Notebooks for SyMPC" in this current issue is broken (404 error), and also, the other issue (#5771) is not a "good first issue", so I'm not sure if I would be able to do that (This would be my first PR in this repository/community).

(Unless the 2 issues are related to each other?)

@rasswanth-s
Copy link
Contributor

rasswanth-s commented Jul 10, 2021

You would like to modify the broken link,in the issue statement right? It got broken due to change in PySyft to monorepo.Yeah you could take up the whole issue(fixing examples) later,if you interested.Both the issues in PySyft and SyMPC are the same.

@kamathhrishi
Copy link
Contributor

@archity It's basically changing the notebooks itself.

@archity
Copy link

archity commented Aug 3, 2021

Hello, I have a couple of questions, and I apologize in advance in case the questions seem to be too basic or obvious, but I just want to make sure that we are indeed on the same page, and also I'm not really sure what am I looking at.

  1. First of all, which notebook(s) are we talking about fixing in this particular GitHub issue? Is it the Duet notebooks (1-DS-1-DO)?

  2. When you say in the issue description "the request name parameter is no longer needed", what exactly do you mean? Assuming we're talking about the MPC Tensor notebooks (1-DS-1-DO ones), are you referring to sy.load("sympc") not requried? Because that seems to prompt a warning saying "sy.load() is deprecated and not needed anymore". That seems to be the only thing related to the "request name parameter" thing.

  3. The notebooks seem to run fine to the point that both parties (DS and DO) are able to connect with each other, and the DS is able to access DO's datasets of 2 row Pandas dataframe. But when the DS tries to perform operations like addition/subtraction, then it throws some errors, saying "UnknownPrivateException has been triggered." During this part in the background, the DO starts logging in a bunch of warnings which for the most part are difficult for me to interpret. it seems to be saying something about "Unable to Get Object with ID x from store. Possible dangling Pointer" My question is, is that the problem that I'm supposed to be fixing? Or is it being caused by me not executing the cells in the correct order, and the problem (to fix) is something else?

@eragon-saphira
Copy link

@archity
2. In the DO notebooks for 1DS-2DO there is an extra name parameter in requests.add_handler, which has been removed thath should be updated.
3. The reason for this dangling pointer exception is that there should be an import sympc in the DO notebooks as well, since the ast library wrappers are only triggered at import(you can also do it by sy.load("sympc",ignore_warning=True)
4. In addtion the Duet notebooks have another issue tagged at Issue #282

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants