diff --git a/code/evaluation/metrics.py b/code/evaluation/metrics.py index 76648e3..27ee87f 100644 --- a/code/evaluation/metrics.py +++ b/code/evaluation/metrics.py @@ -64,6 +64,22 @@ def _safe_ratio(numer: Any, denom: Any) -> float: return n / d +def _first_present_metric(source: Any, keys) -> Any: + """ + Return the first value in `source` whose key is present and not None. + + Selection is based on key presence + "is not None" rather than a boolean + `or` chain, so that a legitimate falsy metric (e.g. a perfect LER of 0.0) + is preserved instead of being skipped and lost. + """ + if not isinstance(source, dict): + return None + for key in keys: + if key in source and source[key] is not None: + return source[key] + return None + + def configure_metrics(rank=0): """ Configure which metric computation functions to use. @@ -246,45 +262,34 @@ def _extract_speedup(basis_dict): pymatching_latency_after_avg = float(sum(post_vals) / len(post_vals)) if isinstance(result, dict): - ler_value = None - for key in [ - 'logical_error_rate', 'ler', 'error_rate', 'avg_ler', 'logical error ratio (mean)' - ]: - if key in result: - ler_value = result[key] - break + # NOTE: select by key presence + "is not None" instead of boolean `or`. + # A perfect LER of 0.0 is a valid value but falsy, so `a or b or c` + # would skip it and could ultimately yield None, discarding the metric. + ler_value = _first_present_metric( + result, + [ + 'logical_error_rate', 'ler', 'error_rate', 'avg_ler', + 'logical error ratio (mean)' + ], + ) if ler_value is None: - if 'X' in result and isinstance(result['X'], dict): - x_ler = ( - result['X'].get('logical error ratio (mean)') or - result['X'].get('logical_error_rate') or result['X'].get('ler') - ) - if x_ler is not None: - ler_value = x_ler + basis_keys = ['logical error ratio (mean)', 'logical_error_rate', 'ler'] - if ler_value is None and 'Z' in result and isinstance(result['Z'], dict): - z_ler = ( - result['Z'].get('logical error ratio (mean)') or - result['Z'].get('logical_error_rate') or result['Z'].get('ler') - ) - if z_ler is not None: - ler_value = z_ler - - if 'X' in result and 'Z' in result and isinstance(result['X'], dict) and isinstance( - result['Z'], dict - ): - x_ler = ( - result['X'].get('logical error ratio (mean)') or - result['X'].get('logical_error_rate') or result['X'].get('ler') - ) - z_ler = ( - result['Z'].get('logical error ratio (mean)') or - result['Z'].get('logical_error_rate') or result['Z'].get('ler') - ) - - if x_ler is not None and z_ler is not None: - ler_value = (x_ler + z_ler) / 2.0 + x_ler = None + if 'X' in result and isinstance(result['X'], dict): + x_ler = _first_present_metric(result['X'], basis_keys) + + z_ler = None + if 'Z' in result and isinstance(result['Z'], dict): + z_ler = _first_present_metric(result['Z'], basis_keys) + + if x_ler is not None and z_ler is not None: + ler_value = (x_ler + z_ler) / 2.0 + elif x_ler is not None: + ler_value = x_ler + elif z_ler is not None: + ler_value = z_ler elif isinstance(result, (float, int)): ler_value = float(result) else: diff --git a/code/tests/test_metrics_extras.py b/code/tests/test_metrics_extras.py index 055ebe4..d476b0d 100644 --- a/code/tests/test_metrics_extras.py +++ b/code/tests/test_metrics_extras.py @@ -18,11 +18,13 @@ import sys import unittest from pathlib import Path +from unittest.mock import patch _repo_code = Path(__file__).resolve().parent.parent if str(_repo_code) not in sys.path: sys.path.insert(0, str(_repo_code)) +import evaluation.metrics as metrics from evaluation.metrics import configure_metrics, _extract_reduction_factor, compute_syndrome_density @@ -83,3 +85,53 @@ def test_sdr_as_percent_not_a_parameter(self): "sdr_as_percent is a display-only flag in train.py and must not be added " "to compute_syndrome_density(); passing it causes TypeError at runtime.", ) + + +class TestComputeSingleLerPreservesZero(unittest.TestCase): + """Regression tests for perfect (zero-error) LER extraction.""" + + def _extract_ler(self, result): + with patch.object(metrics, "compute_logical_error_rate", return_value=result): + ler, _, _ = metrics._compute_single_ler( + model=None, + device="cpu", + dist=None, + cfg=None, + generator=None, + rank=1, + ) + return ler + + def test_zero_ler_for_both_bases_is_preserved(self): + result = { + "X": { + "logical error ratio (mean)": 0.0, + "logical_error_rate": 0.4, + }, + "Z": { + "logical error ratio (mean)": 0.0, + "logical_error_rate": 0.6, + }, + } + + self.assertEqual(self._extract_ler(result), 0.0) + + def test_zero_ler_is_included_in_basis_average(self): + result = { + "X": { + "logical error ratio (mean)": 0.0 + }, + "Z": { + "logical error ratio (mean)": 0.2 + }, + } + + self.assertEqual(self._extract_ler(result), 0.1) + + def test_top_level_zero_ler_is_preserved(self): + result = { + "logical_error_rate": 0.0, + "ler": 0.9, + } + + self.assertEqual(self._extract_ler(result), 0.0) diff --git a/code/training/train.py b/code/training/train.py index 281cec7..ae2e7a8 100644 --- a/code/training/train.py +++ b/code/training/train.py @@ -1268,18 +1268,22 @@ def _print_gen(name, g): ) if 'metadata' in checkpoint_dict and 'best_vloss' in checkpoint_dict['metadata']: saved_using_ler = checkpoint_dict['metadata'].get('using_ler', False) + # With PREDECODER_LER_FINAL_ONLY=1 the per-epoch metric is validation + # loss even when LER validation is enabled, so expect a loss-based best. + ler_final_only = os.environ.get("PREDECODER_LER_FINAL_ONLY", "0") == "1" + expect_ler_metric = use_ler_for_early_stopping and not ler_final_only # Only restore best_vloss if the metric type matches (both LER or both loss) - if saved_using_ler == use_ler_for_early_stopping: + if saved_using_ler == expect_ler_metric: best_vloss = checkpoint_dict['metadata']['best_vloss'] if 'epochs_since_best' in checkpoint_dict['metadata']: epochs_since_best = checkpoint_dict['metadata']['epochs_since_best'] if dist.rank == 0: - metric_name = "LER" if use_ler_for_early_stopping else "validation loss" + metric_name = "LER" if expect_ler_metric else "validation loss" print(f"[Checkpoint] Restored best {metric_name}: {best_vloss:.6f}") else: if dist.rank == 0: old_metric = "LER" if saved_using_ler else "validation loss" - new_metric = "LER" if use_ler_for_early_stopping else "validation loss" + new_metric = "LER" if expect_ler_metric else "validation loss" print( f"[Checkpoint] Metric type changed ({old_metric} → {new_metric}), resetting best metric" ) @@ -1669,7 +1673,10 @@ def _print_gen(name, g): metadata={ "best_vloss": best_vloss, "epochs_since_best": epochs_since_best, - "using_ler": use_ler_for_early_stopping, + # Record the metric that actually produced best_vloss: when LER + # extraction fails, current_metric falls back to validation loss, + # and resume relies on this flag to tell the two scales apart. + "using_ler": use_ler_for_early_stopping and validation_ler is not None, }, global_step=global_step, )