shortint/small optim #536

mayeul-zama · 2023-09-11T12:00:33Z

cleanup inputs of smart ops instead of cleanup clones
do not bootstrap when not necessary
add asserts on smart ops

github-actions · 2023-09-11T12:00:45Z

@slab-ci cpu_fast_test

tfhe/src/shortint/engine/server_side/mod.rs

tfhe/src/shortint/engine/server_side/shift.rs

github-actions · 2023-09-12T11:31:27Z

@slab-ci cpu_fast_test

github-actions · 2023-09-13T07:48:37Z

@slab-ci cpu_fast_test

codecov · 2023-09-13T08:28:39Z

Codecov Report

Merging #536 (79eeb0d) into main (ed83fbb) will decrease coverage by 0.19%.
Report is 61 commits behind head on main.
The diff coverage is 35.23%.

❗ Current head 79eeb0d differs from pull request most recent head aa7b393. Consider uploading reports for the commit aa7b393 to get more accurate results

@@            Coverage Diff             @@
##             main     #536      +/-   ##
==========================================
- Coverage   23.11%   22.93%   -0.19%     
==========================================
  Files         256      256              
  Lines       19573    19633      +60     
==========================================
- Hits         4525     4503      -22     
- Misses      15048    15130      +82

Files	Coverage Δ
tfhe/src/integer/server_key/radix/scalar_mul.rs	`0.00% <ø> (ø)`
...rc/integer/server_key/radix_parallel/scalar_mul.rs	`0.00% <ø> (ø)`
tfhe/src/shortint/server_key/mod.rs	`31.62% <ø> (ø)`
tfhe/src/shortint/engine/server_side/neg.rs	`81.25% <50.00%> (-2.09%)`	⬇️
tfhe/src/integer/server_key/radix/scalar_add.rs	`0.00% <0.00%> (ø)`
tfhe/src/integer/server_key/radix/scalar_sub.rs	`0.00% <0.00%> (ø)`
tfhe/src/integer/server_key/radix_parallel/add.rs	`0.00% <0.00%> (ø)`
...rc/integer/server_key/radix_parallel/scalar_add.rs	`0.00% <0.00%> (ø)`
...rc/integer/server_key/radix_parallel/scalar_sub.rs	`0.00% <0.00%> (ø)`
tfhe/src/shortint/engine/server_side/comp_op.rs	`79.87% <83.33%> (-1.38%)`	⬇️
... and 7 more

... and 12 files with indirect coverage changes

github-actions · 2023-09-13T12:27:19Z

@slab-ci cpu_fast_test

github-actions · 2023-09-14T10:04:16Z

@slab-ci cpu_fast_test

mayeul-zama · 2023-09-14T14:10:26Z

@slab-ci cpu_fast_test

IceTDrinker

Not sure about the pattern where we check if the operation is possible, clean the ciphertexts and then assert right after 🤔

IceTDrinker · 2023-09-18T07:57:44Z

tfhe/src/integer/server_key/crt/scalar_mul_crt.rs

