-
Notifications
You must be signed in to change notification settings - Fork 162
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
docs: update documentation on client / server API #663
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great update, I suggested a few improvements !
070b67d
to
b6d5c85
Compare
client.generate_private_and_evaluation_keys() | ||
# Get the serialized evaluation keys | ||
serialized_evaluation_keys = client.get_serialized_evaluation_keys() | ||
serialized_evaluation_keys = client.generate_private_and_evaluation_keys() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking more the other way 😅 like keep serialized_evaluation_keys = client.get_serialized_evaluation_keys()
but include self.generate_private_and_evaluation_keys()
in get_serialized_evaluation_keys
or then have a new method called "generate_keys_and_get_serialized_evaluation_keys` or similar
but that was more of an idea, we could make it an issue and consider it later if you prefer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah actually it makes more sense the way you describe. Will update.
b6d5c85
to
671045c
Compare
"\n", | ||
"# Get the serialized evaluation keys\n", | ||
"serialized_evaluation_keys = fhemodel_client.get_serialized_evaluation_keys()" | ||
"serialized_evaluation_keys = fhemodel_client.generate_private_and_evaluation_keys()" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line should still use get_serialized_evaluation_keys
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a last change to fix
9fbc831
to
c004669
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks a lot !
Coverage passed ✅Coverage details
|
closes https://github.com/zama-ai/concrete-ml-internal/issues/4430