Convert softmax from CTest to GTest #3479
Conversation
Vsevolod1983
requested review from
BrianHarrisonAMD and
BradPepersAMD
as code owners
| CompareResults(tensorGpuDataBackward, tensorCpuDataBackward); | ||
| } | ||
|
|
||
| std::vector<T> GetForwardCpu() const |
There was a problem hiding this comment.
we also have mloSoftmaxForwardRunHost and mloSoftmaxBackwardRunHost too.
There was a problem hiding this comment.
Since this one is parallel may be we can move this mloSoftmaxForwardRunHost and mloSoftmaxBackwardRunHost to avoid duplications.
There was a problem hiding this comment.
hi, @bghimireamd
In C-test I did not see calls of these functions. Scope of my refactoring was to replace c-test with g-test.
If I misunderstand something, lets discuss.
There was a problem hiding this comment.
Or do you mean that we need to enhance test coverage for softmax? If yes I would prefer to do it in separate PR/ticket.
There was a problem hiding this comment.
yeah separate PR/ticket make more sense here :)
test/gtest/soft_max.cpp
Outdated
| * Copyright (c) 2017 Advanced Micro Devices, Inc. | ||
| * Copyright (c) 2024 Advanced Micro Devices, Inc. |
test/gtest/soft_max.cpp
Outdated
| // taken from the original c test | ||
| double tolerance = 8000; | ||
|
|
||
| /// \todo Resolve this workaround. Regular failures on Radeon VII, ROCm 3.3: | ||
| /// --float --input-dim 1 1 8 8 --algorithm 0 --mode 1 --scales 1 0 --tolerance 8000 | ||
| /// FAILED: -nan | ||
| in_dim_set.erase({1, 1, 8, 8}); | ||
| in_dim_set.erase({1, 1, 14, 14}); | ||
| in_dim_set.erase({1, 1, 27, 27}); | ||
| in_dim_set.erase({1, 32, 7, 7}); | ||
| in_dim_set.erase({1, 32, 8, 8}); | ||
| double threshold = std::numeric_limits<T>::epsilon() * tolerance; | ||
| double error = miopen::rms_range(tensorCPUData, tensorGPUData); |
There was a problem hiding this comment.
This seems like a pretty large tolerance.
For float it might not be as large, but for half I think this would be very substantial.
Is it possible to reduce this, and still have the tests pass?
There was a problem hiding this comment.
Decreased for half to 80
CTest to GTest conversion for Softmax
soft_max.cpp file is detected as renamed (moved), so probably will not be easy to review.