From 5cf207f5425e465338cf37607311ab302ba913a4 Mon Sep 17 00:00:00 2001 From: Oleksandr Pavlyk <21087696+oleksandr-pavlyk@users.noreply.github.com> Date: Thu, 27 Aug 2026 14:51:06 -0500 Subject: [PATCH] Deprecate --threshold-diff, support --status ,,... filter Robust comparison does not take the threshold value into account, so document it as deprecated, emit warning if user specifies it as CLI argument. Support filtering of displayed rows by comparison status. This permits an easy way to select filters with status of interests. --- docs/nvbench_compare_robust.md | 37 ++++- python/scripts/nvbench_compare_robust.py | 95 +++++++++--- python/test/test_nvbench_compare_robust.py | 168 +++++++++++++++++++-- 3 files changed, 262 insertions(+), 38 deletions(-) diff --git a/docs/nvbench_compare_robust.md b/docs/nvbench_compare_robust.md index 7f168fe0..21227a5f 100644 --- a/docs/nvbench_compare_robust.md +++ b/docs/nvbench_compare_robust.md @@ -64,6 +64,16 @@ nvbench-compare-robust --display explain reference.json compare.json nvbench-compare-legacy reference.json compare.json ``` +Filter displayed rows by comparison status. Comma-separated statuses are combined +with OR. Status names are case-insensitive; full names and display abbreviations +are accepted: + +```bash +nvbench-compare-robust --status slow reference.json compare.json +nvbench-compare-robust --status slow,ambg reference.json compare.json +nvbench-compare-robust --status not-unknown reference.json compare.json +``` + Plot the comparison summary, or plot timings along a positive numeric axis. By default, plotting uses Matplotlib's interactive `plt.show()` behavior. Add `--dark` to the summary plot when it should use a dark theme: @@ -259,7 +269,7 @@ the comparison as `AMBG`. `--bulk-debug-python /path/to/output.py` writes a Python script to the specified file. The generated script contains a `bulk_rows` list. Each entry corresponds to one row that `nvbench-compare-robust` prints in its display tables after all -benchmark, axis, device, and threshold filters are applied. +benchmark, axis, device, and status filters are applied. This output is also useful when the built-in `--plot` or `--plot-along` views are too generic. The generated `bulk_rows` data and `load_bulk_data(row)` helper @@ -674,14 +684,27 @@ rare-value filtering has no repeated-value support to preserve. ## Other CLI Options -### `--threshold-diff PERCENT` +### `--status STATUS[,STATUS...]` -Filter displayed table rows to comparisons whose absolute center-to-center -relative difference is at least `PERCENT`. The value is a percentage, not a -fraction: use `--threshold-diff 5` for a 5% threshold. +Filter displayed table rows to selected comparison statuses. Comma-separated +statuses are combined with OR. The accepted spellings are: + +- `unknown`, `unkn`, `????` +- `ambiguous`, `undecided`, `ambg` +- `same` +- `fast` +- `slow` +- `not-unknown` to select `ambiguous`, `same`, `fast`, and `slow` + +This option affects table output, generated `--bulk-debug-python` rows, and +`--plot` summary entries. It does not change summary counters or the data used by +`--plot-along`. + +### `--threshold-diff PERCENT` -This option affects table output. It does not change summary counters or the -data used by `--plot-along`. +Deprecated. The option is accepted for command-line compatibility, but +`nvbench-compare-robust` ignores the value and prints a warning. Use `--status` +to filter displayed rows. ### `--plot-output PATH` diff --git a/python/scripts/nvbench_compare_robust.py b/python/scripts/nvbench_compare_robust.py index a7067f71..37820686 100644 --- a/python/scripts/nvbench_compare_robust.py +++ b/python/scripts/nvbench_compare_robust.py @@ -584,6 +584,61 @@ class ComparisonStatus(str, Enum): SLOW = "SLOW" +STATUS_FILTER_ALIASES = { + "????": ComparisonStatus.UNKNOWN, + "unknown": ComparisonStatus.UNKNOWN, + "unkn": ComparisonStatus.UNKNOWN, + "ambg": ComparisonStatus.UNDECIDED, + "ambiguous": ComparisonStatus.UNDECIDED, + "undecided": ComparisonStatus.UNDECIDED, + "same": ComparisonStatus.SAME, + "fast": ComparisonStatus.FAST, + "slow": ComparisonStatus.SLOW, +} + + +STATUS_FILTER_GROUP_ALIASES = { + "not-unknown": frozenset( + { + ComparisonStatus.UNDECIDED, + ComparisonStatus.SAME, + ComparisonStatus.FAST, + ComparisonStatus.SLOW, + } + ), +} + + +def parse_status_filter(status_arg): + if status_arg is None: + return None + + statuses = set() + for raw_token in status_arg.split(","): + token = raw_token.strip().lower() + if not token: + raise ValueError("--status must not contain an empty status") + status_group = STATUS_FILTER_GROUP_ALIASES.get(token) + if status_group is not None: + statuses.update(status_group) + continue + status = STATUS_FILTER_ALIASES.get(token) + if status is None: + valid = ( + "unknown/unkn/????, ambiguous/undecided/ambg, same, fast, slow, " + "not-unknown" + ) + raise ValueError( + f"--status value {raw_token!r} is invalid; expected {valid}" + ) + statuses.add(status) + return frozenset(statuses) + + +def status_matches_filter(status, status_filter): + return status_filter is None or status in status_filter + + @dataclass(frozen=True) class DecisionReason: code: str @@ -831,11 +886,6 @@ def parse_device_filter(device_arg, option_name): return device_ids -def validate_threshold_diff(threshold): - if not math.isfinite(threshold) or threshold < 0.0: - raise ValueError("--threshold-diff must be a finite non-negative percentage") - - def select_devices(all_devices, device_filter, option_name): if device_filter is None: return list(all_devices) @@ -2977,11 +3027,13 @@ def compare_benches( cmp_json_path=None, comparison_thresholds=None, display="intervals", + status_filter=None, bulk_debug_rows=None, plot_output=None, plot_along_output=None, plot_output_paths=None, ): + del threshold # Kept for source compatibility; --threshold-diff is ignored. if comparison_thresholds is None: comparison_thresholds = get_default_thresholds() if plot_output_paths is None: @@ -3138,10 +3190,7 @@ def compare_benches( ) run_data.stats.record(comparison.status, comparison.reason) - if comparison.status == ComparisonStatus.UNKNOWN or ( - comparison.frac_diff is not None - and abs(comparison.frac_diff) >= threshold - ): + if status_matches_filter(comparison.status, status_filter): axis_filters = matching_axis_filters(cmp_state, axis_filter_groups) append_display_row(row, comparison, no_color, display) @@ -3263,8 +3312,17 @@ def main() -> int: "--threshold-diff", type=float, dest="threshold", - default=0.0, - help="only show rows where abs(%%Diff) is >= THRESHOLD percent", + default=None, + help="deprecated; accepted for compatibility but ignored", + ) + parser.add_argument( + "--status", + default=None, + help=( + "only show rows with these comma-separated comparison statuses: " + "unknown/unkn/????, ambiguous/undecided/ambg, same, fast, slow, " + "not-unknown" + ), ) parser.add_argument( "--preset", @@ -3362,11 +3420,12 @@ def main() -> int: args = parser.parse_args() files_or_dirs = args.files_or_dirs - try: - validate_threshold_diff(args.threshold) - except ValueError as exc: - print(str(exc)) - return 1 + if args.threshold is not None: + print( + "Warning: --threshold-diff is ignored by nvbench-compare-robust; " + "use --status to select displayed rows.", + file=sys.stderr, + ) try: comparison_preset, comparison_thresholds = resolve_comparison_thresholds( @@ -3411,6 +3470,7 @@ def main() -> int: try: filter_plan = build_benchmark_filter_plan(args.filter_actions) + status_filter = parse_status_filter(args.status) reference_device_filter = parse_device_filter( args.reference_devices, "--reference-devices" ) @@ -3529,7 +3589,7 @@ def main() -> int: run_data, ref_root["benchmarks"], cmp_root["benchmarks"], - threshold=args.threshold / 100.0, + threshold=0.0, plot_along=args.plot_along, plot=args.plot, dark=args.dark, @@ -3543,6 +3603,7 @@ def main() -> int: cmp_json_path=comp, comparison_thresholds=comparison_thresholds, display=args.display, + status_filter=status_filter, bulk_debug_rows=bulk_debug_rows, plot_output=args.plot_output, plot_along_output=args.plot_along_output, diff --git a/python/test/test_nvbench_compare_robust.py b/python/test/test_nvbench_compare_robust.py index 7c81d6dd..718d7a4c 100644 --- a/python/test/test_nvbench_compare_robust.py +++ b/python/test/test_nvbench_compare_robust.py @@ -359,6 +359,35 @@ def format_test_percent(value): ] +@pytest.mark.parametrize( + "status_arg, expected", + [ + ("slow", {"SLOW"}), + ("FAST,SAME", {"FAST", "SAME"}), + ("unknown", {"????"}), + ("UNKN", {"????"}), + ("????", {"????"}), + ("ambiguous", {"AMBG"}), + ("undecided", {"AMBG"}), + ("AMBG", {"AMBG"}), + ("not-unknown", {"AMBG", "SAME", "FAST", "SLOW"}), + ("SLOW,not-unknown", {"AMBG", "SAME", "FAST", "SLOW"}), + ], +) +def test_parse_status_filter_accepts_names_and_display_codes( + nvbench_compare, status_arg, expected +): + statuses = nvbench_compare.parse_status_filter(status_arg) + + assert {status.value for status in statuses} == expected + + +@pytest.mark.parametrize("status_arg", ["", "slow,", "missing"]) +def test_parse_status_filter_rejects_invalid_values(nvbench_compare, status_arg): + with pytest.raises(ValueError, match="--status"): + nvbench_compare.parse_status_filter(status_arg) + + def make_gpu_timing_data( nvbench_compare, *, @@ -817,6 +846,86 @@ def test_compare_benches_collects_bulk_debug_rows(tmp_path, nvbench_compare): assert row["compare_frequency_filename"] == str(cmp_freqs_file) +def test_compare_benches_status_filter_selects_rows_and_bulk_debug( + monkeypatch, nvbench_compare +): + run_data = make_comparison_run_data(nvbench_compare) + tabulate_calls = capture_tabulate_calls(monkeypatch, nvbench_compare) + + def fake_compare_gpu_timings(ref_timing, cmp_timing, comparison_thresholds=None): + del comparison_thresholds + status = ( + nvbench_compare.ComparisonStatus.SLOW + if cmp_timing.mean > ref_timing.mean + else nvbench_compare.ComparisonStatus.SAME + ) + return nvbench_compare.SummaryComparison( + ref_interval=None, + cmp_interval=None, + ref_estimate=nvbench_compare.TimeEstimate( + center=ref_timing.mean, relative_dispersion=ref_timing.stdev_relative + ), + cmp_estimate=nvbench_compare.TimeEstimate( + center=cmp_timing.mean, relative_dispersion=cmp_timing.stdev_relative + ), + ref_time=ref_timing.mean, + cmp_time=cmp_timing.mean, + ref_noise=ref_timing.stdev_relative, + cmp_noise=cmp_timing.stdev_relative, + diff=cmp_timing.mean - ref_timing.mean, + frac_diff=(cmp_timing.mean - ref_timing.mean) / ref_timing.mean, + diff_interval=None, + frac_diff_interval=None, + max_noise=max(ref_timing.stdev_relative, cmp_timing.stdev_relative), + status=status, + reason=nvbench_compare.DecisionReason("test", "test"), + ) + + monkeypatch.setattr( + nvbench_compare, "compare_gpu_timings", fake_compare_gpu_timings + ) + + ref_bench = make_benchmark( + [ + make_state(nvbench_compare, "state", mean="1.0", axis_value=1), + make_state(nvbench_compare, "state", mean="1.0", axis_value=2), + ] + ) + cmp_bench = make_benchmark( + [ + make_state(nvbench_compare, "state", mean="1.0", axis_value=1), + make_state(nvbench_compare, "state", mean="1.2", axis_value=2), + ] + ) + bulk_debug_rows = [] + + nvbench_compare.compare_benches( + run_data, + [ref_bench], + [cmp_bench], + threshold=1.0, + plot_along=None, + plot=False, + dark=False, + filter_plan=make_filter_plan(nvbench_compare), + no_color=True, + status_filter=frozenset({nvbench_compare.ComparisonStatus.SLOW}), + bulk_debug_rows=bulk_debug_rows, + ) + + table = find_tabulate_call(tabulate_calls, INTERVAL_DISPLAY_HEADERS) + assert len(table["rows"]) == 1 + assert table["rows"][0][0] == "2" + assert table["rows"][0][-1] == "\U0001f534 SLOW" + + assert run_data.stats.config_count == 2 + assert run_data.stats.pass_count == 1 + assert run_data.stats.regression_count == 1 + assert len(bulk_debug_rows) == 1 + assert bulk_debug_rows[0]["status"] == nvbench_compare.ComparisonStatus.SLOW.value + assert bulk_debug_rows[0]["table_row_index"] == 0 + + def test_bulk_debug_rows_store_absolute_sidecar_paths( tmp_path, monkeypatch, nvbench_compare ): @@ -2350,7 +2459,7 @@ def test_plot_along_rejects_states_without_selected_axis(monkeypatch, nvbench_co ) -def test_plot_along_ignores_threshold_diff_table_filter(monkeypatch, nvbench_compare): +def test_plot_along_ignores_status_table_filter(monkeypatch, nvbench_compare): run_data = make_comparison_run_data(nvbench_compare) plot_calls = [] table_calls = [] @@ -2401,6 +2510,7 @@ def fake_plot(x, y, shape, *args, **kwargs): dark=False, filter_plan=make_filter_plan(nvbench_compare), no_color=True, + status_filter=frozenset({nvbench_compare.ComparisonStatus.SLOW}), ) assert run_data.stats.config_count == 2 @@ -2609,7 +2719,7 @@ def test_plot_along_output_disambiguates_duplicate_paths( assert "Warning: plot-along output" in capsys.readouterr().err -def test_compare_benches_validates_device_metadata_when_threshold_hides_rows( +def test_compare_benches_validates_device_metadata_when_status_filter_hides_rows( nvbench_compare, ): run_data = nvbench_compare.ComparisonRunData( @@ -2635,6 +2745,7 @@ def test_compare_benches_validates_device_metadata_when_threshold_hides_rows( dark=False, filter_plan=make_filter_plan(nvbench_compare), no_color=True, + status_filter=frozenset({nvbench_compare.ComparisonStatus.SLOW}), ) @@ -4502,7 +4613,7 @@ def test_sanitize_plot_output_component_uses_fallback_for_empty_values( assert nvbench_compare.plotting.sanitize_plot_output_component("../../") == "value" -def test_main_converts_threshold_diff_percent_to_fraction(monkeypatch, nvbench_compare): +def test_main_warns_threshold_diff_is_ignored(monkeypatch, capsys, nvbench_compare): devices = [{"id": 0, "name": "Test GPU"}] root = { "devices": devices, @@ -4515,6 +4626,7 @@ def test_main_converts_threshold_diff_percent_to_fraction(monkeypatch, nvbench_c def fake_compare_benches(*args, **kwargs): del args captured["threshold"] = kwargs["threshold"] + captured["status_filter"] = kwargs["status_filter"] monkeypatch.setattr(nvbench_compare, "compare_benches", fake_compare_benches) monkeypatch.setattr( @@ -4530,13 +4642,44 @@ def fake_compare_benches(*args, **kwargs): ) assert nvbench_compare.main() == 0 - assert captured["threshold"] == pytest.approx(0.05) + captured_output = capsys.readouterr() + assert "--threshold-diff is ignored" in captured_output.err + assert captured["threshold"] == pytest.approx(0.0) + assert captured["status_filter"] is None -@pytest.mark.parametrize("threshold", ["nan", "inf", "-1"]) -def test_main_rejects_invalid_threshold_diff( - monkeypatch, capsys, nvbench_compare, threshold -): +def test_main_passes_status_filter_to_compare_benches(monkeypatch, nvbench_compare): + devices = [{"id": 0, "name": "Test GPU"}] + root = { + "devices": devices, + "benchmarks": [], + } + captured = {} + + monkeypatch.setattr(nvbench_compare.reader, "read_file", lambda _: root) + + def fake_compare_benches(*args, **kwargs): + del args + captured["status_filter"] = kwargs["status_filter"] + + monkeypatch.setattr(nvbench_compare, "compare_benches", fake_compare_benches) + monkeypatch.setattr( + sys, + "argv", + [ + "nvbench_compare", + "--status", + "slow,ambg", + "ref.json", + "cmp.json", + ], + ) + + assert nvbench_compare.main() == 0 + assert {status.value for status in captured["status_filter"]} == {"SLOW", "AMBG"} + + +def test_main_rejects_invalid_status_filter(monkeypatch, capsys, nvbench_compare): monkeypatch.setattr( nvbench_compare, "load_nvbench_compare_tooling", @@ -4549,15 +4692,12 @@ def test_main_rejects_invalid_threshold_diff( "argv", [ "nvbench_compare", - "--threshold-diff", - threshold, + "--status", + "missing", "ref.json", "cmp.json", ], ) assert nvbench_compare.main() == 1 - assert ( - "--threshold-diff must be a finite non-negative percentage" - in capsys.readouterr().out - ) + assert "--status value 'missing' is invalid" in capsys.readouterr().out