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

Tpetra: Pass initialization tests with CUDA_LAUNCH_BLOCKING off #7967

Conversation

MicheldeMessieres
Copy link
Contributor

Since packages are getting updated to work with CUDA_LAUNCH_BLOCKING=0
and Tpetra is completed, this allows the unit test suite to all pass for Tpetra.

The tests were checking for CUDA_LAUNCH_BLOCKING off and failing based on std::cerr output.
This keeps the warning but changes it to std::cout so the Tpetra test won't detect it.

I expect we'll want to update this warning again when Trilinos is all working with launch blocking off. Perhaps warn that user code needs fencing if they have it off and warn that they might want to turn it off for efficient Trilinos code, if they have it on.

Note we'll also need #7964 to get the tests passing on white Pascal nodes with blocking off.

@trilinos/tpetra

Motivation

Get Tpetra all passing for CUDA_LAUNCH_BLOCKING=0

Testing

Cuda white Pascal node - Tpetra Core tests

Since packages are getting updated to work with CUDA_LAUNCH_BLOCKING=0
and Tpetra is completed, this allows the unit test suite to all pass.

The tests were checking for CUDA_LAUNCH_BLOCKING off and failing based on std::cerr output.
This keeps the warning but changes it to std::cout so the Tpetra test won't detect it.
@MicheldeMessieres MicheldeMessieres force-pushed the tpetraPassInitializationWithLaunchBlockingOff branch from 5887df7 to 27d9490 Compare September 2, 2020 18:42
Copy link
Member

@csiefer2 csiefer2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thought is to leave this test failing until we're convinced the stack works without launch blocking.

But @kddevin might have other ideas.

@trilinos-autotester
Copy link
Contributor

Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
THE LAST COMMIT TO THIS PULL REQUEST HAS BEEN REVIEWED, BUT NOT ACCEPTED OR REQUIRES CHANGES

@kddevin
Copy link
Contributor

kddevin commented Sep 2, 2020

@MicheldeMessieres I see that you only are changing std::cerr to std::cout. Is there a problem with writing to std::cerr? Does TRIBITS / Ctest trigger a failure in that case?

This code is actually in Kokkos, not Tpetra. We would have to get any changes approved by the Kokkos team. They may or may not have a standard for writing warnings to std::cerr; I don't know.

@MicheldeMessieres
Copy link
Contributor Author

@kddevin I was not paying attention to the fact this was in kokkos - so not the right plan.
The issue is that the Tpetra tests captures the std::cerr stream and will fail if it's not empty. That's not really working anyways (see #7966). So if we want to get these tests passing now maybe we just don't want to check std::cerr from kokkos. There are other warnings though that Tpetra could catch from kokkos.

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

Successfully merging this pull request may close these issues.

4 participants