Full propa test #621
Full propa test #621
Conversation
|
@slab-ci cpu_fast_test |
tfhe/src/integer/server_key/radix_parallel/tests_cases_unsigned.rs
Outdated
Show resolved
Hide resolved
tfhe/src/integer/server_key/radix_parallel/tests_cases_unsigned.rs
Outdated
Show resolved
Hide resolved
tmontaigu
force-pushed
the
full-propa-test
branch
from
dfb4201 to
b11fd87
Compare
|
@slab-ci cpu_fast_test |
tmontaigu
force-pushed
the
full-propa-test
branch
from
b11fd87 to
61d8625
Compare
|
@slab-ci cpu_fast_test |
| ); | ||
| } | ||
|
|
||
| if block_carry_mod == block_msg_mod { |
There was a problem hiding this comment.
block_carry_mod >= block_msg_mod ? I don't know if we have such parameters though
There was a problem hiding this comment.
We have for shortints, but we don't use them in integers.
In integers params needs as many carry bit as message bits to have all operations
But if their are more carry bits than message bits, then we would need 2 full_propagation to actually fully propagate.
There was a problem hiding this comment.
ok so this is correct with our current implementation of full propagate for integers right ?
There was a problem hiding this comment.
Ok Finally plugged my brain,
So using params where block_carry_mod >= block_msg_mod, the full_propagations (both sequential and parallel)
will correctly work, so as long as the block has enough space to receive the carry of the next block.
Which is what is_thing_possible were reworked to ensure (there is probably some is_scalar_thing_possible left to update).
What confused me, is that the parallel carry propagation also works by default even in the case where the whole block is completely full.
For example, in 2_2, the sequential propagation requires that there is enough room to receive worst case carry of preceding block, so max degree is 15 - 3 = 12. Using the parallel algorithm we can propagate carries where blocks
have a degree of 15 (ie degree == max_degree), however that's only true if carry_modulus <= message_modulus.
Parallel propagation could be improved to support the case where carry_modulus > message_modulus and we sum until degree = (message_modulus * message_modulus)- 1.
There was a problem hiding this comment.
I'm adding PARAM_MESAGE_2_CARRY_3 in the instanciated tests
| assert!(ct | ||
| .blocks | ||
| .iter() | ||
| .all(|b| b.degree.0 as u64 == (block_total_mod - 1) - (block_msg_mod - 1)),); |
There was a problem hiding this comment.
Yes I think (block_carry_mod -1) is the correct thing if we want to take into account block_carry_mod >= block_msg_mod, but then In that case our full_propagate won't be a real full_propagate and so this test won't be correct
There was a problem hiding this comment.
Not sure about that, need to think about it 🤔
There was a problem hiding this comment.
Ok so in the case where params are such that block_carry_mod > block_msg_mod, the sequential propagation is fine as long as is_op_possible do proper checks.
The parallel carry propagation would require to be adapted to support that case
There was a problem hiding this comment.
feel free to open follow up issues
|
looks good, just the minor comments for which I may be wrong |
tmontaigu
force-pushed
the
full-propa-test
branch
from
61d8625 to
e34f65f
Compare
|
@slab-ci cpu_fast_test |
|
Pull Request has been approved 🎉 |
|
@slab-ci cpu_test |
|
@slab-ci cpu_integer_test |
tmontaigu
force-pushed
the
full-propa-test
branch
from
e34f65f to
b704c2f
Compare
|
@slab-ci cpu_fast_test |
|
Realized that the test was not meaningful as it did not check manually each block to verify its value (e.g no carry after propagation). Checking the result of the decryption is not a complete check, because decrypting propagtes carries, so an implemenation that did nothing would pass the tests |
tmontaigu
force-pushed
the
full-propa-test
branch
from
b704c2f to
2f5ba9f
Compare
|
@slab-ci cpu_fast_test |
|
Pull Request has been approved 🎉 |
|
@slab-ci cpu_integer_test |
1 similar comment
|
@slab-ci cpu_integer_test |
Check-list:
5