Gtest update #498
Gtest update #498
Conversation
|
bot:retest |
vspetrov
force-pushed
the
topic/gtest_update
branch
from
9ebe916 to
548c0ab
Compare
|
@Sergei-Lebedev @bureddy @manjugv plz review this PR. It is large but simple. All the changes in gtest.h and gtest-all.cc (which are biggest part of the PR) can be ignored in review, since this is just an update of the googletest to the official v1.10 release. All the rest is simple: mostly refactoring of reduction tests - making groups of type+op pairs for cuda/host cases; and adding proper GTEST_SKIP in the unsupported cases. 1 failure in openucx.ucc azure check should also be ignored - there is timeout in clang-format check. apparently 15K lines of code too much for git-clang-format to handle in 60mins. |
test/gtest/common/test_ucc.cc
Outdated
There was a problem hiding this comment.
should we call ucc_collective_finalize to free all successfully initialized requests
There was a problem hiding this comment.
Destructor of UCC_REQ is called later and it takes care of reqs finalize
test/gtest/core/test_mc_reduce.h
Outdated
There was a problem hiding this comment.
some pairs are missing compared to original version e.g. UCC_DT_INT16 max
There was a problem hiding this comment.
I know. We never had ALL the combinations (to limit the time spent in collectives with reductions). I changed it that way: we now test ALL possible combos with test_mc_reduce, AND we test "every possible dtype" and "every possible op" in the reduce coll tests (reduce, allreduce, reduce_scatter), but now every possible combination of them.
Also i added explicitly INT32, FLOAT32, FLOAT64 with all arithmetic ops since those are most commonly used. If you think we should add more plz comment - i will extend.
test/gtest/core/test_mc_reduce.cc
Outdated
There was a problem hiding this comment.
we make pairs so that reduction is always supported then this check is not needed
There was a problem hiding this comment.
For test_mc_reduce (not collective but just reduction backend) i don't enumerate separate dtypes for CUDA. I have the enumerations for ALL possible pairs, but skip not-supported. I could create separate set of testing::types for CUDA only. It will be little bit more explicit but then i can remove test skip here. What do you prefer?
There was a problem hiding this comment.
ok, keep it as is pls, i thought we support all ops and dtypes in cuda
test/gtest/common/test_ucc.h
Outdated
There was a problem hiding this comment.
missing UCC_DT_BFLOAT16
There was a problem hiding this comment.
PREDEFINED_TYPES macro is only used with collectives that do not do any reductions. Do you think we need to add?
There was a problem hiding this comment.
yes, why not, we anyway test different datatypes and I don't think we test bfloat16 copy operation somewhere else
vspetrov
force-pushed
the
topic/gtest_update
branch
from
548c0ab to
1934021
Compare
vspetrov
force-pushed
the
topic/gtest_update
branch
2 times, most recently
from
106219b to
00f69e8
Compare
Don't list all the skipped tests by default
Refactor reduce-ops tests (to support custom tests for CUDA/HOST)
Use GTEST_SKIP and mc_available checks to properly skip
unsupported cases
vspetrov
force-pushed
the
topic/gtest_update
branch
from
00f69e8 to
efbff3d
Compare
|
Merged w/o codestyle/openucx.ucc pass - both time out on the code style format - too large change in gtest.h/gtest_all.cc. |
* TEST: update to gtest 1.10
* TEST: gtest_print_skipped
Don't list all the skipped tests by default
* TEST: gtest improvements
Refactor reduce-ops tests (to support custom tests for CUDA/HOST)
Use GTEST_SKIP and mc_available checks to properly skip
unsupported cases
4 participants
What
A set of gtest improvements: