Parameterize ONNX model tests.

[ScottTodd]

Progress on #6. See how this is used downstream in iree-org/iree#19524.

Overview

This replaces hardcoded flags like

iree_compile_flags = [
    "--iree-hal-target-backends=llvm-cpu",
    "--iree-llvmcpu-target-cpu=host",
]
iree_run_module_flags = [
    "--device=local-task",
]

and inlined marks like

@pytest.mark.xfail(raises=IreeCompileException)
def test_foo():

with a JSON config file passed to the test runner via the --test-config-file option or the IREE_TEST_CONFIG_FILE environment variable.

During test case collection, each test case name is looked up in the config file to determine what the expected outcome is, from ["skip" (special option), "pass", "fail-import", "fail-compile", "fail-run"]. By default, all tests are skipped. This design allows for out of tree testing to be performed using explicit test lists (encoded in a file, unlike the -k option), custom flags, and custom test expectations.

Design details

Compare this implementation with these others:

• https://github.com/iree-org/iree-test-suites/tree/main/onnx_ops also uses config files, but with separate lists for skip_compile_tests, skip_run_tests, expected_compile_failures, and expected_run_failures. All tests are run by default.
• https://github.com/nod-ai/SHARK-TestSuite/blob/main/alt_e2eshark/run.py uses --device=, --backend=, --target-chip=, and --test-filter= arguments. Arbitrary flags are not supported, and test expectations are also not supported, so there is no way to directly signal if tests are unexpectedly passing or failing. A utility script can be used to diff the results of two test reports: https://github.com/nod-ai/SHARK-TestSuite/blob/main/alt_e2eshark/utils/check_regressions.py.
• https://github.com/iree-org/iree-test-suites/blob/main/sharktank_models/llama3.1/test_llama.py parameterizes test cases using @pytest.fixture([params=[...]]) with pytest.mark.target_hip and other custom marks. This is more standard pytest and supports fluent ways to express other test configurations, but it makes annotating large numbers of tests pretty verbose and doesn't allow for out of tree configuration.

I'm imagining a few usage styles:

• Nightly testing in this repository, running all test cases and tracking the current test results in a checked in config file.
  • We could also go with an approach like https://github.com/nod-ai/SHARK-TestSuite/blob/main/alt_e2eshark/utils/check_regressions.py to diff test results but this encodes the test results in the config files rather than in external reports. I see pros and cons to both approaches.
• Presubmit testing in https://github.com/iree-org/iree, running a subset of test cases that pass, ensuring that they do not start failing. We could also run with XFAIL to get early signal for tests that start to pass.
  • If we don't run with XFAIL then we don't need the generalized tests_and_expected_outcomes, we could just limit testing to only models that are passing.
• Developer testing with arbitrary flags.

Follow-up tasks

