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

Tpetra: MultiVector unit test failure with complex turned on #3823

Closed
lucbv opened this issue Nov 7, 2018 · 11 comments
Closed

Tpetra: MultiVector unit test failure with complex turned on #3823

lucbv opened this issue Nov 7, 2018 · 11 comments
Assignees
@mhoemmen
Labels
pkg: Tpetra type: bug The primary issue is a bug in Trilinos code or tests

Comments

@lucbv
Copy link
Contributor

lucbv commented Nov 7, 2018

@trilinos/tpetra @crtrott

Expectations

I am seeing test failures in MueLu's unit-test with complex turned on this morning.
My guess is that the failure is not completely (at all?) in MueLu as no commit was made to it since yesterday's nightly builds.

Current Behavior

The unit-test: TpetraCore_MultiVector_UnitTests_MPI_4 fails when complex is turned on with SHA1 23c8989 but it works fine without complex unabled and with complex unabled and SHA1 bf01cda. Actually I narrowed it down to working commit SHA1 0c5d4cf and not working commit SHA1 eb41c7f which points at the kokkos promotion.

Motivation and Context

I think this might be the reason for failures I am seeing in MueLu with complex enabled. I would like to resolve these...

Definition of Done

We need to have that test pass with complex unabled

Steps to Reproduce

Build with the indicated SHA1 and the following options:

            -D Trilinos_ENABLE_COMPLEX_DOUBLE=ON \
            -D Trilinos_ENABLE_Tpetra=ON \
            -D   Tpetra_ENABLE_TESTS=ON \
@lucbv lucbv added type: bug The primary issue is a bug in Trilinos code or tests pkg: Tpetra labels Nov 7, 2018
lucbv added a commit to lucbv/Trilinos that referenced this issue Nov 7, 2018
@lucbv
Tpetra: adding a new nightly build to avoid problems reported in issue …
c3c8208 
…trilinos#3823
@lucbv lucbv mentioned this issue Nov 7, 2018
Merged
lucbv added a commit that referenced this issue Nov 8, 2018
@lucbv
Merge pull request #3824 from lucbv/Tpetra_add_nightly_complex_build
67dd2ab 
Tpetra: adding a new nightly build to avoid problems reported in issue #3823
ndellingwood added a commit that referenced this issue Nov 9, 2018
@ndellingwood
tpetra: Fix types in MultiVector_MicroBenchmark with complex enabled
65e1c73 
This partially addresses issue #3823 reported by @lucbv.

@crtrott provided this fix which allows Tpetra to compile in Cuda builds
with complex enabled.

In a Cuda build with complex<RealType> enabled, the lambda body in a
Kokkos::parallel_for should capture Kokkos::complex<RealType> values,
not std::complex types.
@ndellingwood ndellingwood mentioned this issue Nov 9, 2018
Merged
@ndellingwood
Copy link
Contributor

Added PR #3838 which addresses the compilation error, thanks to @crtrott for taking a look at it with me. Tpetra compiles without error but there are failing tests in the MultiVector unit test that need to be addressed.

@cgcgcg cgcgcg mentioned this issue Nov 16, 2018
Merged
@lucbv lucbv mentioned this issue Nov 26, 2018
Closed
tjfulle pushed a commit to tjfulle/Trilinos that referenced this issue Dec 6, 2018
@lucbv @tjfulle
Tpetra: adding a new nightly build to avoid problems reported in issue …
b040b0c 
…trilinos#3823
tjfulle pushed a commit to tjfulle/Trilinos that referenced this issue Dec 6, 2018
@ndellingwood @tjfulle
tpetra: Fix types in MultiVector_MicroBenchmark with complex enabled
1b4911b 
This partially addresses issue trilinos#3823 reported by @lucbv.

@crtrott provided this fix which allows Tpetra to compile in Cuda builds
with complex enabled.

In a Cuda build with complex<RealType> enabled, the lambda body in a
Kokkos::parallel_for should capture Kokkos::complex<RealType> values,
not std::complex types.
@ndellingwood ndellingwood mentioned this issue Dec 13, 2018
Closed
@ndellingwood
Copy link
Contributor

Cross-reference #2426

@mhoemmen
Copy link
Contributor

Wow, we never actually fixed this. @trilinos/tpetra @kddevin

Does kokkos-kernels not test complex?
kokkos/kokkos-kernels#360

@mhoemmen
Copy link
Contributor

I know the likely cause and how to fix this:

kokkos/kokkos-kernels#360 (comment)

@ndellingwood
Copy link
Contributor

Does kokkos-kernels not test complex?
kokkos/kokkos-kernels#360

@mhoemmen complex is tested within kokkos-kernels' various builds. I've worked on setting up a complex-specific Jenkins build but have had issues working around Jenkins not parsing the scalar type (issues with the angle brackets < >) properly. In any case, complex types are tested.

Also, if a fix is submitted please test with deprecated code disabled as well as enabled, to rule out any issues that may creep in due to the dual_view changes ("semantic tightening" of memory spaces).

@mhoemmen
Copy link
Contributor

@ndellingwood Does kokkos-kernels actually test the value of the complex dot product?

@ndellingwood
Copy link
Contributor

@mhoemmen kokkos-kernels tests the value of the complex dot product. Two views are initialized with random values, and the dot result is compared with the serialized result computed on the host. There is an error tolerance used in the comparison since sum is not associative for floating point types in a parallel reduction.

@mhoemmen
Copy link
Contributor

@ndellingwood Should we close this issue then? kokkos/kokkos-kernels#360 I mean the promotion broke Trilinos, so something is wrong.

@ndellingwood
Copy link
Contributor

@mhoemmen I would keep this issue open to keep the issue visible until a fix is in Trilinos, but I'll defer to your judgement on the best practice for tracking/reporting when fixed.

@mhoemmen
Copy link
Contributor

"Should we close this issue" was a rhetorical question, whose answer is "no" ;-)

@mhoemmen mhoemmen self-assigned this Jan 23, 2019
@mhoemmen mhoemmen mentioned this issue Jan 23, 2019
Merged
@mhoemmen
Copy link
Contributor

PR: #4253

trilinos-autotester added a commit that referenced this issue Jan 24, 2019
@trilinos-autotester
Merge Pull Request #4253 from trilinos/Trilinos/Fix-4234
3ef91e9 
Automatically Merged using Trilinos Pull Request AutoTester
PR Title: Fix #4250, #4234, #4249, and #3823
PR Author: mhoemmen
@masterleinad masterleinad mentioned this issue Mar 4, 2019
Merged
9 tasks
@trilinos-autotester trilinos-autotester mentioned this issue Mar 6, 2021
Merged
@trilinos-autotester trilinos-autotester mentioned this issue Oct 12, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mhoemmen mhoemmen

Labels
pkg: Tpetra type: bug The primary issue is a bug in Trilinos code or tests
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mhoemmen @ndellingwood @lucbv