MIOpen icon indicating copy to clipboard operation
MIOpen copied to clipboard

[Testing] Convert MIOpenDriver tests into shell script

Open xinlipn opened this issue 10 months ago • 7 comments

The shell script has dependency on the binary, MIOpenDriver

xinlipn avatar Apr 08 '24 07:04 xinlipn

  1. Create a shell script for MIOpenDriver tests starting from the line below, which will be packed along with single gTest binary, miopen_gtest. https://github.com/ROCm/MIOpen/blob/af3c2ded9cf1186c17948dedc8e34b0fba050716/test/CMakeLists.txt#L684

  2. Shell script depends on the binary, /MIOpen/build/bin/MIOpenDriver. Feel free to advise if hardcoding the path is not preferred https://github.com/ROCm/MIOpen/blob/af3c2ded9cf1186c17948dedc8e34b0fba050716/miopendriver_test.sh#L128

  3. These two env vars, MIOPEN_TEST_WITH_MIOPENDRIVER and MIOPEN_TEST_ALL, are used to control miopen tests

  4. Also feel free to advise if there's a more appropriate directory to put the shell script instead of /MIOpen/

  5. Test logs attached. Tested on 10.7.47.119 (MI200), with rocm/miopen:ci_fd8d66 miopendriver_test.log

xinlipn avatar Apr 11 '24 07:04 xinlipn

Adding @atamazov , this looks like adding a functionality which will eventually replace/convert an existing one, I would suggest moving forward if no objections.

junliume avatar Apr 11 '24 18:04 junliume

Unfortunately does not comply #1843 (2) and other things. I recommend implementing these tests just like we do other tests (but set normal environment and then exec MIOpenDriver with necessary command-line arguments instead of instantiating and using test classes from the ./test directory).

Hi @atamazov, thanks for the input. Unlike converting other cTests to gTest by implementing test classes and pack them into a single binary as this PR below, what the script does is to call MIOpenDriver binary with arguments like what you commented.

https://github.com/ROCm/MIOpen/pull/2806

The shell script has dependency on MIOpenDriver binary and also needs to have environment variables set. The reason is

  1. Previous MIOpenDriver test didn't use the test drivers, e.g. conv2d_driver, a shell script seems to be a feasible solution at this stage after discussion. This script along with the single gTest binary miopen_gtest will be delivered and pack to other internal teams.

  2. Although this shells script isn't part of cTest but it meant to have the same coverage as cTest. From this perspective, it's not meant to be standalone, so I kept the same logic and environment variable i.e. MIOPEN_TEST_ALL

Thanks

xinlipn avatar Apr 15 '24 19:04 xinlipn

@xinlipn

Unlike converting other cTests to gTest by implementing test classes and pack them into a single binary as this PR below, what the script does is to call MIOpenDriver binary with arguments like what you commented.

What I am proposing is implementing the minimum test classes and calling MIOpenDriver from within the test, just like the script does. The advantage is that the test can work standalone -- it can check the GPU type, XNACK feature etc. But of course is will require the driver to exist, i.e. there is a dependence.

atamazov avatar Apr 16 '24 13:04 atamazov

@xinlipn

Unlike converting other cTests to gTest by implementing test classes and pack them into a single binary as this PR below, what the script does is to call MIOpenDriver binary with arguments like what you commented.

What I am proposing is implementing the minimum test classes and calling MIOpenDriver from within the test, just like the script does. The advantage is that the test can work standalone -- it can check the GPU type, XNACK feature etc. But of course is will require the driver to exist, i.e. there is a dependence.

Thanks for the comment. This script can indeed detect/check GPU type at runtime as below we can check other features too if they are relevant to MIOpenDriver tests. https://github.com/ROCm/MIOpen/blob/aaaf0be5c442c14faacd6402a57146aa59483601/miopendriver_test.sh#L86

This design offers a quick solution and hopefully same logic/coveratge as current MIOpenDriver tests to be part of the test package along with the stand alone gTest binary, miopen_gtest. We can do it with a class as proposed in another implementation/PR.

FYI, this script does run by itself (given the presence of MIOpenDriver) if this makes sense. Thanks

xinlipn avatar Apr 16 '24 14:04 xinlipn

@xinlipn

Unlike converting other cTests to gTest by implementing test classes and pack them into a single binary as this PR below, what the script does is to call MIOpenDriver binary with arguments like what you commented.

What I am proposing is implementing the minimum test classes and calling MIOpenDriver from within the test, just like the script does. The advantage is that the test can work standalone -- it can check the GPU type, XNACK feature etc. But of course is will require the driver to exist, i.e. there is a dependence.

Creating a gTest to run MIOpenDriver is probably a better way than shell script. Will close this PR and create a new one for new implementation.

xinlipn avatar Apr 24 '24 05:04 xinlipn

I guess it can be closed, since #2984 has been merged.

CAHEK7 avatar Jun 10 '24 10:06 CAHEK7