From 26809748b694e6ee77ee696a6a27b4968814e9c3 Mon Sep 17 00:00:00 2001 From: jiagaoxiang Date: Fri, 28 Aug 2026 07:11:53 +0000 Subject: [PATCH 1/3] test: pin apply_multi_node_invalid_variants CONC filter (PRPUNDIT-10) Lock the single-node no-op, multi-node cuda-graph-max-bs drop/keep split, CONC=0 guard, and unparseable CONC fallback to 64. --- .../tests/test_grid_variant_filter.py | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 src/hyperloom/inference_optimizer/tests/test_grid_variant_filter.py diff --git a/src/hyperloom/inference_optimizer/tests/test_grid_variant_filter.py b/src/hyperloom/inference_optimizer/tests/test_grid_variant_filter.py new file mode 100644 index 0000000000..08f3a84ea1 --- /dev/null +++ b/src/hyperloom/inference_optimizer/tests/test_grid_variant_filter.py @@ -0,0 +1,71 @@ +# SPDX-FileCopyrightText: 2026 Advanced Micro Devices, Inc. +# SPDX-License-Identifier: MIT + +"""Unit tests for apply_multi_node_invalid_variants.""" + +from __future__ import annotations + +import pytest + +from hyperloom.orchestrator.actions.executors import _multi_node_env as mn +from hyperloom.orchestrator.actions.executors._grid_base import GridVariant +from hyperloom.orchestrator.actions.executors._grid_variant_filter import ( + apply_multi_node_invalid_variants, +) + + +def _v(name: str, *, args: str = "") -> GridVariant: + return GridVariant(name=name, extra_server_args=args) + + +@pytest.fixture() +def _multi_node(monkeypatch): + monkeypatch.setattr(mn, "is_multi_node", lambda: True) + + +def test_single_node_is_a_strict_noop(monkeypatch): + monkeypatch.setattr(mn, "is_multi_node", lambda: False) + monkeypatch.setenv("CONC", "64") + grid = [ + _v("low-graph", args="--cuda-graph-max-bs 8"), + _v("keep", args="--cuda-graph-max-bs 64"), + ] + kept, dropped = apply_multi_node_invalid_variants(grid) + assert kept is grid + assert dropped == [] + + +def test_multi_node_drops_cuda_graph_max_bs_below_conc(_multi_node, monkeypatch): + monkeypatch.setenv("CONC", "64") + grid = [ + _v("space-form", args="--cuda-graph-max-bs 32"), + _v("equals-form", args="--cuda_graph_max_bs=8"), + _v("at-threshold", args="--cuda-graph-max-bs 64"), + _v("above", args="--cuda-graph-max-bs 128"), + _v("no-flag", args="--chunked-prefill-size 8192"), + ] + kept, dropped = apply_multi_node_invalid_variants(grid) + assert [v.name for v in kept] == ["at-threshold", "above", "no-flag"] + assert [row["name"] for row in dropped] == ["space-form", "equals-form"] + assert all(row["source"] == "multi_node_invalid" for row in dropped) + assert "CONC=64" in dropped[0]["reason"] + + +def test_conc_zero_does_not_drop(_multi_node, monkeypatch): + monkeypatch.setenv("CONC", "0") + grid = [_v("low-graph", args="--cuda-graph-max-bs 1")] + kept, dropped = apply_multi_node_invalid_variants(grid) + assert [v.name for v in kept] == ["low-graph"] + assert dropped == [] + + +def test_unparseable_conc_falls_back_to_64(_multi_node, monkeypatch): + monkeypatch.setenv("CONC", "not-an-int") + grid = [ + _v("below-default", args="--cuda-graph-max-bs 32"), + _v("at-default", args="--cuda-graph-max-bs 64"), + ] + kept, dropped = apply_multi_node_invalid_variants(grid) + assert [v.name for v in kept] == ["at-default"] + assert [row["name"] for row in dropped] == ["below-default"] + assert "CONC=64" in dropped[0]["reason"] From 5ed6c6b8b3951008ab264d78846fe743fbf379d5 Mon Sep 17 00:00:00 2001 From: Douglas Jia Date: Mon, 31 Aug 2026 19:48:39 +0000 Subject: [PATCH 2/3] fix(test+prod): pin .search/.match, CONC unset/empty; drop unreachable conc>0 guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production (_grid_variant_filter.py:202): drop 'conc > 0 and'. The regex (\d+) guarantees conc >= 0 always, so the guard is dead code; 'n < conc' is already False for every n >= 1 when conc == 0, preserving the same observable behaviour. Tests: - test_multi_node_flag_in_non_leading_position_is_detected: flag embedded in a multi-flag string ('--tp 8 --cuda-graph-max-bs 32') pins re.search vs re.match - test_conc_unset_defaults_to_64: monkeypatch.delenv exercises the os.environ.get default '64' path (mutating '64' → '1' now turns this red) - test_conc_empty_string_defaults_to_64: CONC='' exercises the 'or 64' branch (mutating 'or 64' → 'or 1' now turns this red) - Add comment to test_conc_zero_does_not_drop explaining it documents observable behaviour rather than the (now-removed) guard. Addresses zoroyihan7 review on PR #1325. --- .../tests/test_grid_variant_filter.py | 42 +++++++++++++++++++ .../actions/executors/_grid_variant_filter.py | 2 +- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/hyperloom/inference_optimizer/tests/test_grid_variant_filter.py b/src/hyperloom/inference_optimizer/tests/test_grid_variant_filter.py index 08f3a84ea1..b4c2457229 100644 --- a/src/hyperloom/inference_optimizer/tests/test_grid_variant_filter.py +++ b/src/hyperloom/inference_optimizer/tests/test_grid_variant_filter.py @@ -52,6 +52,10 @@ def test_multi_node_drops_cuda_graph_max_bs_below_conc(_multi_node, monkeypatch) def test_conc_zero_does_not_drop(_multi_node, monkeypatch): + # Documents observable behaviour: CONC=0 never drops anything. + # The `conc > 0 and` guard was removed from production because the regex + # (\d+) guarantees conc is always >= 0, making the guard unreachable; the + # invariant is preserved by `n < conc` being False for all n >= 1 when conc=0. monkeypatch.setenv("CONC", "0") grid = [_v("low-graph", args="--cuda-graph-max-bs 1")] kept, dropped = apply_multi_node_invalid_variants(grid) @@ -59,6 +63,44 @@ def test_conc_zero_does_not_drop(_multi_node, monkeypatch): assert dropped == [] +def test_multi_node_flag_in_non_leading_position_is_detected(_multi_node, monkeypatch): + """The filter uses re.search(), not re.match() — flag anywhere in the string must fire.""" + monkeypatch.setenv("CONC", "64") + grid = [ + _v("multi-flag-drop", args="--tp 8 --cuda-graph-max-bs 32"), + _v("multi-flag-keep", args="--tp 8 --cuda-graph-max-bs 128"), + ] + kept, dropped = apply_multi_node_invalid_variants(grid) + assert [v.name for v in kept] == ["multi-flag-keep"] + assert [row["name"] for row in dropped] == ["multi-flag-drop"] + + +def test_conc_unset_defaults_to_64(_multi_node, monkeypatch): + """CONC env var absent → the os.environ.get default of '64' applies.""" + monkeypatch.delenv("CONC", raising=False) + grid = [ + _v("below-default", args="--cuda-graph-max-bs 32"), + _v("at-default", args="--cuda-graph-max-bs 64"), + ] + kept, dropped = apply_multi_node_invalid_variants(grid) + assert [v.name for v in kept] == ["at-default"] + assert [row["name"] for row in dropped] == ["below-default"] + assert "CONC=64" in dropped[0]["reason"] + + +def test_conc_empty_string_defaults_to_64(_multi_node, monkeypatch): + """CONC='' → the `or 64` branch applies (empty string is falsy).""" + monkeypatch.setenv("CONC", "") + grid = [ + _v("below-default", args="--cuda-graph-max-bs 32"), + _v("at-default", args="--cuda-graph-max-bs 64"), + ] + kept, dropped = apply_multi_node_invalid_variants(grid) + assert [v.name for v in kept] == ["at-default"] + assert [row["name"] for row in dropped] == ["below-default"] + assert "CONC=64" in dropped[0]["reason"] + + def test_unparseable_conc_falls_back_to_64(_multi_node, monkeypatch): monkeypatch.setenv("CONC", "not-an-int") grid = [ diff --git a/src/hyperloom/orchestrator/actions/executors/_grid_variant_filter.py b/src/hyperloom/orchestrator/actions/executors/_grid_variant_filter.py index 50c3d071a8..b9aca80f53 100644 --- a/src/hyperloom/orchestrator/actions/executors/_grid_variant_filter.py +++ b/src/hyperloom/orchestrator/actions/executors/_grid_variant_filter.py @@ -199,7 +199,7 @@ def apply_multi_node_invalid_variants( dropped: list[dict] = [] for v in grid: m = _RE_CUDA_GRAPH_MAX_BS.search(v.extra_server_args or "") - if conc > 0 and m and int(m.group(1)) < conc: + if m and int(m.group(1)) < conc: dropped.append( { "name": v.name, From dbc2a7d6c187c25b53d18b4e3cada7152593bdd3 Mon Sep 17 00:00:00 2001 From: jiagaoxiang Date: Mon, 31 Aug 2026 21:33:15 +0000 Subject: [PATCH 3/3] fix(test): pin dropped reason bs=32, None args, and document kept-is-grid Review nits on #1325: the dropped reason is now self-describing (cuda_graph_max_bs=32 plus CONC=64), extra_server_args=None pins the `or ""` guard, and the single-node `kept is grid` identity is documented as the production no-copy contract. --- .../tests/test_grid_variant_filter.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/hyperloom/inference_optimizer/tests/test_grid_variant_filter.py b/src/hyperloom/inference_optimizer/tests/test_grid_variant_filter.py index b4c2457229..99f88b3108 100644 --- a/src/hyperloom/inference_optimizer/tests/test_grid_variant_filter.py +++ b/src/hyperloom/inference_optimizer/tests/test_grid_variant_filter.py @@ -31,6 +31,9 @@ def test_single_node_is_a_strict_noop(monkeypatch): _v("keep", args="--cuda-graph-max-bs 64"), ] kept, dropped = apply_multi_node_invalid_variants(grid) + # Identity is the production contract: single-node returns the input + # list (`return grid, []`), not a copy. Sibling filters copy on their + # no-op path; this one does not, so `is` is the pin. assert kept is grid assert dropped == [] @@ -49,6 +52,7 @@ def test_multi_node_drops_cuda_graph_max_bs_below_conc(_multi_node, monkeypatch) assert [row["name"] for row in dropped] == ["space-form", "equals-form"] assert all(row["source"] == "multi_node_invalid" for row in dropped) assert "CONC=64" in dropped[0]["reason"] + assert "cuda_graph_max_bs=32" in dropped[0]["reason"] def test_conc_zero_does_not_drop(_multi_node, monkeypatch): @@ -73,6 +77,8 @@ def test_multi_node_flag_in_non_leading_position_is_detected(_multi_node, monkey kept, dropped = apply_multi_node_invalid_variants(grid) assert [v.name for v in kept] == ["multi-flag-keep"] assert [row["name"] for row in dropped] == ["multi-flag-drop"] + assert "cuda_graph_max_bs=32" in dropped[0]["reason"] + assert "CONC=64" in dropped[0]["reason"] def test_conc_unset_defaults_to_64(_multi_node, monkeypatch): @@ -86,6 +92,7 @@ def test_conc_unset_defaults_to_64(_multi_node, monkeypatch): assert [v.name for v in kept] == ["at-default"] assert [row["name"] for row in dropped] == ["below-default"] assert "CONC=64" in dropped[0]["reason"] + assert "cuda_graph_max_bs=32" in dropped[0]["reason"] def test_conc_empty_string_defaults_to_64(_multi_node, monkeypatch): @@ -99,6 +106,7 @@ def test_conc_empty_string_defaults_to_64(_multi_node, monkeypatch): assert [v.name for v in kept] == ["at-default"] assert [row["name"] for row in dropped] == ["below-default"] assert "CONC=64" in dropped[0]["reason"] + assert "cuda_graph_max_bs=32" in dropped[0]["reason"] def test_unparseable_conc_falls_back_to_64(_multi_node, monkeypatch): @@ -111,3 +119,13 @@ def test_unparseable_conc_falls_back_to_64(_multi_node, monkeypatch): assert [v.name for v in kept] == ["at-default"] assert [row["name"] for row in dropped] == ["below-default"] assert "CONC=64" in dropped[0]["reason"] + assert "cuda_graph_max_bs=32" in dropped[0]["reason"] + + +def test_none_extra_server_args_is_treated_as_empty(_multi_node, monkeypatch): + """`v.extra_server_args or ""` must not raise or match when args is None.""" + monkeypatch.setenv("CONC", "64") + v = GridVariant(name="none-args", extra_server_args=None) + kept, dropped = apply_multi_node_invalid_variants([v]) + assert kept == [v] + assert dropped == []