diff --git a/rs/cli/src/runner.rs b/rs/cli/src/runner.rs index 71e8ffa25..0d40de293 100644 --- a/rs/cli/src/runner.rs +++ b/rs/cli/src/runner.rs @@ -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)), }) } diff --git a/rs/decentralization/src/lib.rs b/rs/decentralization/src/lib.rs index e2d543720..c74f47ecc 100644 --- a/rs/decentralization/src/lib.rs +++ b/rs/decentralization/src/lib.rs @@ -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(); @@ -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 { @@ -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 { @@ -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); @@ -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(()) diff --git a/rs/decentralization/src/nakamoto/mod.rs b/rs/decentralization/src/nakamoto/mod.rs index 97c771303..53e6db9d8 100644 --- a/rs/decentralization/src/nakamoto/mod.rs +++ b/rs/decentralization/src/nakamoto/mod.rs @@ -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() + ) + }, ); } @@ -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) + ) + }, ); } @@ -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] ) }, @@ -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] + ) }, ); } @@ -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 + ) + }, ); } @@ -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(), diff --git a/rs/decentralization/src/network.rs b/rs/decentralization/src/network.rs index 178894833..da72abf90 100644 --- a/rs/decentralization/src/network.rs +++ b/rs/decentralization/src/network.rs @@ -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::>(), ); 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 { @@ -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::>(), ); 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 { "" } @@ -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::>(); + 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!("