Skip to content

Commit 078b251

Browse files
fix(perf): validate profile primary thresholds (#70)
Co-authored-by: Arbousier1 <elderli@foxmail.com>
1 parent 05a1a4c commit 078b251

3 files changed

Lines changed: 46 additions & 15 deletions

File tree

perf/ab/gate-config.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@
116116
"display.spawn.time",
117117
"display.spawn.alloc"
118118
],
119-
"minimum_passes": 2,
119+
"minimum_passes": 1,
120120
"must_pass": [
121121
"display.spawn.time"
122122
],
@@ -130,7 +130,7 @@
130130
"region.queue.time",
131131
"region.queue.alloc"
132132
],
133-
"minimum_passes": 2,
133+
"minimum_passes": 1,
134134
"must_pass": [
135135
"region.queue.time"
136136
],

perf/ab/gate.py

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -451,33 +451,44 @@ def render_markdown(decision: dict[str, Any]) -> str:
451451
return "\n".join(lines)
452452

453453

454-
def evaluate(
454+
def validate_profile_config(
455455
config: dict[str, Any],
456456
profile_name: str,
457457
benchmark_specs: list[dict[str, Any]],
458-
aa_pairs: dict[str, list[tuple[float, float]]],
459-
ab_pairs: dict[str, list[tuple[float, float]]],
460-
units: dict[str, str | None],
461-
) -> dict[str, Any]:
462-
bootstrap = config["bootstrap"]
463-
aa_results: list[dict[str, Any]] = []
464-
benchmark_results: list[dict[str, Any]] = []
465-
aa_passed = True
466-
reasons: list[str] = []
467-
458+
) -> tuple[set[str], int]:
468459
profile = config["profiles"][profile_name]
469-
must_pass = set(profile.get("must_pass", []))
470460
configured_ids = [entry["id"] for entry in benchmark_specs]
471461
if len(configured_ids) != len(set(configured_ids)):
472462
raise ValueError(f"profile {profile_name} contains duplicate benchmark ids")
473463
primary_ids = {
474464
entry["id"] for entry in benchmark_specs if entry.get("metric", "primary") == "primary"
475465
}
466+
must_pass = set(profile.get("must_pass", []))
476467
if not must_pass or not must_pass.issubset(primary_ids):
477468
raise ValueError(f"profile {profile_name} must_pass must contain only primary metrics")
478-
minimum_passes = int(profile["minimum_passes"])
469+
minimum_passes = profile.get("minimum_passes")
470+
if isinstance(minimum_passes, bool) or not isinstance(minimum_passes, int):
471+
raise ValueError(f"profile {profile_name} has a non-integer primary minimum_passes")
479472
if not 1 <= minimum_passes <= len(primary_ids):
480473
raise ValueError(f"profile {profile_name} has an invalid primary minimum_passes")
474+
return must_pass, minimum_passes
475+
476+
477+
def evaluate(
478+
config: dict[str, Any],
479+
profile_name: str,
480+
benchmark_specs: list[dict[str, Any]],
481+
aa_pairs: dict[str, list[tuple[float, float]]],
482+
ab_pairs: dict[str, list[tuple[float, float]]],
483+
units: dict[str, str | None],
484+
) -> dict[str, Any]:
485+
bootstrap = config["bootstrap"]
486+
aa_results: list[dict[str, Any]] = []
487+
benchmark_results: list[dict[str, Any]] = []
488+
aa_passed = True
489+
reasons: list[str] = []
490+
491+
must_pass, minimum_passes = validate_profile_config(config, profile_name, benchmark_specs)
481492
for entry in benchmark_specs:
482493
benchmark_id = entry["id"]
483494
aa_metrics = paired_statistics(
@@ -583,6 +594,7 @@ def run(args: argparse.Namespace) -> int:
583594
benchmark_specs = [benchmark_index[benchmark_id] for benchmark_id in profile["benchmark_ids"]]
584595
if not benchmark_specs:
585596
raise ValueError("at least one benchmark metric must be configured")
597+
validate_profile_config(config, args.profile, benchmark_specs)
586598
config_sha = sha256_file(config_path)
587599
repetitions = int(config["matrix"]["repetitions_per_order"])
588600
aa_pairs, aa_units, aa_run = load_pairs(

perf/ab/tests/test_gate.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,25 @@ def test_global_states_use_explicit_three_state_semantics(self) -> None:
272272
self.assertEqual("INCONCLUSIVE_NOISE_OR_NO_GAIN", gate.INCONCLUSIVE)
273273
self.assertEqual("FAIL_REGRESSION_OR_BEHAVIOR", gate.FAIL)
274274

275+
def test_every_profile_has_valid_primary_thresholds(self) -> None:
276+
benchmark_index = {entry["id"]: entry for entry in self.config["benchmarks"]}
277+
for profile_name, profile in self.config["profiles"].items():
278+
with self.subTest(profile=profile_name):
279+
benchmark_specs = [benchmark_index[entry] for entry in profile["benchmark_ids"]]
280+
gate.validate_profile_config(self.config, profile_name, benchmark_specs)
281+
282+
def test_profile_validation_rejects_minimum_above_primary_count(self) -> None:
283+
config = json.loads(json.dumps(self.config))
284+
profile_name = "display-spawn"
285+
config["profiles"][profile_name]["minimum_passes"] = 2
286+
benchmark_index = {entry["id"]: entry for entry in config["benchmarks"]}
287+
benchmark_specs = [
288+
benchmark_index[entry]
289+
for entry in config["profiles"][profile_name]["benchmark_ids"]
290+
]
291+
with self.assertRaisesRegex(ValueError, "invalid primary minimum_passes"):
292+
gate.validate_profile_config(config, profile_name, benchmark_specs)
293+
275294

276295
class MatrixScheduleTest(unittest.TestCase):
277296
def test_ab_schedule_is_exactly_four_interleaved_ab_and_ba_pairs(self) -> None:

0 commit comments

Comments
 (0)