• Add job matrix to workflow (needs runners in this repo with GPUs)
• Add an easy way to update the list of XFAILs (maybe switch to https://github.com/gsnedders/pytest-expect and use its --update-xfail?)
• Triage some of the failures (e.g. can adjust tolerances on Vulkan)
• Adjust file downloading / caching behavior to avoid redownloading and using significant bandwidth when used together with persistent self-hosted runners or github actions caches

[ScottTodd]

I'm looking for some early feedback, @zjgarvey or others. Early testing shows that running these tests on HIP would have caught some recent regressions. The specific mechanics used for setting flags, choosing which tests to run, and checking or reporting which stages passed/failed can be implemented in multiple ways.

[ScottTodd]

• https://github.com/nod-ai/SHARK-TestSuite/blob/main/alt_e2eshark/run.py uses --device=, --backend=, --target-chip=, and --test-filter= arguments. Arbitrary flags are not supported, and test expectations are also not supported, so there is no way to directly signal if tests are unexpectedly passing or failing.

Here are some ideas, adding complexity in the conftest file but making it more flexible:

• Add more options here to use instead of --test-config-file, like --iree-compile-flags
• Make every option a flag then use a flagfile pattern like https://stackoverflow.com/a/27434050 so "config.json" is just a collection of regular flags
  • Not sure how that would work with the lists of tests with expectations... I like having the full list of tests that will run be sorted and not split between groups like in the onnx op tests
• Could load a .py file that has the list of tests and statuses, or even just implements the pytest_collection_modifyitems hook

[ScottTodd]

Using https://github.com/gsnedders/pytest-expect or https://github.com/projectcaluma/pytest-xfaillist is also an option.

Both support updating the xfail files via a --update-xfail or --generate-xfaillist option. Neither supports xfail reasons, but that's not critical.

https://github.com/projectcaluma/pytest-xfaillist seems to only support a hardcoded xfails.list file name next to the config root (source here), which wouldn't work with separate lists of xfails depending on the configuration (e.g. backend choice)

https://github.com/gsnedders/pytest-expect does allow specifying a file with --xfail-file

---

What I have right now in this "config JSON file" groups these three items:

1. The flags to use when compiling and running, allowing you to choose between CPU or GPU, for example
2. The list of tests to run
3. The list of tests that are expected to fail (and how)

We'll always want a custom implementation for (1). For (2), https://community.lambdatest.com/t/how-to-run-pytest-tests-from-a-list-of-test-paths/31682/2 has some answers but I don't see an existing plugin or standard convention. For (3), we could use one of those projects.

[zjgarvey]

Moving the expected outcomes to a json is a good idea for making it customizable to the different configs for each test.

I think it will probably be helpful to have the ability to specify extra test-specific options for an individual test through the json file. E.g. for large models, it might be useful to pass additional importer and runtime flags for externalizing params. Although I'm not sure if models that large are going to be in the scope of this testing suite right now.

One thing I think will be rather helpful is to have a way to take a list of urls and automatically generate the test functions. For example:

from typing import List

test_urls = [
      "https://github.com/onnx/models/raw/main/validated/vision/object_detection_segmentation/faster-rcnn/model/FasterRCNN-12.onnx",
      "https://github.com/onnx/models/raw/main/validated/vision/object_detection_segmentation/fcn/model/fcn-resnet50-12.onnx",
]

def generate_name(url: str):
    split = url.split("/")
    return f'test_{split[-3]}_{split[-1].removesuffix(".onnx")}'

def make_function(url: str):
    def func(compare_between_iree_and_onnx_runtime):
        compare_between_iree_and_onnx_runtime(
            model_url=url,
            artifacts_subdir=artifacts_subdir,
        )
    return func

def define_functions(url_list: List[str]):
    for url in url_list:
        globals()[generate_name(url)] = make_function(url)

[ScottTodd]

I think it will probably be helpful to have the ability to specify extra test-specific options for an individual test through the json file.

Yes! However, I'm on the fence about specifically where to support extra options. We can add options in all these places:

• In the test runner (conftest.py)
• In individual test cases within files (test_foo.py)
• In configuration files (config.json)
• As parameters on environment variables (e.g. IREE_HIP_TEST_TARGET_CHIP)

I don't want to confuse developers with too many choices, but flexibility can helpful for a variety of situations.

E.g. for large models, it might be useful to pass additional importer and runtime flags for externalizing params.

For example here, that sounds like those options should apply regardless of the backend configuration, so the flags could go in the test cases.

Although I'm not sure if models that large are going to be in the scope of this testing suite right now.

I think they should be, as we can choose which tests to run on what schedule. How I have the PR right now allows for the list of tests to be fully opt-in using the default "skip" behavior. You can also filter with pytest:

# run only tests matching a string
-k resnet

# skip all large tests
-m "not size_large"

We can also use something like https://pypi.org/project/pytest-shard/ to run across multiple machines.

---

One thing I think will be rather helpful is to have a way to take a list of urls and automatically generate the test functions.

Yeah! I like how easy it looks to add test cases when they are in files like https://github.com/nod-ai/SHARK-TestSuite/blob/main/alt_e2eshark/onnx_tests/models/external_lists/onnx_model_zoo_computer_vision_1.txt. One of the tradeoffs I'm considering here is how it isn't obvious at all from a single test case name what the test is actually doing. I'd like for test suites to be forkable into user code, not be their own world of meta programming. The new tests that Rob added in https://github.com/iree-org/iree-test-suites/blob/main/sharktank_models/llama3.1/test_llama.py are on the other side of that spectrum:

Resemble user code                                               Templated to run lots of tests
<--------------------------------------------------------------------------------------------->

sharktank_models/                                          onnx_models/           alt_e2eshark/

What's in this test suite right now is closer to alt_e2eshark in that there is code for each test case... but that code is nearly all boilerplate:

def test_age_gender_gender_googlenet(compare_between_iree_and_onnxruntime):
    compare_between_iree_and_onnxruntime(
        model_url="https://github.com/onnx/models/raw/main/validated/vision/body_analysis/age_gender/models/gender_googlenet.onnx",
        artifacts_subdir=artifacts_subdir,
    )

I can iterate on further parameterization as you suggest here... maybe using pytest.mark.parametrize (docs here) or one of the more powerful options.

The ergonomics question is partially solved by

1. keeping the test runner code organized and documented
2. having the tests log what they are doing (iree-import-onnx -> iree-compile -> iree-run-module), since then users can run the tests and copy the log output for their own uses

[ScottTodd]

Thanks again for the review comments @zjgarvey . I'm planning to pick this back up soon.

[ScottTodd]

One thing I think will be rather helpful is to have a way to take a list of urls and automatically generate the test functions.

How's this @zjgarvey ?

Some details:

• If no marks are needed (all models in the list are supported by the test suite, no "large" or other special model tags needed), then the pytest.param() wrappers could be removed, for just:

  @pytest.mark.parametrize(
      "model",
      [
          "duc/model/ResNet101-DUC-12.onnx",
          "faster-rcnn/model/FasterRCNN-12.onnx",
          "fcn/model/fcn-resnet50-12.onnx",
          "mask-rcnn/model/MaskRCNN-12.onnx",

• Using fmt: off to prevent the formatter from wrapping lines, so each test case gets its own line, even if it becomes very long

• Test function names could be customized here: https://stackoverflow.com/questions/37575690/override-a-pytest-parameterized-functions-name . Along with that, I could change how the setup code in conftest.py decides which test cases to modify, using regex match or some shorthand, instead of the explicit

    "tests_and_expected_outcomes": {
      "tests/model_zoo/validated/vision/classification_models_test.py::test_models[inception_and_googlenet/inception_v1/model/inception-v1-12.onnx]": "fail-compile",

  The explicit format is copy-paste friendly but not typing friendly :P

• I figure for any test cases that need extra arguments, they could be their own groups that call compare_between_iree_and_onnxruntime or another test helper function, instead of packing more options in to this @pytest.mark.parametrize

[zjgarvey]

Yeah, that makes sense to me.

I'll go through and take another look at this PR when I get the chance. If I don't get to it for a while and you'd like a review, please feel free to message/ping me.

[zjgarvey]

Actually, I was able to look through this pretty quickly now, and it looks like a good change to me. What else needs to be done to undraft this PR?

[ScottTodd]

Actually, I was able to look through this pretty quickly now, and it looks like a good change to me. What else needs to be done to undraft this PR?

I'm not totally content with a few design points yet, but this could be good enough to merge and start using. I'll iterate a bit on the PR description and docs so they reflect the current status then mark as ready for review. Thanks for taking a look!

[ScottTodd]

Follow-up tasks

• Adjust file downloading / caching behavior to avoid redownloading and using significant bandwidth when used together with persistent self-hosted runners or github actions caches

For a sense of scale, the onnx_models/artifacts/ directory is around 34GB on my Windows system right now, including .mlir and .vmfb files. I don't want CI runs to redownload 10GB+ from GitHub each job run, since I think that cuts in to the Git LFS bandwidth quota for https://github.com/onnx/models . The docs at https://docs.github.com/en/repositories/working-with-files/managing-large-files/about-storage-and-bandwidth-usage say that quota comes from the repository owner, not the user, so I want to be a good citizen here.

I may take ideas from #59 (comment) and build some caching layer that can be shared across the test suites here.