-
Notifications
You must be signed in to change notification settings - Fork 13
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
Big Count Collectives Test Suite #15
Conversation
db878fb
to
d73c295
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What a great PR. I commented on a few minor nits.
A few enhancements for future features could include:
o testing other reduction ops other than MPI_SUM
o Test MPI_Reduce_Local.
o Test new embigening in MPI-4.0 standard.
# count = TEST_PAYLOAD_SIZE / sizeof(datatype) | ||
###################################################################### | ||
# INT_MAX : == 2 GB so guard will not trip | ||
TEST_PAYLOAD_SIZE=2147483647 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we want this TEST_PAYLOAD_SIZE to be BIGGER than TEST_UNIFORM_COUNT? say 4 GB or 4GB + a few KB?
README.md (at the top) seems to suggest that the default TEST_PAYLOAD_SIZE would be UINT_MAX ???
See: https://github.com/open-mpi/ompi-tests-public/pull/15/files#diff-26cf9c4e5070bbfe5af943597e4ee73210cb78749e416e5bcb00fb204331f490R98
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can, but it should be relative to the datatype. So we don't want to set it to INT_MAX x 8
because then the count would be too big for an int
datatype. I left it as a default of INT_MAX
and folks can change it as they see fit. The TEST_UNIFORM_COUNT
is probably the more useful of the two variants.
* Run collectives with a `count` parameter as close to `INT_MAX` as possible. - This test suite often highlights cases where the underlying algorithm assumes that the payload (roughly `count x sizeof(datatype)`) is and `int` when it should be handled as a `size_t`. * Includes: - Test with `int` and `double _Complex` primitive data types - Correctness checks - Mechanism to control (as best as we can) the amount of memory consumed per node. * Assumes: - Roughly the same amount of memory per node - Same number of processes per node * See `README.md` for details Signed-off-by: Joshua Hursey <jhursey@us.ibm.com>
d73c295
to
6332a0e
Compare
I just force pushed the resolved changes from the review. |
Discussed on March 1, 2022 teleconf. Ok to merge. |
count
parameter as close toINT_MAX
as possible.assumes that the payload (roughly
count x sizeof(datatype)
) isand
int
when it should be handled as asize_t
.int
anddouble _Complex
primitive data typesper node.
README.md
for detailsSigned-off-by: Joshua Hursey jhursey@us.ibm.com