Should `integer::CompressedServerKey` have `decompress` method?
matthiasgeihs opened this issue Β· 6 comments
Describe the bug
Not necessarily a bug description, but an observation and a corresponding question.
In the documentation, compressed server key decompression is performed via:
let sks = compressed_sks.decompress();
However, the decompress
method is only available on the high level API.
For example, decompress
is not available on type integer::CompressedServerKey
.
Is this intentional?
To Reproduce
use tfhe::integer::{gen_keys_radix, CompressedServerKey};
use tfhe::shortint::prelude::PARAM_MESSAGE_2_CARRY_2_KS_PBS;
use tfhe::shortint::ClassicPBSParameters;
pub fn main() {
const PARAMS: ClassicPBSParameters = PARAM_MESSAGE_2_CARRY_2_KS_PBS;
const NUM_BLOCKS: usize = 4;
let (ck, sk) = gen_keys_radix(PARAMS, NUM_BLOCKS);
let sk_compressed = CompressedServerKey::new_radix_compressed_server_key(&ck.into());
let sk = sk_compressed.decompress(); // no method named `decompress` found for struct `tfhe::integer::CompressedServerKey` in the current scope
}
(Also it's a bit surprising that we need to use ck.into()
in combination with new_radix_compressed_server_key
even though ck
is already a radix key.)
Expected behaviour
Should compile.
Additional context
tfhe = { version = "0.5.0" }
Hello @matthiasgeihs
Some of the APIs are still work in progress, your remark about the decompression primitive is valid and we will add it for sure, we tried to use the Into trait from rust for some of those operations, but it seems it's not discoverable enough.
As for the "into" question, we implement the AsRef trait which should make things a bit less confusing, we have two types of integers and the CRT ones were added late in the original integer design, leaving some API in a subpar state, AsRef is likely the way we will give access to underlying primitives moving forward.
If you think the documentation could be improved we are open source and will gladly take suggestions or contributions once we agree on the format of the documentation π
here is your example revisited:
use tfhe::integer::{CompressedServerKey, RadixClientKey, ServerKey};
use tfhe::shortint::prelude::PARAM_MESSAGE_2_CARRY_2_KS_PBS;
use tfhe::shortint::ClassicPBSParameters;
pub fn main() {
const PARAMS: ClassicPBSParameters = PARAM_MESSAGE_2_CARRY_2_KS_PBS;
const NUM_BLOCKS: usize = 4;
let ck = RadixClientKey::new(PARAM_MESSAGE_2_CARRY_2_KS_PBS, NUM_BLOCKS);
let sk_compressed = CompressedServerKey::new_radix_compressed_server_key(ck.as_ref());
let sk: ServerKey = sk_compressed.into(); // no method named `decompress` found for struct
// `tfhe::integer::CompressedServerKey` in the current
// scope
}
Cheers
Cool. Thx. Looks clean, although, as you also said, I find that into conversion and as_ref are less discoverable compared to explicit functions. Anyhow. Thanks for the quick feedback as usual and good to hear that you are aware and potentially reiterating in the future. π
of course, we are trying to find a reasonable middle ground between "going fast with features" and "keep features easy to use/discoverable" generally the HL API is the one getting the most attention in that respect !
generally the HL API is the one getting the most attention in that respect !
Good to know. I feel like in the documentation that does not come through all the time. Only in the quick start and the How To the HL API gets some attention but then not anymore in fine grained APIs. I guess thatβs intentional, but as a developer I would usually look up the fundamentals first (i.e., api reference) and then reside to how to guides for higher level concepts if needed.
thanks that's valuable feedback
Maybe it's just me though π π€·ββοΈ Overall I think the quality of the documentation is already very good π Cheers!