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

Uniformize examples readme and quickstart [ready for reviews] #704

Merged

Conversation

bcm-at-zama
Copy link
Contributor

Mainly, this PR is to unify examples in README and in quick_start.md, as requested by @yuxizama.

closes zama-ai/concrete-internal#603

By the way, I used the opportunity to update a bit the example

  • with no more hardcoded pairs (clearer inputset, clearer samples for execution)
  • with an assert of the correct execution

I find this example easier to be reused for users (eg, me, each time, I apply this very same change). Is it ok with you?

@cla-bot cla-bot bot added the cla-signed label Feb 8, 2024
@bcm-at-zama
Copy link
Contributor Author

@BourgerieQuentin , @umut-sahin , you'll tell me if you're fine with these changes. And then, we'll have to handle the conflicts in the other PR by @yuxizama (#700)

@bcm-at-zama bcm-at-zama force-pushed the uniformize_examples_readme_and_quickstart_603 branch from fc7f6d3 to 34c8085 Compare February 8, 2024 11:01
@bcm-at-zama
Copy link
Contributor Author

Let me fix the build.

@bcm-at-zama
Copy link
Contributor Author

Note that we could squash the commits, I just made them separate for you to have a look to my changes in the example itself.

@bcm-at-zama
Copy link
Contributor Author

And we can make it reviewed by an English native speaker before pushing.

@bcm-at-zama bcm-at-zama force-pushed the uniformize_examples_readme_and_quickstart_603 branch 4 times, most recently from 8aa6dab to c910554 Compare February 8, 2024 13:19
@bcm-at-zama
Copy link
Contributor Author

Thanks @BourgerieQuentin for your feedback. Do you want to take the lead now on this PR, and change what you want? What we need at least is to uniformize the README and QuickStart. For the rest, you can do what you want. Just maybe wait that #700 is merged before merging this one.

@BourgerieQuentin BourgerieQuentin force-pushed the uniformize_examples_readme_and_quickstart_603 branch 3 times, most recently from b07013f to 503e6ed Compare February 19, 2024 17:11
@BourgerieQuentin
Copy link
Member

any feedback @bcm-at-zama @yuxizama ?

@bcm-at-zama
Copy link
Contributor Author

Thx, let me have a look

Copy link
Contributor Author

@bcm-at-zama bcm-at-zama left a comment

Choose a reason for hiding this comment

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

I really dislike the fact that it has been over simplified:

  • for newbies: it's not clear to me that they shouldn't hardcode their inputset; it's not clear that they should have long inputset
  • for developers: I use the README to have a template I can change; each time I'll have to re-add what I did previously in this PR, which is having random and with a chosen-bit width inputs.

But I guess we have to converge so I'll accept.

@bcm-at-zama
Copy link
Contributor Author

Let me make this build green.

@bcm-at-zama bcm-at-zama force-pushed the uniformize_examples_readme_and_quickstart_603 branch from 503e6ed to a4cf3a0 Compare March 8, 2024 10:00
@bcm-at-zama
Copy link
Contributor Author

Rebased and fixed lint

@bcm-at-zama
Copy link
Contributor Author

Build is green

@bcm-at-zama bcm-at-zama changed the title Uniformize examples readme and quickstart Uniformize examples readme and quickstart [ready for reviews] Mar 8, 2024
@bcm-at-zama bcm-at-zama force-pushed the uniformize_examples_readme_and_quickstart_603 branch from a4cf3a0 to 3ed0338 Compare March 15, 2024 19:26
@bcm-at-zama
Copy link
Contributor Author

Rebasing

@BourgerieQuentin BourgerieQuentin force-pushed the uniformize_examples_readme_and_quickstart_603 branch from 3ed0338 to c614b69 Compare April 15, 2024 07:38
@BourgerieQuentin BourgerieQuentin merged commit 2d36bb1 into main Apr 15, 2024
22 checks passed
@BourgerieQuentin BourgerieQuentin deleted the uniformize_examples_readme_and_quickstart_603 branch April 15, 2024 07:44
@BourgerieQuentin
Copy link
Member

pushed on release/v2.6.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants