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

Type annotations for Approximations #258 #322

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Type annotations for Approximations #258 #322

wants to merge 3 commits into from

Conversation

manel1874
Copy link

@manel1874 manel1874 commented Feb 2, 2022 •
edited

Description

Added type annotations to the approximations module of SyMPC:

  • exponential.py
  • log.py
  • reciprocal.py
  • sigmoid.py
  • softmax.py
  • tanh.py
  • utils.py

This fixes #258.

Affected Dependencies

None.

How has this been tested?

  • Tested each function individually in a jupyter notebook.

Checklist

@gmuraru
Copy link
Member

gmuraru commented Feb 6, 2022

Hello @manel1874.
Thank you for the PR - I have some suggestions. Probably you will encounter the circular import problem if you import the MPCTensor.

One solution for this will be to create a register decorator that will keep track of all the approximations we have - something like:

@register_approximation("log")
def log(....)

And this decorator will add the functions in a dictionary.

Then in the MPCTensor we will import a wrapper class around that dictionary and use that to call the approximations.

Let me know if this sounds ok - also if you have more questions - feel free to message me on Slack.

@manel1874
src/sympc/tensor/register_approximations.py: Created a register decor…
1d36c25 
…ator that keeps track of the approximations we have.
gmuraru
gmuraru approved these changes Apr 18, 2022
View reviewed changes
Copy link
Member

@gmuraru gmuraru left a comment

Choose a reason for hiding this comment

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

LGTM! Nice job!

@gmuraru
Copy link
Member

gmuraru commented Apr 18, 2022

@manel1874 could you do a rebase? there was a problem with the black code format tool, but it should be solved now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmuraru gmuraru gmuraru approved these changes

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

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

@rasswanth-s rasswanth-s Awaiting requested review from rasswanth-s rasswanth-s is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Type annotations for Approximations
2 participants
@manel1874 @gmuraru