Skip to content

Test Filter Standardization POC - Changes to integrate MIOpen - #3097

Closed
dileepr1 wants to merge 27 commits into
mainfrom
users/dravindr/ctest_miopen
Closed

dileepr1 wants to merge 27 commits into
mainfrom
users/dravindr/ctest_miopen

Conversation

@dileepr1

Copy link
Copy Markdown
Contributor

Motivation

This PR is to integrate the Test Filter Standardization POC done in MIOpen to TheRock

Technical Details

The POC in MIOpen is done here: ROCm/rocm-libraries#3513.
This PR modifies test_miopen.py to call the required test categories with CTest.

@jayhawk-commits

Copy link
Copy Markdown
Contributor

A lot of the functions introduced here are not specific to MIOpen. Please move those functions to another python script, that can be leveraged for all ROCm projects.

@dileepr1
dileepr1 force-pushed the users/dravindr/ctest_miopen branch from 3cc11fd to f8fc406 Compare January 27, 2026 21:14
@dileepr1
dileepr1 force-pushed the users/dravindr/ctest_miopen branch from f8fc406 to d436a36 Compare January 27, 2026 21:21
@dileepr1
dileepr1 requested a review from a team as a code owner January 28, 2026 14:43

@geomin12 geomin12 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i notice it is failing, but this is not sharded!

Comment thread rocm-libraries

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this is for testing, but let's remember to remove this before merge in

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes for sure and yes, it is for testing.
Happy to know that there are more set of eyes on this!

"8",
"--test-dir",
f"{THEROCK_BIN_DIR}/MIOpen",
"-V", # Always run in verbose mode

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's add this

# shards the tests by running a specific set of tests based on starting test (shard_index) and stride (total_shards)
"--tests-information",
f"{shard_index},,{total_shards}",

so it provides sharding

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is done!

category = "standard"

# Use AMDGPU_FAMILIES from environment variable, extract gfx<xxx> part
gpu_arch = "gfx1151" # default

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prob would avoid a "default" arch, just an empty string is fine. we'll be guaranteed to have a gpu arch, and if none, there's a bigger problem at hand

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, will change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Dileep Ravindranathan and others added 8 commits February 6, 2026 13:38
…ategory}-{gpu_arch}-exclude to {target_name}-{category}-{gpu_arch}-suite
…job and add `rocblas` to the component directory mapping in `test_filters.py`.
…ocblas` job and add `rocblas` to the component directory mapping in `test_filters.py`."

This reverts commit 9efe697.
@dileepr1

Copy link
Copy Markdown
Contributor Author

I'm closing this PR since it has got multiple commits pointing to local rocm-libraries branches for testing.
Created a new PR #3595 with the required changes.

@dileepr1 dileepr1 closed this Feb 24, 2026
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage Feb 24, 2026
@dileepr1
dileepr1 deleted the users/dravindr/ctest_miopen branch March 13, 2026 18:47
dileepr1 added a commit to ROCm/rocm-libraries that referenced this pull request Jun 24, 2026
## Summary

Adds YAML-driven CTest test categorization for the **miopen-provider**
plugin so TheRock CI can drive `ctest -L
{quick|standard|comprehensive|full}` from the installed
`bin/miopen_plugin/` tree via the generic `test_runner.py` (replacing
the legacy `test_miopenprovider.py` one-off).
More details available at
[test_filtering.md](https://github.com/ROCm/TheRock/blob/main/docs/development/test_filtering.md).

Paired TheRock branch:
[`users/dravindr/tr_miopen_prov`](https://github.com/ROCm/TheRock/tree/users/dravindr/tr_miopen_prov)
(PR forthcoming) — switches the `miopenprovider` job's `test_script` to
`test_runner.py`, adds `miopenprovider -> miopen_plugin` to
`COMPONENT_DIR_MAPPING`, and broadens `artifact-miopenprovider.toml`'s
test include to `bin/miopen_plugin/**`.

Reference (canonical pattern):
[`#3513`](#3513)
+ [`ROCm/TheRock#3097`](ROCm/TheRock#3097)
(miopen).

## Changes

### `dnn-providers/miopen-provider/test_categories.yaml` (new)
Added the test_categories.yaml for test selection

### `dnn-providers/miopen-provider/CMakeLists.txt`
- Set `ROCM_LIBRARIES_ROOT` (guarded `if NOT DEFINED`) so
`shared/ctest/TestCategories.cmake` is discoverable from the monorepo
root.
- After `finalize_test_targets(...)`, call
`apply_test_category_labels()` once per test binary, writing both sets
of `_quick_suite` / `_standard_suite` / `_comprehensive_suite` /
`_full_suite` entries into a single install-time `CTestTestfile.cmake`
with relative paths (`../miopen_plugin_tests`,
`../miopen_plugin_integration_tests`).
- Install the YAML-generated `CTestTestfile.cmake` to
`bin/miopen_plugin/` when shared/ctest is available; otherwise fall back
to the project's existing `install_provider_ctest_files()` so standalone
/ sparse checkouts that lack `shared/ctest/` keep working with their
current unlabeled install file.

### `.github/actions/ci-env/action.yml` *(WIP — revert before merge)*
Bumps the `therock-ref` output from the pinned 2026-05-29 SHA
(`685f8c3b890063...`) to the paired TheRock branch
`users/dravindr/tr_miopen_prov`. This file is the single source of truth
for `therock-ci-linux.yml`, `therock-ci-windows.yml`, `therock-ci.yml`,
`therock-test-component.yml`, and `therock-test-packages.yml`, so this
one edit replaces what would otherwise be four parallel diffs.

**Must be reverted to a pinned `ROCm/TheRock` SHA before merge**, once
the paired TheRock PR lands and gives us a stable commit on `main`.

## Test plan

- [x] CI build (Linux) on `users/dravindr/tr_miopen_prov` produces a
`miopenprovider` artifact whose `bin/miopen_plugin/CTestTestfile.cmake`
lists all 8 `*_suite` entries with labels.
- [x] CI build (Windows) ditto.
- [x] `therock-test-component.yml` / `therock-test-packages.yml` runs of
the `miopenprovider` job invoke `test_runner.py` with
`TEST_COMPONENT=miopenprovider` and `TEST_TYPE=quick` and pass.
- [x] Revert `.github/actions/ci-env/action.yml` to a pinned
`ROCm/TheRock` SHA once the paired TheRock PR is merged.

Made with [Cursor](https://cursor.com)

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants