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(optimizer): fix leveled noise propagation #933

Merged
merged 1 commit into from
Jul 23, 2024
Merged

Conversation

aPere3
Copy link
Collaborator

@aPere3 aPere3 commented Jul 8, 2024

No description provided.

@cla-bot cla-bot bot added the cla-signed label Jul 8, 2024
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Benchmark

Benchmark suite Current: af7d986 Previous: dd8cb06 Ratio
v0 PBS table generation 60049852 ns/iter (± 376730) 60058969 ns/iter (± 940607) 1.00
v0 PBS simulate dag table generation 37623699 ns/iter (± 443648) 37806462 ns/iter (± 292750) 1.00
v0 WoP-PBS table generation 68694502 ns/iter (± 2861756) 68533645 ns/iter (± 388289) 1.00

This comment was automatically generated by workflow using github-action-benchmark.

@aPere3 aPere3 force-pushed the alex/fix_leveled_noise branch 2 times, most recently from 376a65d to e6912b4 Compare July 9, 2024 15:06
@aPere3 aPere3 requested a review from rudy-6-4 July 15, 2024 07:41
@aPere3 aPere3 force-pushed the alex/fix_leveled_noise branch 3 times, most recently from a0eae73 to 4ab336f Compare July 15, 2024 13:49
@aPere3 aPere3 force-pushed the alex/fix_leveled_noise branch 2 times, most recently from b3d2823 to dd8cb06 Compare July 22, 2024 12:36
@aPere3 aPere3 force-pushed the alex/fix_leveled_noise branch from dd8cb06 to af7d986 Compare July 22, 2024 13:15
@aPere3 aPere3 merged commit 2aeec5c into main Jul 23, 2024
35 of 38 checks passed
@aPere3 aPere3 deleted the alex/fix_leveled_noise branch July 23, 2024 07:40
Copy link
Member

@BourgerieQuentin BourgerieQuentin left a comment

Choose a reason for hiding this comment

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

I'm not sure about the conversion between the manp and the average weight for levelled op, it feels to me it's not always right

Comment on lines -147 to -178
#[allow(clippy::float_cmp)]
pub fn after_levelled_op(&self, manp: f64) -> Self {
let new_coeff = manp * manp;
// detect the previous base manp level
// this is the maximum value of fresh base noise and pbs base noise
let mut current_max: f64 = 0.0;
for partition in PartitionIndex::range(0, self.nb_partitions()) {
let fresh_coeff = self.coeff_input(partition);
let pbs_noise_coeff = self.coeff_pbs(partition);
current_max = current_max.max(fresh_coeff).max(pbs_noise_coeff);
}
// assert!(1.0 <= current_max);
// assert!(
// current_max <= new_coeff,
// "Non monotonious levelled op: {current_max} <= {new_coeff}"
// );
// replace all current_max by new_coeff
// multiply everything else by new_coeff / current_max
let mut new = self.clone();
if current_max == 0.0 {
return new;
}
for cell in &mut new.coeffs.values {
if *cell == current_max {
*cell = new_coeff;
} else {
*cell *= new_coeff / current_max;
}
}
new
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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