Skip to content

Commit

Permalink
feat(decentralization): Provide more details on the node replacement …
Browse files Browse the repository at this point in the history 
…proposals (#816)

Co-authored-by: sa-github-api <138766536+sa-github-api@users.noreply.github.com>
  • Loading branch information
@sasa-tomic @sa-github-api
sasa-tomic and sa-github-api committed Aug 28, 2024
1 parent 3752142 commit c161563
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 42 deletions.
6 changes: 1 addition & 5 deletions rs/cli/src/runner.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -729,11 +729,7 @@ pub fn replace_proposal_options(change: &SubnetChangeResponse) -> anyhow::Result
Ok(ic_admin::ProposeOptions {
title: format!("Replace {replace_target} in subnet {subnet_id_short}",).into(),
summary: format!("# Replace {replace_target} in subnet {subnet_id_short}",).into(),
motivation: Some(format!(
"{}\n\n```\n{}\n```\n",
change.motivation.as_ref().unwrap_or(&String::new()),
change
)),
motivation: Some(format!("{}\n\n{}\n", change.motivation.as_ref().unwrap_or(&String::new()), change)),
})
}

Expand Down
14 changes: 7 additions & 7 deletions rs/decentralization/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl Display for SubnetChangeResponse {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
writeln!(
f,
"Decentralization Nakamoto coefficient changes for subnet {}:\n",
"Decentralization Nakamoto coefficient changes for subnet `{}`:\n```",
self.subnet_id.unwrap_or_default()
)?;
let before_individual = self.score_before.scores_individual();
Expand All @@ -105,14 +105,14 @@ impl Display for SubnetChangeResponse {
let total_before = self.score_before.score_avg_linear();
let total_after = self.score_after.score_avg_linear();
let output = format!(
"**Mean Nakamoto comparison:** {:.2} -> {:.2} ({:+.0}%)\n\nOverall replacement impact: {}",
"\n**Mean Nakamoto comparison:** {:.2} -> {:.2} ({:+.0}%)\n\nOverall replacement impact: {}",
total_before,
total_after,
((total_after - total_before) / total_before) * 100.,
self.score_after.describe_difference_from(&self.score_before).1
);

writeln!(f, "\n{}\n\n# Details\n\n", output)?;
writeln!(f, "```\n{}\n\n# Details\n\n", output)?;

writeln!(f, "Nodes removed:")?;
for (id, desc) in &self.removed_with_desc {
Expand All @@ -121,7 +121,7 @@ impl Display for SubnetChangeResponse {
.get(id)
.map(|h| h.to_string().to_lowercase())
.unwrap_or("unknown".to_string());
writeln!(f, " --> {} [health: {}, impact on decentralization: {}]", id, health, desc).expect("write failed");
writeln!(f, "- `{}` [health: {}, impact on decentralization: {}]", id, health, desc).expect("write failed");
}
writeln!(f, "\nNodes added:")?;
for (id, desc) in &self.added_with_desc {
Expand All @@ -130,7 +130,7 @@ impl Display for SubnetChangeResponse {
.get(id)
.map(|h| h.to_string().to_lowercase())
.unwrap_or("unknown".to_string());
writeln!(f, " ++> {} [health: {}, impact on decentralization: {}]", id, health, desc).expect("write failed");
writeln!(f, "- `{}` [health: {}, impact on decentralization: {}]", id, health, desc).expect("write failed");
}

let rows = self.feature_diff.values().map(|diff| diff.len()).max().unwrap_or(0);
Expand Down Expand Up @@ -166,10 +166,10 @@ impl Display for SubnetChangeResponse {
}));
}

writeln!(f, "\n\n{}", table)?;
writeln!(f, "\n\n```\n{}```\n", table)?;

if let Some(comment) = &self.comment {
writeln!(f, "*** Note ***\n{}", comment)?;
writeln!(f, "### Business rules analysis\n{}", comment)?;
}

Ok(())
Expand Down
95 changes: 74 additions & 21 deletions rs/decentralization/src/nakamoto/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,11 +348,19 @@ impl NakamotoScore {
if cmp != Some(Ordering::Equal) {
return (
cmp,
format!(
"the minimum Nakamoto coefficient across all features changes from {} to {}",
other.score_min(),
self.score_min()
),
if cmp == Some(Ordering::Less) {
format!(
"(gets worse) the minimum Nakamoto coefficient across all features decreases from {} to {}",
other.score_min(),
self.score_min()
)
} else {
format!(
"(gets better) the minimum Nakamoto coefficient across all features increases from {} to {}",
other.score_min(),
self.score_min()
)
},
);
}

Expand All @@ -362,11 +370,19 @@ impl NakamotoScore {
if cmp != Some(Ordering::Equal) {
return (
cmp,
format!(
"the average log2 of Nakamoto Coefficients across all features changes from {:.4} to {:.4}",
other.score_avg_log2().unwrap_or(0.0),
self.score_avg_log2().unwrap_or(0.0)
),
if cmp == Some(Ordering::Less) {
format!(
"(gets worse) the average log2 of Nakamoto Coefficients across all features decreases from {:.4} to {:.4}",
other.score_avg_log2().unwrap_or(0.0),
self.score_avg_log2().unwrap_or(0.0)
)
} else {
format!(
"(gets better) the average log2 of Nakamoto Coefficients across all features increases from {:.4} to {:.4}",
other.score_avg_log2().unwrap_or(0.0),
self.score_avg_log2().unwrap_or(0.0)
)
},
);
}

Expand All @@ -380,13 +396,25 @@ impl NakamotoScore {
return (
cmp,
if val_self[0] != val_other[0] {
if val_self[0] > val_other[0] {
format!(
"(gets better) the number of nodes controlled by dominant NPs decreases from {} to {}",
val_other[0], val_self[0]
)
} else {
format!(
"(gets worse) the number of nodes controlled by dominant NPs increases from {} to {}",
val_other[0], val_self[0]
)
}
} else if val_self[1] > val_other[1] {
format!(
"the number of nodes controlled by dominant NPs changes from {} to {}",
val_other[0], val_self[0]
"(gets better) the number of nodes controlled by dominant Country actors decreases from {} to {}",
val_other[1], val_self[1]
)
} else {
format!(
"the number of nodes controlled by dominant Country actors changes from {} to {}",
"(gets worse) the number of nodes controlled by dominant Country actors increases from {} to {}",
val_other[1], val_self[1]
)
},
Expand All @@ -405,9 +433,27 @@ impl NakamotoScore {
return (
cmp,
if val_self[0] != val_other[0] {
format!("the number of different NP actors changes from {} to {}", val_other[0], val_self[0])
if val_other[0] < val_self[0] {
format!(
"(gets better) the number of different NP actors increases from {} to {}",
val_other[0], val_self[0]
)
} else {
format!(
"(gets worse) the number of different NP actors decreases from {} to {}",
val_other[0], val_self[0]
)
}
} else if val_other[1] < val_self[1] {
format!(
"(gets better) the number of different Country actors increases from {} to {}",
val_other[1], val_self[1]
)
} else {
format!("the number of different Country actors changes from {} to {}", val_other[1], val_self[1])
format!(
"(gets worse) the number of different Country actors decreases from {} to {}",
val_other[1], val_self[1]
)
},
);
}
Expand All @@ -421,10 +467,17 @@ impl NakamotoScore {
if cmp != Some(Ordering::Equal) {
return (
cmp,
format!(
"the number of Nakamoto coefficients with extremely low values changes from {} to {}",
c2, c1
),
if cmp == Some(Ordering::Less) {
format!(
"(gets better) the number of Nakamoto coefficients with extremely low values decreases from {} to {}",
c2, c1
)
} else {
format!(
"(gets worse) the number of Nakamoto coefficients with extremely low values increases from {} to {}",
c2, c1
)
},
);
}

Expand Down Expand Up @@ -491,12 +544,12 @@ impl Eq for NakamotoScore {}
impl Display for NakamotoScore {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
let avg_log2_str = match self.avg_log2 {
Some(v) => format!("{:0.2}", v),
Some(v) => format!("{:0.4}", v),
None => "undefined".to_string(),
};
write!(
f,
"NakamotoScore: min {:0.2} avg log2 {} #crit nodes {:?} # crit uniq {:?} #crit coeff {} avg linear {:0.2}",
"NakamotoScore: min {:0.2} avg log2 {} #crit nodes {:?} # crit uniq {:?} #crit coeff {} avg linear {:0.4}",
self.min,
avg_log2_str,
self.critical_features_num_nodes(),
Expand Down
55 changes: 46 additions & 9 deletions rs/decentralization/src/network.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -631,18 +631,26 @@ impl DecentralizedSubnet {
best_result
.business_rules_log
.iter()
.map(|s| format!("node {}/{} ({}): {}", i + 1, how_many_nodes, best_result.node.id, s))
.map(|s| {
format!(
"- adding node {} of {} ({}): {}",
i + 1,
how_many_nodes,
best_result.node.id.to_string().split('-').next().unwrap_or_default(),
s
)
})
.collect::<Vec<String>>(),
);
if i + 1 == how_many_nodes {
if total_penalty != 0 {
comment = Some(format!(
"Subnet extension with {} nodes finished with the total penalty {}. Penalty causes throughout the extension:\n{}\n\n{}",
"Subnet extension with {} nodes finished with the total penalty {}. Penalty causes throughout the extension:\n\n{}\n\n{}",
how_many_nodes,
total_penalty,
business_rules_log.join("\n"),
if how_many_nodes > 1 {
"Note that the penalty for nodes before the last node may not be relevant in the end. We leave this to humans to assess."
"Business rules analysis is calculated on each operation. Typically only the last operation is relevant, although this may depend on the case."
} else { "" }
));
} else {
Expand Down Expand Up @@ -714,18 +722,26 @@ impl DecentralizedSubnet {
best_result
.business_rules_log
.iter()
.map(|s| format!("node {}/{} ({}): {}", i + 1, how_many_nodes, best_result.node.id, s))
.map(|s| {
format!(
"- removing node {} of {} ({}): {}",
i + 1,
how_many_nodes,
best_result.node.id.to_string().split('-').next().unwrap_or_default(),
s
)
})
.collect::<Vec<String>>(),
);
if i + 1 == how_many_nodes {
if total_penalty != 0 {
comment = Some(format!(
"Subnet removal of {} nodes finished with the total penalty {}. Penalty causes throughout the removal:\n{}\n\n{}",
"Subnet removal of {} nodes finished with the total penalty {}. Penalty causes throughout the removal:\n\n{}\n\n{}",
how_many_nodes,
total_penalty,
business_rules_log.join("\n"),
if how_many_nodes > 1 {
"Note that the penalty for nodes before the last node may not be relevant in the end. We leave this to humans to assess."
"Business rules analysis is calculated on each operation. Typically only the last operation is relevant, although this may depend on the case."
} else {
""
}
Expand Down Expand Up @@ -1332,19 +1348,40 @@ impl NetworkHealRequest {
"- {} additional node{}: {}",
num_opt,
if num_opt > 1 { "s" } else { "" },
change.score_after.describe_difference_from(&changes[num_opt - 1].score_after).1
change
.score_after
.describe_difference_from(&changes[num_opt.saturating_sub(1)].score_after)
.1
)
})
.collect::<Vec<_>>();
info!("Max score: {}", changes_max_score.score_after);

let change = changes
.iter()
.find(|change| change.score_after == changes_max_score.score_after)
.find(|change: &&SubnetChangeResponse| change.score_after == changes_max_score.score_after)
.expect("No suitable changes found");

info!(
"Replacing {} nodes in subnet {} gives Nakamoto coefficient: {}\n",
change.removed_with_desc.len(),
subnet.decentralized_subnet.id,
change.score_after
);

let num_opt = change.removed_with_desc.len() - unhealthy_nodes_len;
let reason_additional_optimizations = if num_opt == 0 {
"\n\nNot replacing any additional nodes to improve optimization.\n\n".to_string()
format!(
"
Calculated impact on subnet decentralization if replacing:
{}
Based on the calculated impact, not replacing additional nodes to improve optimization.
",
optimizations_desc.join("\n")
)
} else {
format!("
Expand Down

0 comments on commit c161563

Please sign in to comment.