@@ -176,15 +172,14 @@ impl ServerKey {
        if !self.is_crt_scalar_mul_possible(ctxt, scalar) {
            self.full_extract_message_assign(ctxt);
        }
+
+        // assert!(self.is_crt_scalar_mul_possible(ctxt, scalar));


what is breaking this ?

IceTDrinker · 2023-09-18T08:00:20Z

tfhe/src/integer/server_key/radix/scalar_mul.rs

@@ -255,6 +255,7 @@ impl ServerKey {
        if !self.is_small_scalar_mul_possible(ctxt, scalar) {
            self.full_propagate(ctxt);
        }
+        // assert!(self.is_small_scalar_mul_possible(ctxt, scalar));


same question everywhere, why is it commented here and in this case even more so given it's not commented below

github-actions · 2023-09-20T08:24:18Z

@slab-ci cpu_fast_test

mayeul-zama · 2023-09-20T08:56:56Z

@slab-ci cpu_fast_test

github-actions · 2023-09-20T12:56:06Z

@slab-ci cpu_fast_test

github-actions · 2023-09-22T14:20:16Z

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 · 2023-09-22T20:42:55Z

that one https://github.com/zama-ai/tfhe-rs/actions/runs/6275470013/job/17043013270#step:8:3202

is a hard coded test that can be updated safely and was a sanity check with the previous accpeted degree value (though we knew it was broken, and needed updating later)

…riate

github-actions · 2023-09-26T14:25:18Z

@slab-ci cpu_fast_test

github-actions · 2023-09-27T06:44:38Z

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 · 2023-09-27T07:36:44Z

@slab-ci csprng_randomness_testing

github-actions · 2023-09-27T14:02:09Z

@slab-ci cpu_fast_test

mayeul-zama · 2023-09-27T14:02:33Z

Moved add assert commit to PR #590

github-actions · 2023-10-02T15:32:38Z

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

cla-bot bot added the cla-signed label Sep 11, 2023

mayeul-zama requested review from IceTDrinker and tmontaigu September 11, 2023 12:01

IceTDrinker reviewed Sep 11, 2023

View reviewed changes

tfhe/src/shortint/engine/server_side/mod.rs Outdated Show resolved Hide resolved

tmontaigu reviewed Sep 11, 2023

View reviewed changes

tfhe/src/shortint/engine/server_side/shift.rs Outdated Show resolved Hide resolved

mayeul-zama mentioned this pull request Sep 11, 2023

Clean smart ops #516

Closed

mayeul-zama force-pushed the mz/refactor/cleanup_shortint branch 3 times, most recently from 57ab09d to 3f13028 Compare September 12, 2023 11:30

mayeul-zama force-pushed the mz/shortint/small_optim branch from 168374f to f116b83 Compare September 12, 2023 11:31

mayeul-zama force-pushed the mz/refactor/cleanup_shortint branch from 3f13028 to e1e5822 Compare September 12, 2023 11:59

Base automatically changed from mz/refactor/cleanup_shortint to main September 13, 2023 07:44

mayeul-zama force-pushed the mz/shortint/small_optim branch from f116b83 to 4cf5b56 Compare September 13, 2023 07:48

mayeul-zama force-pushed the mz/shortint/small_optim branch from 4cf5b56 to 7316e86 Compare September 13, 2023 12:27

mayeul-zama force-pushed the mz/shortint/small_optim branch from 7316e86 to 79eeb0d Compare September 14, 2023 10:04

mayeul-zama requested review from tmontaigu and IceTDrinker September 14, 2023 16:08

IceTDrinker reviewed Sep 18, 2023

View reviewed changes

mayeul-zama force-pushed the mz/shortint/small_optim branch from 79eeb0d to 5293645 Compare September 20, 2023 08:24

mayeul-zama force-pushed the mz/shortint/small_optim branch from 5293645 to 79f5c50 Compare September 20, 2023 12:55

tmontaigu approved these changes Sep 22, 2023

View reviewed changes

github-actions bot added the approved label Sep 22, 2023

tmontaigu mentioned this pull request Sep 22, 2023

fix(integer): is_neg/sub/add possible #581

Merged

mayeul-zama added 2 commits September 26, 2023 16:23

feat(shortint): cleanup input if necessary in apply_lookup_table_biva…

10d1133

…riate

feat(shortint): cleanup input if necessary in ops

de470ad

mayeul-zama force-pushed the mz/shortint/small_optim branch from c98514d to b0e5dbe Compare September 26, 2023 14:25

github-actions bot removed the approved label Sep 26, 2023

mayeul-zama requested a review from tmontaigu September 26, 2023 15:39

tmontaigu approved these changes Sep 27, 2023

View reviewed changes

github-actions bot added the approved label Sep 27, 2023

mayeul-zama added 5 commits September 27, 2023 15:56

style(shortint): do not use assign ops on a cloned input

bffa4b5

fix(integer): fix small mul test

04e5a6f

style(crt): compute modulus from base in tests

fb94366

fix(crt): use 3_3 parameters for crt tests

89ad20e

fix(integer): fix max_degree formula

aa7b393

mayeul-zama force-pushed the mz/shortint/small_optim branch from b0e5dbe to aa7b393 Compare September 27, 2023 14:01

github-actions bot removed the approved label Sep 27, 2023

mayeul-zama requested a review from jborfila September 27, 2023 14:06

tmontaigu approved these changes Oct 2, 2023

View reviewed changes

github-actions bot added the approved label Oct 2, 2023

mayeul-zama merged commit f08ea8c into main Oct 17, 2023
20 checks passed

mayeul-zama deleted the mz/shortint/small_optim branch October 17, 2023 09:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

shortint/small optim #536

shortint/small optim #536

mayeul-zama commented Sep 11, 2023

github-actions bot commented Sep 11, 2023

github-actions bot commented Sep 12, 2023

github-actions bot commented Sep 13, 2023

codecov bot commented Sep 13, 2023 •

edited

Loading

github-actions bot commented Sep 13, 2023

github-actions bot commented Sep 14, 2023

mayeul-zama commented Sep 14, 2023

IceTDrinker left a comment

IceTDrinker Sep 18, 2023

IceTDrinker Sep 18, 2023

github-actions bot commented Sep 20, 2023

mayeul-zama commented Sep 20, 2023

github-actions bot commented Sep 20, 2023

github-actions bot commented Sep 22, 2023

IceTDrinker commented Sep 22, 2023

github-actions bot commented Sep 26, 2023

github-actions bot commented Sep 27, 2023

IceTDrinker commented Sep 27, 2023

github-actions bot commented Sep 27, 2023

mayeul-zama commented Sep 27, 2023

github-actions bot commented Oct 2, 2023

shortint/small optim #536

shortint/small optim #536

Conversation

mayeul-zama commented Sep 11, 2023

github-actions bot commented Sep 11, 2023

github-actions bot commented Sep 12, 2023

github-actions bot commented Sep 13, 2023

codecov bot commented Sep 13, 2023 • edited Loading

Codecov Report

github-actions bot commented Sep 13, 2023

github-actions bot commented Sep 14, 2023

mayeul-zama commented Sep 14, 2023

IceTDrinker left a comment

Choose a reason for hiding this comment

IceTDrinker Sep 18, 2023

Choose a reason for hiding this comment

IceTDrinker Sep 18, 2023

Choose a reason for hiding this comment

github-actions bot commented Sep 20, 2023

mayeul-zama commented Sep 20, 2023

github-actions bot commented Sep 20, 2023

github-actions bot commented Sep 22, 2023

IceTDrinker commented Sep 22, 2023

github-actions bot commented Sep 26, 2023

github-actions bot commented Sep 27, 2023

IceTDrinker commented Sep 27, 2023

github-actions bot commented Sep 27, 2023

mayeul-zama commented Sep 27, 2023

github-actions bot commented Oct 2, 2023

codecov bot commented Sep 13, 2023 •

edited

Loading