From 7a573d76482b052f430118c61874050528bf6f9a Mon Sep 17 00:00:00 2001 From: nerdCopter <56646290+nerdCopter@users.noreply.github.com> Date: Thu, 21 May 2026 15:36:54 -0500 Subject: [PATCH 1/2] fix: report actual failing condition(s) in consistency warning (#149) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The warning message always attributed inconsistency to CV>40% regardless of which condition actually failed. When consistency% was the trigger (e.g. CV=31.9% but only 62% of samples within ±1 SD), the message incorrectly implied CV exceeded the threshold. Apply Option B (issue #149) to both console output and PNG label: pattern-match (cv_failed, consistency_failed) and emit only the threshold(s) that were actually crossed: - CV only: "CV=42.3% (>40%)" - Consistency only: "Consistency=62% (<70%)" - Both: "CV=42.3% (>40%) and Consistency=62% (<70%)" - Neither: "insufficient samples (need >= 2)" Flagging logic in is_consistent() is unchanged; only diagnostic text fixed. Co-Authored-By: Claude Sonnet 4.6 --- src/data_analysis/optimal_p_estimation.rs | 40 ++++++++++++++++---- src/plot_functions/plot_step_response.rs | 46 +++++++++++++++++++---- 2 files changed, 70 insertions(+), 16 deletions(-) diff --git a/src/data_analysis/optimal_p_estimation.rs b/src/data_analysis/optimal_p_estimation.rs index e4edec9..e5665be 100644 --- a/src/data_analysis/optimal_p_estimation.rs +++ b/src/data_analysis/optimal_p_estimation.rs @@ -513,17 +513,41 @@ impl OptimalPAnalysis { self.td_stats.consistency * 100.0 )); - // Warning for low consistency (inline) + // Warning for low consistency (inline) — report only the condition(s) that actually failed if !self.td_stats.is_consistent() { - let cv_percent = self + let cv_failed = self .td_stats .coefficient_of_variation - .map_or(0.0, |cv| cv * 100.0); - output.push_str(&format!( - " ⚠ Low consistency (CV={:.1}%) — unreliable (>{:.0}%)\n", - cv_percent, - TD_COEFFICIENT_OF_VARIATION_MAX * 100.0 - )); + .is_some_and(|cv| cv > TD_COEFFICIENT_OF_VARIATION_MAX); + let consistency_failed = self.td_stats.consistency < TD_CONSISTENCY_MIN_THRESHOLD; + let reason = match (cv_failed, consistency_failed) { + (true, true) => format!( + "CV={:.1}% (>{:.0}%) and Consistency={:.0}% (<{:.0}%)", + self.td_stats + .coefficient_of_variation + .map_or(0.0, |cv| cv * 100.0), + TD_COEFFICIENT_OF_VARIATION_MAX * 100.0, + self.td_stats.consistency * 100.0, + TD_CONSISTENCY_MIN_THRESHOLD * 100.0, + ), + (true, false) => format!( + "CV={:.1}% (>{:.0}%)", + self.td_stats + .coefficient_of_variation + .map_or(0.0, |cv| cv * 100.0), + TD_COEFFICIENT_OF_VARIATION_MAX * 100.0, + ), + (false, true) => format!( + "Consistency={:.0}% (<{:.0}%)", + self.td_stats.consistency * 100.0, + TD_CONSISTENCY_MIN_THRESHOLD * 100.0, + ), + (false, false) => format!( + "insufficient samples (need >= {})", + TD_SAMPLES_MIN_FOR_STDDEV, + ), + }; + output.push_str(&format!(" ⚠ Low consistency ({reason}) — unreliable\n")); } // Compact recommendation diff --git a/src/plot_functions/plot_step_response.rs b/src/plot_functions/plot_step_response.rs index f73cef2..42585bb 100644 --- a/src/plot_functions/plot_step_response.rs +++ b/src/plot_functions/plot_step_response.rs @@ -12,6 +12,7 @@ use crate::constants::{ FINAL_NORMALIZED_STEADY_STATE_TOLERANCE, LINE_WIDTH_PLOT, LOW_AUTHORITY_SETPOINT_THRESHOLD_DEG_S, POST_AVERAGING_SMOOTHING_WINDOW, RESPONSE_LENGTH_S, STEADY_STATE_END_S, STEADY_STATE_START_S, TD_COEFFICIENT_OF_VARIATION_MAX, + TD_CONSISTENCY_MIN_THRESHOLD, TD_SAMPLES_MIN_FOR_STDDEV, }; use crate::data_analysis::calc_step_response; // For average_responses and moving_average_smooth_f64 use crate::data_analysis::optimal_p_estimation::{OptimalPAnalysis, PRecommendation}; @@ -631,19 +632,48 @@ pub fn plot_step_response( // Consistency — always shown; orange warning when poor { - let cv_percent = analysis - .td_stats - .coefficient_of_variation - .map_or(0.0, |cv| cv * 100.0); let consistency_pct = (analysis.td_stats.consistency * 100.0).round() as u32; let (cons_label, cons_color) = if !analysis.td_stats.is_consistent() { + let cv_failed = analysis + .td_stats + .coefficient_of_variation + .is_some_and(|cv| cv > TD_COEFFICIENT_OF_VARIATION_MAX); + let consistency_failed = + analysis.td_stats.consistency < TD_CONSISTENCY_MIN_THRESHOLD; + let reason = match (cv_failed, consistency_failed) { + (true, true) => format!( + "CV={:.1}% (>{:.0}%) and Consistency={:.0}% (<{:.0}%)", + analysis + .td_stats + .coefficient_of_variation + .map_or(0.0, |cv| cv * 100.0), + TD_COEFFICIENT_OF_VARIATION_MAX * 100.0, + analysis.td_stats.consistency * 100.0, + TD_CONSISTENCY_MIN_THRESHOLD * 100.0, + ), + (true, false) => format!( + "CV={:.1}% (>{:.0}%)", + analysis + .td_stats + .coefficient_of_variation + .map_or(0.0, |cv| cv * 100.0), + TD_COEFFICIENT_OF_VARIATION_MAX * 100.0, + ), + (false, true) => format!( + "Consistency={:.0}% (<{:.0}%)", + analysis.td_stats.consistency * 100.0, + TD_CONSISTENCY_MIN_THRESHOLD * 100.0, + ), + (false, false) => format!( + "insufficient samples (need >= {})", + TD_SAMPLES_MIN_FOR_STDDEV, + ), + }; ( format!( - " Consistency: {}% (CV={:.1}%) — unreliable (>{:.0}%)", - consistency_pct, - cv_percent, - TD_COEFFICIENT_OF_VARIATION_MAX * 100.0 + " Consistency: {}% — unreliable ({})", + consistency_pct, reason ), COLOR_OPTIMAL_P_WARNING, ) From f79f39b45bfdaf38c38dae47004db806c59d6125 Mon Sep 17 00:00:00 2001 From: nerdCopter <56646290+nerdCopter@users.noreply.github.com> Date: Thu, 21 May 2026 16:04:25 -0500 Subject: [PATCH 2/2] fix: always show both Consistency and CV metrics (#149) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the per-condition failure match with a single always-shown reliability line in both console output and PNG legend: Reliable: Consistency=82% (≥70%), CV=25.1% (≤40%) Unreliable: Consistency=69% (≥70%), CV=31.9% (≤40%) Changes: - Remove Consistency from the compact header line (now in reliability line) - Always emit both metrics regardless of which condition failed - Use ≥/≤ threshold notation for unambiguous pass/fail direction - Show CV=N/A when insufficient samples (coefficient_of_variation is None) - Drop TD_SAMPLES_MIN_FOR_STDDEV import from plot_step_response (unused) - Consistent plain-text prefix: no warning icon on either reliable or unreliable Closes #149. Co-Authored-By: Claude Sonnet 4.6 --- src/data_analysis/optimal_p_estimation.rs | 60 ++++++++------------ src/plot_functions/plot_step_response.rs | 67 +++++++---------------- 2 files changed, 44 insertions(+), 83 deletions(-) diff --git a/src/data_analysis/optimal_p_estimation.rs b/src/data_analysis/optimal_p_estimation.rs index e5665be..aa28c41 100644 --- a/src/data_analysis/optimal_p_estimation.rs +++ b/src/data_analysis/optimal_p_estimation.rs @@ -503,51 +503,37 @@ impl OptimalPAnalysis { // Compact header - axis name and basic info output.push_str(&format!( - "{}: Td={:.1}ms (target {}, {:+.0}% dev, windows={}), Noise={}, Consistency={:.0}%\n", + "{}: Td={:.1}ms (target {}, {:+.0}% dev, windows={}), Noise={}\n", axis_name, self.td_stats.mean_ms, target_display, self.td_deviation_percent, self.td_stats.num_samples, self.noise_level.name(), - self.td_stats.consistency * 100.0 )); - // Warning for low consistency (inline) — report only the condition(s) that actually failed - if !self.td_stats.is_consistent() { - let cv_failed = self - .td_stats - .coefficient_of_variation - .is_some_and(|cv| cv > TD_COEFFICIENT_OF_VARIATION_MAX); - let consistency_failed = self.td_stats.consistency < TD_CONSISTENCY_MIN_THRESHOLD; - let reason = match (cv_failed, consistency_failed) { - (true, true) => format!( - "CV={:.1}% (>{:.0}%) and Consistency={:.0}% (<{:.0}%)", - self.td_stats - .coefficient_of_variation - .map_or(0.0, |cv| cv * 100.0), - TD_COEFFICIENT_OF_VARIATION_MAX * 100.0, - self.td_stats.consistency * 100.0, - TD_CONSISTENCY_MIN_THRESHOLD * 100.0, - ), - (true, false) => format!( - "CV={:.1}% (>{:.0}%)", - self.td_stats - .coefficient_of_variation - .map_or(0.0, |cv| cv * 100.0), - TD_COEFFICIENT_OF_VARIATION_MAX * 100.0, - ), - (false, true) => format!( - "Consistency={:.0}% (<{:.0}%)", - self.td_stats.consistency * 100.0, - TD_CONSISTENCY_MIN_THRESHOLD * 100.0, - ), - (false, false) => format!( - "insufficient samples (need >= {})", - TD_SAMPLES_MIN_FOR_STDDEV, - ), - }; - output.push_str(&format!(" ⚠ Low consistency ({reason}) — unreliable\n")); + // Reliability line — always shown with both metrics + { + let cv_str = self.td_stats.coefficient_of_variation.map_or_else( + || "CV=N/A".to_string(), + |cv| { + format!( + "CV={:.1}% (≤{:.0}%)", + cv * 100.0, + TD_COEFFICIENT_OF_VARIATION_MAX * 100.0, + ) + }, + ); + let cons_str = format!( + "Consistency={:.0}% (≥{:.0}%)", + self.td_stats.consistency * 100.0, + TD_CONSISTENCY_MIN_THRESHOLD * 100.0, + ); + if self.td_stats.is_consistent() { + output.push_str(&format!(" Reliable: {cons_str}, {cv_str}\n")); + } else { + output.push_str(&format!(" Unreliable: {cons_str}, {cv_str}\n")); + } } // Compact recommendation diff --git a/src/plot_functions/plot_step_response.rs b/src/plot_functions/plot_step_response.rs index 42585bb..b71183d 100644 --- a/src/plot_functions/plot_step_response.rs +++ b/src/plot_functions/plot_step_response.rs @@ -12,7 +12,7 @@ use crate::constants::{ FINAL_NORMALIZED_STEADY_STATE_TOLERANCE, LINE_WIDTH_PLOT, LOW_AUTHORITY_SETPOINT_THRESHOLD_DEG_S, POST_AVERAGING_SMOOTHING_WINDOW, RESPONSE_LENGTH_S, STEADY_STATE_END_S, STEADY_STATE_START_S, TD_COEFFICIENT_OF_VARIATION_MAX, - TD_CONSISTENCY_MIN_THRESHOLD, TD_SAMPLES_MIN_FOR_STDDEV, + TD_CONSISTENCY_MIN_THRESHOLD, }; use crate::data_analysis::calc_step_response; // For average_responses and moving_average_smooth_f64 use crate::data_analysis::optimal_p_estimation::{OptimalPAnalysis, PRecommendation}; @@ -630,57 +630,32 @@ pub fn plot_step_response( stroke_width: 0, }); - // Consistency — always shown; orange warning when poor + // Reliability — always shown with both metrics; orange when poor { - let consistency_pct = - (analysis.td_stats.consistency * 100.0).round() as u32; - let (cons_label, cons_color) = if !analysis.td_stats.is_consistent() { - let cv_failed = analysis - .td_stats - .coefficient_of_variation - .is_some_and(|cv| cv > TD_COEFFICIENT_OF_VARIATION_MAX); - let consistency_failed = - analysis.td_stats.consistency < TD_CONSISTENCY_MIN_THRESHOLD; - let reason = match (cv_failed, consistency_failed) { - (true, true) => format!( - "CV={:.1}% (>{:.0}%) and Consistency={:.0}% (<{:.0}%)", - analysis - .td_stats - .coefficient_of_variation - .map_or(0.0, |cv| cv * 100.0), - TD_COEFFICIENT_OF_VARIATION_MAX * 100.0, - analysis.td_stats.consistency * 100.0, - TD_CONSISTENCY_MIN_THRESHOLD * 100.0, - ), - (true, false) => format!( - "CV={:.1}% (>{:.0}%)", - analysis - .td_stats - .coefficient_of_variation - .map_or(0.0, |cv| cv * 100.0), + let cv_str = analysis.td_stats.coefficient_of_variation.map_or_else( + || "CV=N/A".to_string(), + |cv| { + format!( + "CV={:.1}% (≤{:.0}%)", + cv * 100.0, TD_COEFFICIENT_OF_VARIATION_MAX * 100.0, - ), - (false, true) => format!( - "Consistency={:.0}% (<{:.0}%)", - analysis.td_stats.consistency * 100.0, - TD_CONSISTENCY_MIN_THRESHOLD * 100.0, - ), - (false, false) => format!( - "insufficient samples (need >= {})", - TD_SAMPLES_MIN_FOR_STDDEV, - ), - }; + ) + }, + ); + let cons_str = format!( + "Consistency={:.0}% (≥{:.0}%)", + analysis.td_stats.consistency * 100.0, + TD_CONSISTENCY_MIN_THRESHOLD * 100.0, + ); + let (cons_label, cons_color) = if analysis.td_stats.is_consistent() { ( - format!( - " Consistency: {}% — unreliable ({})", - consistency_pct, reason - ), - COLOR_OPTIMAL_P_WARNING, + format!(" Reliable: {cons_str}, {cv_str}"), + COLOR_OPTIMAL_P_TEXT, ) } else { ( - format!(" Consistency: {}%", consistency_pct), - COLOR_OPTIMAL_P_TEXT, + format!(" Unreliable: {cons_str}, {cv_str}"), + COLOR_OPTIMAL_P_WARNING, ) }; series.push(PlotSeries {