Repository navigation
Changes to integrate Test Filter Standardization for miopen in TheRock - #3595
Conversation
geomin12
left a comment
geomin12
left a comment
There was a problem hiding this comment.
looks like tests are passing! just a few comments
…that it may be reused
… gpu specific tests
| 1. The script is invoked by the **Test component** workflow with env vars set (component name, GPU arch, test type, sharding). | ||
| 1. The script expects the component under test to have a ctest based interface, with labels corresponding to test categories like quick, standard, comprehensive and full(which can be run for scenarios like smoke test, pre-commit etc) | ||
| 1. The component can opt to always exclude some tests or based on condition like OS and GPU where the test is getting run | ||
| 1. The script discovers which **GPU-specific test suites** exist by running `ctest --print-labels` and parsing labels of the form `ex_gpu_{gpu_arch}`. |
There was a problem hiding this comment.
We're discussing how to handle systems having variable GPU memory. This is being driven by APUs such as Strix Halo. We probably don't need a complete answer for this PR, but it wouldn't hurt to start thinking about it.
There was a problem hiding this comment.
The particular issue is that many existing tests assuming having a certain amount of GPU memory. For MIOpen, this wasn't part of the design so we don't know the actual minimum, but roughly 12GB. We were seeing lots of failures on a Strix Halo having 16GB system memory, in which case the memory allocated to GPU was appx 4GB 'dedicated' and 4.9GB 'shared' for a total of 8.9GB.
There was a problem hiding this comment.
I think this can be easily handled if we know the gfx models and their available memory and the memory needed for each of the tests before hand by using the exclude_gpu_gfx* section in test_categories.yaml.
Automation may be possible by adding the memory details of gpus and tests in the yaml file as well.
But it will need more thought in case the same gpu model can have multiple memory values in the test set up.
There was a problem hiding this comment.
Unfortunately, the latter is the case on APU. CPU and GPU memory come from the same physical RAM sticks and the OS divides it up. GPU memory can be user-modified not only by changing the system RAM, but also modifying firmware and/or software settings. The memory configuration has to be queried before each run just prior to launching the tests.
geomin12
left a comment
geomin12
left a comment
There was a problem hiding this comment.
lgtm! let's wait for CI to pass properly then can approve
geomin12
left a comment
geomin12
left a comment
There was a problem hiding this comment.
FYI for future reference, you can add test:miopen label to only test miopen (no need to wait for all tests): https://github.com/ROCm/TheRock/blob/main/docs/development/ci_behavior_manipulation.md
## Motivation Code unrelated to the github actions API keeps landing in this file as if it were a general utility: * #2873 (comment) * #3595 (comment) See the comment at the top of the file: https://github.com/ROCm/TheRock/blob/ca2a06c6c9fa47a21e21d1c6f0c133e9566eeb05/build_tools/github_actions/github_actions_utils.py#L4-L7 ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
## Motivation Bumping the rock hash to include rcom-system fixes ROCm/TheRock@4b49136 This should also include the work for the test filter standardization: ROCm/TheRock#3595 ## Technical Details Update the rock hash ## Test Plan Tests Pass for miopen and other projects Check if test filter work is included here ## Test Result <!-- Briefly summarize test outcomes. --> ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
## Motivation Moving the function out of github_actions_utils.py as per [this](#3595 (comment)) comment in #3595. ## Technical Details NA ## Test Plan CI tests handled by test_runner.py should continue to run. ## Test Result NA ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. --------- Co-authored-by: Dileep Ravindranathan <dileep@dell-rack-13.amd.com>
## Motivation Bumping the rock hash to include rcom-system fixes ROCm/TheRock@4b49136 This should also include the work for the test filter standardization: ROCm/TheRock#3595 ## Technical Details Update the rock hash ## Test Plan Tests Pass for miopen and other projects Check if test filter work is included here ## Test Result <!-- Briefly summarize test outcomes. --> ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
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 adds test_filters.py as a generic runner to call the required test categories from MIOpen, and can be expanded to other components.
Test Plan
Test Result
Once active, the MIOpen CI tests will be triggered through this.