From f72427605be28abf2e225f65f03c2afddf071213 Mon Sep 17 00:00:00 2001 From: jrstevens Date: Thu, 13 Aug 2026 07:59:02 -0700 Subject: [PATCH] Arm backend: Xfail RIFE VGF tests with lazy import (#21786) Summary: Reapplies https://github.com/pytorch/executorch/issues/21755 after the revert in https://github.com/pytorch/executorch/issues/21783, but imports the model-converter version helpers lazily during test collection. This avoids importing torch from the Arm conftest before internal test infrastructure has initialized its symbols, while preserving the RIFE VGF quant test xfail behavior. The import is attempted only when matching RIFE VGF quant tests are collected. Test Plan: CI cc digantdesai freddan80 per zingo oscarandersson8218 mansnils Sebastian-Larsson robell rascani Reviewed By: digantdesai Differential Revision: D115750356 Pulled By: JakeStevens --- backends/arm/test/conftest.py | 55 +++++++- backends/arm/test/misc/test_vgf_check_env.py | 133 ++++++++++++++++++ .../test/runtime/_vgf_runtime_test_utils.py | 69 ++++----- .../test/runtime/test_vgf_aliasing_runtime.py | 4 +- .../runtime/test_vgf_combinations_runtime.py | 4 +- .../runtime/test_vgf_multi_segment_runtime.py | 4 +- .../runtime/test_vgf_sampler_image_runtime.py | 4 +- .../runtime/test_vgf_tensor_buffer_runtime.py | 4 +- backends/arm/vgf/model_converter.py | 69 +++++++++ 9 files changed, 286 insertions(+), 60 deletions(-) diff --git a/backends/arm/test/conftest.py b/backends/arm/test/conftest.py index 56e817d86c4..52288821af8 100644 --- a/backends/arm/test/conftest.py +++ b/backends/arm/test/conftest.py @@ -2,6 +2,9 @@ # # This source code is licensed under the BSD-style license found in the # LICENSE file in the root directory of this source tree. +"""Pytest hooks and fixtures for the Arm test suite.""" + +from __future__ import annotations import logging import os @@ -11,9 +14,7 @@ import pytest -""" -This file contains the pytest hooks, fixtures etc. for the Arm test suite. -""" +logger: logging.Logger = logging.getLogger(__name__) # ==== Pytest hooks ==== @@ -41,8 +42,53 @@ def pytest_report_header(config): return config._test_seed_label +def _mark_rife_vgf_xfails_for_model_converter_below_minimum_version( + items, reason: str +) -> None: + for item in items: + nodeid = item.nodeid.lower() + if "test_rife.py" not in nodeid or "vgf_quant" not in nodeid: + continue + item.add_marker(pytest.mark.xfail(reason=reason, strict=False)) + + +def _has_rife_vgf_quant_tests(items) -> bool: + return any( + "test_rife.py" in item.nodeid.lower() and "vgf_quant" in item.nodeid.lower() + for item in items + ) + + def pytest_collection_modifyitems(config, items): - pass + if not _has_rife_vgf_quant_tests(items): + return + + try: + from executorch.backends.arm.vgf.model_converter import ( + get_model_converter_minimum_version_failure_reason, + get_model_converter_version_text, + MIN_MODEL_CONVERTER_VERSION_FOR_VGF_TESTS, + ) + except Exception: + logger.warning( + "Could not import the model-converter version helpers; leaving the " + "RIFE VGF quant tests unmarked.", + exc_info=True, + ) + return + + version_text = get_model_converter_version_text() + if version_text is None: + return + + reason = get_model_converter_minimum_version_failure_reason( + version_text, + MIN_MODEL_CONVERTER_VERSION_FOR_VGF_TESTS, + requirement_name="the copied RIFE VGF quant tests", + ) + if reason is None: + return + _mark_rife_vgf_xfails_for_model_converter_below_minimum_version(items, reason) def pytest_addoption(parser): @@ -127,7 +173,6 @@ def is_option_enabled(option: str, fail_if_not_enabled: bool = False) -> bool: RuntimeError instead of returning False. """ - if hasattr(pytest, "_test_options") and option in pytest._test_options and pytest._test_options[option]: # type: ignore[attr-defined] return True else: diff --git a/backends/arm/test/misc/test_vgf_check_env.py b/backends/arm/test/misc/test_vgf_check_env.py index 646eb6b9a79..c062ca0b021 100644 --- a/backends/arm/test/misc/test_vgf_check_env.py +++ b/backends/arm/test/misc/test_vgf_check_env.py @@ -7,11 +7,13 @@ import stat from pathlib import Path +from typing import Any import executorch.backends.arm.vgf.check_env as check_env import executorch.backends.arm.vgf.model_converter as model_converter import pytest +from executorch.backends.arm.test import conftest as arm_conftest from executorch.backends.arm.vgf import backend as vgf_backend from executorch.backends.arm.vgf.compile_spec import VgfCompileSpec @@ -151,6 +153,137 @@ def test_model_converter_check_reports_version(monkeypatch, tmp_path): assert "0.9.0" in result.detail +def test_get_model_converter_version_text(monkeypatch, tmp_path): + converter = _make_executable( + tmp_path / "model-converter", + "#!/usr/bin/env python3\n" + "import sys\n" + "if '--version' in sys.argv:\n" + " print('model-converter d8c1b8e')\n" + " raise SystemExit(0)\n" + "raise SystemExit(1)\n", + ) + monkeypatch.setattr( + model_converter, "find_model_converter_binary", lambda: str(converter) + ) + + assert model_converter.get_model_converter_version_text() == ( + "model-converter d8c1b8e" + ) + + +def test_parse_model_converter_version_uses_known_build_alias(monkeypatch, tmp_path): + converter = _make_executable( + tmp_path / "model-converter", + "#!/usr/bin/env python3\n" + "import sys\n" + "if '--version' in sys.argv:\n" + " print('model-converter d8c1b8e')\n" + " raise SystemExit(0)\n" + "raise SystemExit(1)\n", + ) + monkeypatch.setattr( + model_converter, "find_model_converter_binary", lambda: str(converter) + ) + + version_text = model_converter.get_model_converter_version_text() + + assert version_text is not None + assert model_converter.parse_model_converter_version(version_text) == ( + model_converter.Version("0.9.0") + ) + + +def test_below_minimum_model_converter_reason(monkeypatch, tmp_path): + converter = _make_executable( + tmp_path / "model-converter", + "#!/usr/bin/env python3\n" + "import sys\n" + "if '--version' in sys.argv:\n" + " print('model-converter d8c1b8e')\n" + " raise SystemExit(0)\n" + "raise SystemExit(1)\n", + ) + monkeypatch.setattr( + model_converter, "find_model_converter_binary", lambda: str(converter) + ) + + version_text = model_converter.get_model_converter_version_text() + + assert version_text is not None + assert model_converter.get_model_converter_minimum_version_failure_reason( + version_text, + model_converter.MIN_MODEL_CONVERTER_VERSION_FOR_VGF_TESTS, + requirement_name="the copied RIFE VGF quant tests", + ) == ( + "model-converter d8c1b8e is below the minimum supported version " + "0.10.0 required for the copied RIFE VGF quant tests" + ) + + +def test_mark_rife_vgf_xfails_marks_only_rife_vgf_quant(): + class DummyItem: + def __init__(self, nodeid: str): + self.nodeid = nodeid + self.markers: list[Any] = [] + + def add_marker(self, marker: Any) -> None: + self.markers.append(marker) + + matching = DummyItem("backends/arm/test/models/test_RIFE.py::test_vgf_quant") + non_matching = DummyItem("backends/arm/test/models/test_RIFE.py::test_tosa") + + arm_conftest._mark_rife_vgf_xfails_for_model_converter_below_minimum_version( + [matching, non_matching], "below minimum version" + ) + + assert len(matching.markers) == 1 + assert matching.markers[0].name == "xfail" + assert not non_matching.markers + + +def test_has_rife_vgf_quant_tests_matches_only_rife_vgf_quant(): + class DummyItem: + __slots__ = ("nodeid",) + + def __init__(self, nodeid: str): + self.nodeid = nodeid + + assert arm_conftest._has_rife_vgf_quant_tests( + [ + DummyItem("backends/arm/test/models/test_RIFE.py::test_tosa"), + DummyItem("backends/arm/test/models/test_RIFE.py::test_vgf_quant"), + ] + ) + assert not arm_conftest._has_rife_vgf_quant_tests( + [ + DummyItem("backends/arm/test/models/test_RIFE.py::test_tosa"), + DummyItem("backends/arm/test/ops/test_add.py::test_basic"), + ] + ) + + +def test_collection_hook_skips_converter_probe_when_no_rife_vgf_quant(monkeypatch): + class DummyItem: + __slots__ = ("nodeid",) + + def __init__(self, nodeid: str): + self.nodeid = nodeid + + def fail_probe(): + raise AssertionError("converter probe should not run") + + monkeypatch.setattr(model_converter, "get_model_converter_version_text", fail_probe) + + arm_conftest.pytest_collection_modifyitems( + None, + [ + DummyItem("backends/arm/test/models/test_RIFE.py::test_tosa"), + DummyItem("backends/arm/test/ops/test_add.py::test_basic"), + ], + ) + + def test_model_converter_lib_dir_fails_when_invalid(monkeypatch, tmp_path): missing = tmp_path / "missing" monkeypatch.setenv("MODEL_CONVERTER_LIB_DIR", str(missing)) diff --git a/backends/arm/test/runtime/_vgf_runtime_test_utils.py b/backends/arm/test/runtime/_vgf_runtime_test_utils.py index d72099796a3..b99fddfa575 100644 --- a/backends/arm/test/runtime/_vgf_runtime_test_utils.py +++ b/backends/arm/test/runtime/_vgf_runtime_test_utils.py @@ -42,8 +42,10 @@ ) from executorch.backends.arm.vgf import VgfCompileSpec, VgfPartitioner from executorch.backends.arm.vgf.model_converter import ( - find_model_converter_binary, - model_converter_env, + get_model_converter_minimum_version_failure_reason, + get_model_converter_version_text, + MIN_MODEL_CONVERTER_VERSION_FOR_VGF_TESTS, + parse_model_converter_version, ) from executorch.exir import EdgeCompileConfig, to_edge_transform_and_lower from executorch.exir.pass_base import ExportPass @@ -67,60 +69,37 @@ def ensure_glslc() -> None: @functools.lru_cache(maxsize=1) -def _model_converter_is_legacy_release() -> tuple[bool, str]: - model_converter = find_model_converter_binary() - if model_converter is None: +def _model_converter_supports_vgf_tests() -> tuple[bool, str]: + version_text = get_model_converter_version_text() + if version_text is None: warnings.warn( "Could not find model-converter while evaluating the VGF runtime " - "legacy-version xfail gate; assuming a newer/custom build.", + "minimum-version xfail gate; assuming a newer/custom build.", stacklevel=2, ) - return False, "" + return True, "" - try: - result = subprocess.run( # nosec B603 - trusted local tool - [model_converter, "--version"], - check=True, - capture_output=True, - text=True, - env=model_converter_env(), - ) - except Exception as exc: - warnings.warn( - "Failed to query model-converter --version while evaluating the VGF " - f"runtime legacy-version xfail gate ({exc}); assuming a newer/custom " - "build.", - stacklevel=2, - ) - return False, "" + reason = get_model_converter_minimum_version_failure_reason( + version_text, + MIN_MODEL_CONVERTER_VERSION_FOR_VGF_TESTS, + requirement_name="these VGF runtime tests", + ) + if reason is not None: + return False, reason - version_text = (result.stdout or result.stderr).strip() - if not version_text: + if parse_model_converter_version(version_text) is None: warnings.warn( - "model-converter --version returned no output while evaluating the VGF " - "runtime legacy-version xfail gate; assuming a newer/custom build.", + "Could not map model-converter version output to a comparable " + f"release while evaluating the VGF runtime minimum-version xfail " + f"gate; detected {version_text!r}. Assuming a newer/custom build.", stacklevel=2, ) - return False, "" - - if "d8c1b8e" in version_text: - return ( - True, - "released model-converter build d8c1b8e predates required VGF custom " - "shader features; use a newer source build", - ) - - warnings.warn( - "model-converter legacy-version xfail gate expected d8c1b8e; detected " - f"{version_text!r}. Assuming a newer/custom build.", - stacklevel=2, - ) - return False, "" + return True, "" -def xfail_if_legacy_model_converter_release() -> pytest.MarkDecorator: - is_legacy_release, reason = _model_converter_is_legacy_release() - return pytest.mark.xfail(is_legacy_release, reason=reason, strict=False) +def xfail_if_model_converter_below_minimum_version() -> pytest.MarkDecorator: + supports_vgf_tests, reason = _model_converter_supports_vgf_tests() + return pytest.mark.xfail(not supports_vgf_tests, reason=reason, strict=False) def find_single_vgf_json(output_dir: Path) -> Path: diff --git a/backends/arm/test/runtime/test_vgf_aliasing_runtime.py b/backends/arm/test/runtime/test_vgf_aliasing_runtime.py index 1d86d872235..16349abde54 100644 --- a/backends/arm/test/runtime/test_vgf_aliasing_runtime.py +++ b/backends/arm/test/runtime/test_vgf_aliasing_runtime.py @@ -16,11 +16,11 @@ lower_sampler_vgf, lower_threes_vgf, make_sampler_probe_inputs, - xfail_if_legacy_model_converter_release, + xfail_if_model_converter_below_minimum_version, ) from executorch.backends.arm.test import common -pytestmark = xfail_if_legacy_model_converter_release() +pytestmark = xfail_if_model_converter_below_minimum_version() class _ThreesModule(torch.nn.Module): diff --git a/backends/arm/test/runtime/test_vgf_combinations_runtime.py b/backends/arm/test/runtime/test_vgf_combinations_runtime.py index 51c02d71383..b70ae3d4c1b 100644 --- a/backends/arm/test/runtime/test_vgf_combinations_runtime.py +++ b/backends/arm/test/runtime/test_vgf_combinations_runtime.py @@ -18,11 +18,11 @@ lower_threes_vgf, make_sampler_probe_inputs, segment_types, - xfail_if_legacy_model_converter_release, + xfail_if_model_converter_below_minimum_version, ) from executorch.backends.arm.test import common -pytestmark = xfail_if_legacy_model_converter_release() +pytestmark = xfail_if_model_converter_below_minimum_version() def _has_alias_pair(vgf_json: dict, lhs: str, rhs: str) -> bool: diff --git a/backends/arm/test/runtime/test_vgf_multi_segment_runtime.py b/backends/arm/test/runtime/test_vgf_multi_segment_runtime.py index 1d3fba1c00e..85b364cda84 100644 --- a/backends/arm/test/runtime/test_vgf_multi_segment_runtime.py +++ b/backends/arm/test/runtime/test_vgf_multi_segment_runtime.py @@ -17,11 +17,11 @@ make_identity_grid, make_input_tensor, make_sampler_probe_inputs, - xfail_if_legacy_model_converter_release, + xfail_if_model_converter_below_minimum_version, ) from executorch.backends.arm.test import common -pytestmark = xfail_if_legacy_model_converter_release() +pytestmark = xfail_if_model_converter_below_minimum_version() class _GraphThenShader(torch.nn.Module): diff --git a/backends/arm/test/runtime/test_vgf_sampler_image_runtime.py b/backends/arm/test/runtime/test_vgf_sampler_image_runtime.py index d4a8aef150a..0d5c519d568 100644 --- a/backends/arm/test/runtime/test_vgf_sampler_image_runtime.py +++ b/backends/arm/test/runtime/test_vgf_sampler_image_runtime.py @@ -16,11 +16,11 @@ make_identity_grid, make_input_tensor, make_sampler_probe_inputs, - xfail_if_legacy_model_converter_release, + xfail_if_model_converter_below_minimum_version, ) from executorch.backends.arm.test import common -pytestmark = xfail_if_legacy_model_converter_release() +pytestmark = xfail_if_model_converter_below_minimum_version() class _IdentitySampler(torch.nn.Module): diff --git a/backends/arm/test/runtime/test_vgf_tensor_buffer_runtime.py b/backends/arm/test/runtime/test_vgf_tensor_buffer_runtime.py index 21cb4ef2db8..d4be7a04364 100644 --- a/backends/arm/test/runtime/test_vgf_tensor_buffer_runtime.py +++ b/backends/arm/test/runtime/test_vgf_tensor_buffer_runtime.py @@ -18,11 +18,11 @@ lower_in_tree_vgf, make_identity_grid, make_input_tensor, - xfail_if_legacy_model_converter_release, + xfail_if_model_converter_below_minimum_version, ) from executorch.backends.arm.test import common -pytestmark = xfail_if_legacy_model_converter_release() +pytestmark = xfail_if_model_converter_below_minimum_version() class _IdentityGridSample(torch.nn.Module): diff --git a/backends/arm/vgf/model_converter.py b/backends/arm/vgf/model_converter.py index d00a17049c2..e68ec4f57ae 100644 --- a/backends/arm/vgf/model_converter.py +++ b/backends/arm/vgf/model_converter.py @@ -6,14 +6,22 @@ from __future__ import annotations import os +import re import subprocess # nosec B404 - invoked only for trusted local converter tools from dataclasses import dataclass from pathlib import Path from shutil import which from typing import Optional +from packaging.version import InvalidVersion, Version + MODEL_CONVERTER_BINARY = "model-converter" _MODEL_CONVERTER_FALLBACK_BINARY = "model_converter" +MIN_MODEL_CONVERTER_VERSION_FOR_VGF_TESTS = Version("0.10.0") +_MODEL_CONVERTER_VERSION_PATTERN = re.compile(r"\b\d+\.\d+\.\d+(?:[A-Za-z0-9_.+-]*)?\b") +_MODEL_CONVERTER_BUILD_VERSION_ALIASES = { + "d8c1b8e": Version("0.9.0"), +} STATUS_OK = "PASS" STATUS_FAIL = "FAIL" @@ -143,6 +151,67 @@ def _command_output(result: subprocess.CompletedProcess[str]) -> str: return "\n".join(lines[:4]) +def get_model_converter_version_text() -> str | None: + """Return the raw ``model-converter --version`` output, if available.""" + binary = find_model_converter_binary() + if binary is None: + return None + + executable = resolve_model_converter_executable(binary) + if executable is None: + return None + + try: + result = subprocess.run( # nosec B603 - trusted local converter tool + [str(executable), "--version"], + check=True, + capture_output=True, + text=True, + timeout=20, + env=model_converter_env(), + ) + except Exception: + return None + + version_text = (result.stdout or result.stderr).strip() + return version_text or None + + +def parse_model_converter_version(version_text: str) -> Version | None: + """Parse a comparable model-converter version from ``--version`` output.""" + match = _MODEL_CONVERTER_VERSION_PATTERN.search(version_text) + if match is not None: + try: + return Version(match.group(0)) + except InvalidVersion: + pass + + for revision, version in _MODEL_CONVERTER_BUILD_VERSION_ALIASES.items(): + if revision in version_text: + return version + return None + + +def get_model_converter_minimum_version_failure_reason( + version_text: str, + minimum_version: Version, + *, + requirement_name: str, +) -> str | None: + """Return a reason when the installed converter is unsupported. + + The converter is unsupported when it is below ``minimum_version``. + + """ + version = parse_model_converter_version(version_text) + if version is None or version >= minimum_version: + return None + return ( + f"{version_text} is below the minimum supported version " + f"{minimum_version} required for {requirement_name}" + ) + + def check_model_converter_environment() -> ModelConverterEnvironmentCheck: """Check the model-converter dependency used by VGF compilation.""" binary = find_model_converter_binary()