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