Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GTest improvements #3140

Open
1 of 6 tasks
CAHEK7 opened this issue Jul 23, 2024 · 3 comments
Open
1 of 6 tasks

GTest improvements #3140

CAHEK7 opened this issue Jul 23, 2024 · 3 comments

Comments

@CAHEK7
Copy link
Contributor

CAHEK7 commented Jul 23, 2024

It's an umbrella ticket for better tracking of GTest improvement activity ordered by priority.

The tickets under the different checkboxes are independent and can be done in-parallel, while tickets under the same checkbox must be done sequential and in a predefined order.

@junliume
Copy link
Collaborator

junliume commented Aug 8, 2024

Recommend to add to the list of investigation:

why skipping a test is taking so long? It does not make sense to spend 3 seconds just to skip a test IMHO

379/8695 Test  #381: Full/GPU_Adam_FP16.AdamFloat16TestFw/adam_w input:1 lr:0.001 beta1:0.9 beta2:0.999 weight_decay:0 eps:1e-06 amsgrad:0 maximize:1 ..................................................................................................................................................................................................................................................................................................***Skipped   3.05 sec

@CAHEK7
Copy link
Contributor Author

CAHEK7 commented Aug 8, 2024

Recommend to add to the list of investigation:

why skipping a test is taking so long? It does not make sense to spend 3 seconds just to skip a test IMHO

379/8695 Test  #381: Full/GPU_Adam_FP16.AdamFloat16TestFw/adam_w input:1 lr:0.001 beta1:0.9 beta2:0.999 weight_decay:0 eps:1e-06 amsgrad:0 maximize:1 ..................................................................................................................................................................................................................................................................................................***Skipped   3.05 sec

@junliume because they initialize the buffers and skip the test. We have A LOT of similar issues.

It is already described in https://github.com/ROCm/MIOpen/wiki/GTest-development#early-skip

Exit from the test as early as possible. Any test skipping functionality must reside in void SetUp() override. For example, right now 198 AddLayerNorm tests take around 20s to skip, but if we move skip routine at the beginning of the SetUp, it takes 1ms

It happened because initially we've added this evil patter to all GTESTS and even for our "smoke" tests, we initialize ALL the buffers for most of the tests disabled by MIOPEN_TEST_ALL or restricted by the particular data type, because most of the tests check MIOPEN_TEST_ALL and MIOPEN_FLOAT inside the test body right after all the initialization.

@junliume
Copy link
Collaborator

junliume commented Aug 9, 2024

Thank you for the detailed explanation @CAHEK7

we've added this evil patter to all GTESTS and even for our "smoke" tests

Let's discuss further on ways to resolve these issues :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants