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

fix/improve docs #540

Merged
merged 4 commits into from
Sep 18, 2023
Merged

fix/improve docs #540

merged 4 commits into from
Sep 18, 2023

Conversation

mayeul-zama
Copy link
Contributor

No description provided.

@github-actions
Copy link

@slab-ci cpu_fast_test

tfhe/docs/application_tutorials/dark_market.md Outdated Show resolved Hide resolved
tfhe/docs/fine_grained_api/Boolean/readme.md Show resolved Hide resolved
tfhe/docs/how_to/parallelized_pbs.md Outdated Show resolved Hide resolved
tfhe/docs/how_to/parallelized_pbs.md Outdated Show resolved Hide resolved
tfhe/docs/tutorials/ascii_fhe_string.md Outdated Show resolved Hide resolved
@@ -0,0 +1,111 @@
use std::time::Instant;
Copy link
Member

Choose a reason for hiding this comment

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

dark market changes are just file reorgs ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the second commit, yes, its mostly file reorgs plus closure converted to static functions for readability
The last commit is a formula change for the improved parallelized version

Copy link
Member

Choose a reason for hiding this comment

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

did you check the updated function works as expected ?

Copy link
Contributor Author

@mayeul-zama mayeul-zama Sep 13, 2023

Choose a reason for hiding this comment

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

Yes (expect for the last test case which is very slow), I just had to fix an underflow which got caught in devo profile

@github-actions
Copy link

@slab-ci cpu_fast_test

@github-actions
Copy link

@slab-ci cpu_fast_test

@github-actions
Copy link

@slab-ci cpu_fast_test

@github-actions
Copy link

@slab-ci cpu_fast_test

@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Merging #540 (a0fb04b) into main (ed83fbb) will increase coverage by 0.16%.
Report is 6 commits behind head on main.
The diff coverage is n/a.

❗ Current head a0fb04b differs from pull request most recent head 065d415. Consider uploading reports for the commit 065d415 to get more accurate results

@@            Coverage Diff             @@
##             main     #540      +/-   ##
==========================================
+ Coverage   23.11%   23.28%   +0.16%     
==========================================
  Files         256      255       -1     
  Lines       19573    19536      -37     
==========================================
+ Hits         4525     4549      +24     
+ Misses      15048    14987      -61     

see 18 files with indirect coverage changes

@github-actions
Copy link

@slab-ci cpu_fast_test

@@ -3,6 +3,7 @@ TFHE-rs includes features to reduce the size of both keys and ciphertexts, by co

In the library, entities that can be compressed are prefixed by `Compressed`. For instance, the type of a compressed `FheUint256` is `CompressedFheUint256`.

In the following example code, we use the `bincode` crate dependency to compare serialized sizes.
Copy link
Contributor

Choose a reason for hiding this comment

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

phrasing may make it seems like bincode only serves too compare serialized sizes,

In the following example code, we use the `bincode` crate dependency to serialize in a binary format and compare serialized sizes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

tfhe/docs/how_to/compress.md Outdated Show resolved Hide resolved
tfhe/docs/how_to/compress.md Outdated Show resolved Hide resolved
tfhe/docs/how_to/compress.md Outdated Show resolved Hide resolved
tfhe/docs/tutorials/ascii_fhe_string.md Outdated Show resolved Hide resolved
@github-actions
Copy link

@slab-ci cpu_fast_test

tfhe/docs/application_tutorials/dark_market.md Outdated Show resolved Hide resolved
tfhe/docs/tutorials/ascii_fhe_string.md Outdated Show resolved Hide resolved
tfhe/docs/tutorials/ascii_fhe_string.md Outdated Show resolved Hide resolved

* The input string can only contain ascii characters.

It is not possible to create branches where the condition if an encrypted integer, meaning the function cannot use conditional statements.
Copy link
Member

Choose a reason for hiding this comment

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

where the condition if ? the sentence looks odd

also now we have if_then_else that does basically what the code describe, maybe the correct way of writing it is to say "it is not possible to branch on an encrypted value, however it is possible to evaluate a boolean condition and output the proper result" or something along those lines ?

follow up issue for updating the code to use if_then_else

https://github.com/zama-ai/tfhe-rs-internal/issues/236

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rephrased

@github-actions
Copy link

@slab-ci cpu_fast_test
@slab-ci code_coverage

@github-actions
Copy link

Pull Request has been approved 🎉
Launching full test suite...
@slab-ci cpu_test
@slab-ci cpu_integer_test
@slab-ci cpu_multi_bit_test
@slab-ci cpu_wasm_test
@slab-ci csprng_randomness_testing

@IceTDrinker
Copy link
Member

fine to merge if you had time to check the updated code is valid

@mayeul-zama
Copy link
Contributor Author

fine to merge if you had time to check the updated code is valid

Yes, I tested it manually in a AWS instance

@mayeul-zama mayeul-zama merged commit b553a68 into main Sep 18, 2023
30 checks passed
@mayeul-zama mayeul-zama deleted the mz/docs/improve branch September 18, 2023 07:58
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.

3 participants