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

doc(tfhe): add dark market example #188

Merged
merged 3 commits into from
May 24, 2023

Conversation

yagizsenal
Copy link
Contributor

This pull request solves the Dark Market bounty.

Two file change has been made and two new files are added to the repository.

  1. tfhe/Cargo.toml file has been updated to include the new example.
  2. tfhe/docs/SUMMARY.md file has been changed to include links to the added Dark Market tutorial.
  3. tfhe/docs/tutorial/dark_market.md file is added to describe how to implement the Dark Market algorithm with TFHE-rs.
  4. tfhe/examples/dark_market.rs file is added that consists of three different implementations of the Dark Market algorithm: Plain, FHE and Parallelized FHE. These implementations can be run with the commands given in the tutorial.

Thank you so much

Yagiz

@cla-bot
Copy link

cla-bot bot commented Apr 7, 2023

Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @yagizsenal on file. In order for us to review and merge your code, please send an email to [email protected] to get yourself added

Copy link
Member

@IceTDrinker IceTDrinker left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your bounty contribution!

There is a small addition we would want to make in the tutorial.

For the code there may be a way to showcase one of TFHE-rs tree-like reduction (though you used the one from rayon) and there may be a trick to speed up the fill order routine by potentially doing more work but exploiting more threads on an m6i.metal AWS instance, the machine has 64 CPUS and 128 threads and 500 Gigs of ram

My suggestion would be to explore the proposed alternative approach (creating a dedicated function in your code) on a small use case for your dev machine (which has a lower amount of thread to see if the implementation works ok) and then we can bench that on the m6i.metal to see if we are gaining for the overall computation time.

Cheers

tfhe/docs/SUMMARY.md Outdated Show resolved Hide resolved
tfhe/docs/tutorial/dark_market.md Outdated Show resolved Hide resolved
tfhe/examples/dark_market.rs Show resolved Hide resolved
tfhe/examples/dark_market.rs Outdated Show resolved Hide resolved
tfhe/examples/dark_market.rs Outdated Show resolved Hide resolved
@cla-bot
Copy link

cla-bot bot commented May 4, 2023

Thank you for your pull request. We require contributors to sign our Contributor License Agreement / Terms and Conditions, and we don't seem to have the users @yagizsenal on file. In order for us to review and merge your code, please sign:

  • For individual contribution: our CLA
  • for Bounty submission, if you are an individual: our T&C
  • for Bounty submission, if you are a company: our T&C
    to get yourself added.

If you already signed one of this document, just wait to be added to the bot config.

@cla-bot
Copy link

cla-bot bot commented May 4, 2023

Thank you for your pull request. We require contributors to sign our Contributor License Agreement / Terms and Conditions, and we don't seem to have the users @yagizsenal on file. In order for us to review and merge your code, please sign:

  • For individual contribution: our CLA
  • for Bounty submission, if you are an individual: our T&C
  • for Bounty submission, if you are a company: our T&C
    to get yourself added.

If you already signed one of this document, just wait to be added to the bot config.

@aquint-zama
Copy link
Contributor

@yagizsenal have you signed the T&C for individual contributor?

@IceTDrinker
Copy link
Member

first request for this PR: squash in a single commit and follow the commit message format, in that case it should be

docs(tfhe): add dark market tutorial

(... more details)

@cla-bot
Copy link

cla-bot bot commented May 10, 2023

Thank you for your pull request. We require contributors to sign our Contributor License Agreement / Terms and Conditions, and we don't seem to have the users @yagizsenal on file. In order for us to review and merge your code, please sign:

  • For individual contribution: our CLA
  • for Bounty submission, if you are an individual: our T&C
  • for Bounty submission, if you are a company: our T&C
    to get yourself added.

If you already signed one of this document, just wait to be added to the bot config.

@yagizsenal
Copy link
Contributor Author

I just signed CLA and T&C. I also updated the commit message.

@IceTDrinker
Copy link
Member

I just signed CLA and T&C. I also updated the commit message.

ok, we should have the updated infos for the cla bot soon, thanks

Copy link
Member

@IceTDrinker IceTDrinker left a comment

Choose a reason for hiding this comment

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

Thanks a few more detailed comments otherwise this looks very good.

We'll probably manage the location in our docs ourselves to make it easier and avoid additional back and forth on this PR.

Cheers!

tfhe/docs/tutorial/dark_market.md Outdated Show resolved Hide resolved
tfhe/docs/tutorial/dark_market.md Outdated Show resolved Hide resolved
tfhe/docs/tutorial/dark_market.md Outdated Show resolved Hide resolved
tfhe/docs/tutorial/dark_market.md Outdated Show resolved Hide resolved
@aquint-zama
Copy link
Contributor

@cla-bot check

@cla-bot cla-bot bot added the cla-signed label May 11, 2023
@cla-bot
Copy link

cla-bot bot commented May 11, 2023

The cla-bot has been summoned, and re-checked this pull request!

@yagizsenal
Copy link
Contributor Author

Thanks, updated accordingly.

@aquint-zama
Copy link
Contributor

close zama-ai/bounty-program#40

@soonum soonum requested a review from tmontaigu May 24, 2023 15:14
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.

5 participants