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

refactor!: use strong types for outputs of DispersionParameters trait fns #1845

Merged
merged 1 commit into from
Dec 5, 2024

Conversation

IceTDrinker
Copy link
Member

No description provided.

@cla-bot cla-bot bot added the cla-signed label Dec 3, 2024
@IceTDrinker IceTDrinker force-pushed the am/refactor/dispersion-strong-types branch from 33a56ef to c731615 Compare December 3, 2024 18:13
@IceTDrinker
Copy link
Member Author

this is part of the clean up tasks we wanted to do, I'm working around those so did the refactor here

@@ -93,7 +93,7 @@ fn lwe_compact_public_key_encryption_expected_variance(
lwe_dimension: LweDimension,
) -> Variance {
let input_variance = input_noise.get_variance();
Variance(input_variance * (lwe_dimension.to_lwe_size().0 as f64))
Variance(input_variance.0 * (lwe_dimension.to_lwe_size().0 as f64))
Copy link
Contributor

Choose a reason for hiding this comment

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

We could have a Mul<Rhs = usize> for 'Variance'

Copy link
Member Author

Choose a reason for hiding this comment

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

I did not want to update the usability, just that we return strong types, this can be done in a follow up

Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -25,7 +25,7 @@ impl Gaussian<f64> {

pub fn from_dispersion_parameter(dispersion: impl DispersionParameter, mean: f64) -> Self {
Self {
std: dispersion.get_standard_dev(),
std: dispersion.get_standard_dev().0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could remove the generic and make std a StandardDev

Copy link
Member Author

Choose a reason for hiding this comment

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

not really as it would mean making StandardDev a generic, so I did not touch the design, just updated the functions to be more type safe

Copy link
Contributor

@mayeul-zama mayeul-zama left a comment

Choose a reason for hiding this comment

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

Thanks!

@IceTDrinker IceTDrinker merged commit f24fa62 into main Dec 5, 2024
110 checks passed
@IceTDrinker IceTDrinker deleted the am/refactor/dispersion-strong-types branch December 5, 2024 10:19
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