Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Add Standard Error to Weight Template #7652

Merged
merged 4 commits into from
Dec 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .maintain/frame-weight-template.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
) -> Weight {
({{underscore benchmark.base_weight}} as Weight)
{{~#each benchmark.component_weight as |cw|}}
// Standard Error: {{underscore cw.error}}
.saturating_add(({{underscore cw.slope}} as Weight).saturating_mul({{cw.name}} as Weight))
{{~/each}}
{{~#if (ne benchmark.base_reads "0")}}
Expand Down Expand Up @@ -82,6 +83,7 @@ impl WeightInfo for () {
) -> Weight {
({{underscore benchmark.base_weight}} as Weight)
{{~#each benchmark.component_weight as |cw|}}
// Standard Error: {{underscore cw.error}}
.saturating_add(({{underscore cw.slope}} as Weight).saturating_mul({{cw.name}} as Weight))
{{~/each}}
{{~#if (ne benchmark.base_reads "0")}}
Expand Down
1 change: 1 addition & 0 deletions utils/frame/benchmarking-cli/src/template.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ impl<T: frame_system::Config> {{pallet}}::WeightInfo for WeightInfo<T> {
) -> Weight {
({{underscore benchmark.base_weight}} as Weight)
{{~#each benchmark.component_weight as |cw|}}
// Standard Error: {{underscore cw.error}}
.saturating_add(({{underscore cw.slope}} as Weight).saturating_mul({{cw.name}} as Weight))
{{~/each}}
{{~#if (ne benchmark.base_reads "0")}}
Expand Down
80 changes: 56 additions & 24 deletions utils/frame/benchmarking-cli/src/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ struct ComponentSlope {
name: String,
#[serde(serialize_with = "string_serialize")]
slope: u128,
#[serde(serialize_with = "string_serialize")]
error: u128,
}

// Small helper to create an `io::Error` from a string.
Expand Down Expand Up @@ -145,27 +147,45 @@ fn get_benchmark_data(batch: &BenchmarkBatch) -> BenchmarkData {
let mut used_reads = Vec::new();
let mut used_writes = Vec::new();

extrinsic_time.slopes.into_iter().zip(extrinsic_time.names.iter()).for_each(|(slope, name)| {
if !slope.is_zero() {
if !used_components.contains(&name) { used_components.push(name); }
used_extrinsic_time.push(ComponentSlope {
name: name.clone(),
slope: slope.saturating_mul(1000),
});
}
});
reads.slopes.into_iter().zip(reads.names.iter()).for_each(|(slope, name)| {
if !slope.is_zero() {
if !used_components.contains(&name) { used_components.push(name); }
used_reads.push(ComponentSlope { name: name.clone(), slope });
}
});
writes.slopes.into_iter().zip(writes.names.iter()).for_each(|(slope, name)| {
if !slope.is_zero() {
if !used_components.contains(&name) { used_components.push(name); }
used_writes.push(ComponentSlope { name: name.clone(), slope });
}
});
extrinsic_time.slopes.into_iter()
.zip(extrinsic_time.names.iter())
.zip(extrinsic_time.model.unwrap().se.regressor_values.iter())
.for_each(|((slope, name), error)| {
if !slope.is_zero() {
Copy link
Member

Choose a reason for hiding this comment

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

Ahh fuck :P This could have been a filter(). But yeah, not that important xD

Copy link
Member Author

Choose a reason for hiding this comment

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

Would it actually be better given that i need to use all the variables in the code block below?

I would need a filter statement on slope and a then statement looking at all the variables again?

Copy link
Member

Choose a reason for hiding this comment

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

You would just remove the if. Nothing else would change.

if !used_components.contains(&name) { used_components.push(name); }
used_extrinsic_time.push(ComponentSlope {
name: name.clone(),
slope: slope.saturating_mul(1000),
error: (*error as u128).saturating_mul(1000),
});
}
});
reads.slopes.into_iter()
.zip(reads.names.iter())
.zip(reads.model.unwrap().se.regressor_values.iter())
.for_each(|((slope, name), error)| {
if !slope.is_zero() {
if !used_components.contains(&name) { used_components.push(name); }
used_reads.push(ComponentSlope {
name: name.clone(),
slope,
error: *error as u128,
});
}
});
writes.slopes.into_iter()
.zip(writes.names.iter())
.zip(writes.model.unwrap().se.regressor_values.iter())
.for_each(|((slope, name), error)| {
if !slope.is_zero() {
if !used_components.contains(&name) { used_components.push(name); }
used_writes.push(ComponentSlope {
name: name.clone(),
slope,
error: *error as u128,
});
}
});

// This puts a marker on any component which is entirely unused in the weight formula.
let components = batch.results[0].components
Expand Down Expand Up @@ -379,18 +399,30 @@ mod test {
assert_eq!(benchmark.base_weight, base * 1_000);
assert_eq!(
benchmark.component_weight,
vec![ComponentSlope { name: component.to_string(), slope: slope * 1_000 }]
vec![ComponentSlope {
name: component.to_string(),
slope: slope * 1_000,
error: 0,
}]
);
// DB Reads/Writes are untouched
assert_eq!(benchmark.base_reads, base);
assert_eq!(
benchmark.component_reads,
vec![ComponentSlope { name: component.to_string(), slope: slope }]
vec![ComponentSlope {
name: component.to_string(),
slope,
error: 0,
}]
);
assert_eq!(benchmark.base_writes, base);
assert_eq!(
benchmark.component_writes,
vec![ComponentSlope { name: component.to_string(), slope: slope }]
vec![ComponentSlope {
name: component.to_string(),
slope,
error: 0,
}]
);
}

Expand Down