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

Type annotations for Approximations #258

Open
7 tasks
kamathhrishi opened this issue Jun 27, 2021 · 4 comments · May be fixed by #322
Open
7 tasks

Type annotations for Approximations #258

kamathhrishi opened this issue Jun 27, 2021 · 4 comments · May be fixed by #322
Assignees
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@kamathhrishi
Copy link
Contributor

Where?

Currently, the approximations module of SyMPC does not have type annotations.
The following are the files with missing type annotations:

  • exponential.py
  • log.py
  • reciprocal.py
  • sigmoid.py
  • softmax.py
  • tanh.py
  • utils.py
@kamathhrishi kamathhrishi added documentation Improvements or additions to documentation good first issue Good for newcomers labels Jun 27, 2021
@rhit-namburs
Copy link

Hi I would like to contribute to this. Note that this is my first time contributing to a open source project.

@rasswanth-s
Copy link
Contributor

Sure @rhit-namburs , go for it 👍

@rasswanth-s
Copy link
Contributor

@rhit-namburs are you still working on it?

@manel1874
Copy link

Hi! I am not sure if @rhit-namburs is still working on this issue so I decided to take a look.

I noticed that softmax.py and tanh.py are already annotated, so I decided to follow their approach. However, this leads to a circular import issue:

  1. sympc.tensor.mpc_tensor.py imports APPROXIMATIONS from sympc.approximations, which includes func = (sigmoid.py, log.py, exponential.py, reciprocal.py, utils.py).
  2. softmax.py and tanh.py import MPCTensor from sympc.tensor to be used for the type annotations. If we do the same for func we go into a loop of imports.

Without restructuring both sympc.approximations folder and MPCTensor class, I think the easiest way to go around this issue is to use TypeVar from typing to define an MPCTensor annotation. I tested and this solution passes the pre-commit tests.

Another possible solution could be to include the approximation functions defined in sympc.approximations folder inside MPCTensor class.

@manel1874 manel1874 linked a pull request Feb 2, 2022 that will close this issue
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants