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.

Sign up for GitHub

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

Jump to bottom

Fix tests #47

Merged
merged 3 commits into from
Mar 30, 2024
Merged

Fix tests #47

merged 3 commits into from
Mar 30, 2024

Conversation

sthibaul
Copy link
Contributor

The test_mpi_indexed_gets test is currently failing spuriously in debian unstable due to an uninitialized value, e.g. https://ci.debian.net/packages/a/armci-mpi/testing/amd64/44486187/ , preventing the newer mpich version from migrating to unstable.

@jeffhammond
Copy link
Member

all the fabs look good. initializing the buffer is also good, although i haven't thought about whether it should be initialized to zero instead. i do not understand the XDIM change.

@jeffhammond
Copy link
Member

the contribution is very much appreciated, of course.

@sthibaul
tests/mpi/test_mpi_indexed_gets.c: Fix reception buffer initialization
80cc917 
loc_buf was completely uninitialized, while the second and third loop nests
are checking that the values are 1.0 + rank. With luck it would be zero, and
thus the actual - expected > 1e-10 check would pass (since the difference is
then negative). With less luck the check would spuriously fail for the part
that is not overwritten by the MPI_Get.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
@sthibaul
tests/mpi: Fix comparison of floating-double types
9c0f6b0 
In case the actual value is zero and the expected value is positive, the
check would spuriously succeed.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
@sthibaul
tests/mpi/test_mpi_indexed_gets.c: Strengthen test
cd001a4 
Now that it is fixed, we can make it actually perform strided accesses.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
@sthibaul
Copy link
Contributor Author

The initialize needs to be 1.0+rank, the second and third loop nests actually check that (to check that MPI_Get properly intermixes values reception, not overwriting everything). I have improved the log message to mention that.

@jeffhammond jeffhammond merged commit a133ab0 into pmodels:master Mar 30, 2024
@jeffhammond
Copy link
Member

i recall i had to do a release for debian long ago, or something like that, because they don't just pull from git HEAD. do i need to do that again?

@sthibaul
Copy link
Contributor Author

Debian can cherry-pick fixes when they are committed upstream

@sthibaul
Copy link
Contributor Author

(I plan to do it actually)

@jeffhammond
Copy link
Member

ok, cool. i think it's good now but please let me know if you need anything else.

@sthibaul sthibaul deleted the fix-tests branch March 30, 2024 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@sthibaul @jeffhammond