Stratimikos: add adapter for Amesos2

[ibaned]

Okay, here is the big patch I mentioned in #1083.
This PR adds support for using Amesos2 through Stratimikos.
The motivation for this is to allow Albany to use direct solvers through the Tpetra stack.
A rundown of key points:

• The interface is instantiated for the Teuchos scalar types, but so far just uses the default ordinal and node types
• There is a test added, modeled after the Belos adapter test, which can exercise any of Amesos2's underlying solvers
• Invocations of this test were added to CMake for the TPLs I had enabled, namely KLU2 and SuperLU_DIST. Others can be added, but I didn't want to add tests without confidence they would pass once enabled.
• The KLU2 test passes.
• The SuperLU_DIST test is failing right now. I'm not sure if it is me coding the adapter wrong, or if Amesos2's SuperLU_DIST interface is wrong. Since no automated testing happens with SuperLU_DIST, this shouldn't impact many people, and I prefer to get the rest of this reviewed before looking into that.

@trilinos/stratimikos @trilinos/amesos2

[bartlettroscoe]

@ibaned,

I can review this in a bit. How urgent is it to get this into Trilinos 'develop'?

[srajama1]

Can you file a github issue for what is the issue for SuperLU-Dist ?

If Ross can review all the changes that is great. If you need me to review anything let me know.

[bartlettroscoe]

Someone needs to add automated testing for Trilinos with the SuperLUDist TPL turned on. This was identified as one of the TPLs that important SNL customers require (see #410). Can someone add a new Trilinos Framework GitHub issue for adding SuperLUDist testing for Trilinos? Really it should be added to the SEMS TPL mount.

[srajama1]

Ross: Totally agreed. What is this issue on Trilinos Framework or SEMS ? Can you please file it wherever appropriate ?

[ibaned]

@bartlettroscoe its not super urgent, a time frame of a few business days is fine.

@srajama1 I was going to wait for this PR to get accepted, but I guess I can open an issue now and reference this PR. If you could help me with that issue, it would be greatly appreciated. Likewise, maybe if you review how I'm calling the Amesos2::Solver, you may spot something I did wrong.

@bartlettroscoe I can also open the automated testing issue.

stand by...

[ibaned]

@bartlettroscoe I've opened #1091 for automated testing with SuperLUDist

[ibaned]

@krcb found that the SuperLU_Dist test failure was specific to the matrix, and that a different Amesos2 test matrix lets the test pass. This points more towards some issue in SuperLU_Dist itself, for that one matrix. Given that we now have good tests for both these adapters, I'd say this PR is ready for final approval by @trilinos/stratimikos folks.

[bartlettroscoe]

I'd say this PR is ready for final approval by @trilinos/stratimikos folks.

Okay, I will take a look and get comment here.

[ibaned]

@bartlettroscoe there is renewed interest in this feature (comment to follow regarding that). Its still not urgent, but this is a decent feature that is just waiting to be merged, so could you review / approve it in the coming days / weeks?

[kuberry]

We've recently been converting to using Stratimikos to manage our linear solver / preconditioner on the Compadre LDRD (Meshless toolkit), and it would be great if we could access Amesos2 functionality through Stratimikos.

[bartlettroscoe]

there is renewed interest in this feature (comment to follow regarding that). Its still not urgent, but this is a decent feature that is just waiting to be merged, so could you review / approve it in the coming days / weeks?

Sorry, this has been on my todo list for some time now. Given how long a detailed review is taking me I don't see the harm in doing a faster spot check and then doing an explicit merge commit and push with the checkin-test-sems.sh script. If something were to go wrong in nightly testing or something, we could just back out that one merge commit and an then fix the problem offline.

[ibaned]

doing a faster spot check and then doing an explicit merge commit and push with the checkin-test-sems.sh script

Thats fine with me. Just let me know when you're ready for me to do the latter two parts.

[bartlettroscoe]

Thats fine with me. Just let me know when you're ready for me to do the latter two parts.

Okay, I will do after I can get to #1303 (which I assume is higher priority?)

[ibaned]

#1303 (which I assume is higher priority?)

Yes, I definitely agree with that.

[aprokop]

This PR would also benefit ForTrilinos project, as it would allow us to avoid wrapping Amesos2. Any chance of merging this PR by the end of November?

[ibaned]

@aprokop all we're waiting on is a cursory review from @bartlettroscoe , but he's really busy. I'm inclined to just go ahead and merge this via the checkin script (non-rebased explicit merge commit, as discussed with him above).

[srajama1]

@bartlettroscoe : Any objections to merging this with checkin testing ?

[bartlettroscoe]

Let me take a quick look to see if anything sticks out. Otherwise, someone can merge locally and push with the checkin-test-sems.sh script.

[bartlettroscoe]

I checked out the branch ibaned/stratimikos-amesos2, merged in github/develop and then did the standard CI build using a do-configure script. The build failed with results shown at:

• https://testing.sandia.gov/cdash/index.php?project=Trilinos&filtercount=1&showfilters=1&field1=buildstamp&compare1=65&value1=20171024-1645

The do-configure script I used is displayed at:

• https://testing.sandia.gov/cdash/viewNotes.php?buildid=3181474##note2

So to reproduce, you can do:

$ cd <build-dir>/
$ ./do-configure -DTriilnos_ENABLE_Stratimikos=ON  # use the do-configure script above
$ make -j16

The build error shown is:

In file included from /ascldap/users/rabartl/Trilinos.base2/Trilinos/packages/amesos2/src/Amesos2_Superludist_FunctionMap.hpp:63:0,
                 from /ascldap/users/rabartl/Trilinos.base2/Trilinos/packages/amesos2/src/Amesos2_Superludist_decl.hpp:58,
                 from /ascldap/users/rabartl/Trilinos.base2/Trilinos/packages/amesos2/src/Amesos2_Superludist.hpp:47,
                 from /ascldap/users/rabartl/Trilinos.base2/Trilinos/packages/stratimikos/adapters/amesos2/src/Thyra_Amesos2LinearOpWithSolveFactory_def.hpp:63,
                 from /ascldap/users/rabartl/Trilinos.base2/Trilinos/packages/stratimikos/adapters/amesos2/src/Thyra_Amesos2LinearOpWithSolveFactory.cpp:48:
/ascldap/users/rabartl/Trilinos.base2/Trilinos/packages/amesos2/src/Amesos2_Superludist_TypeMap.hpp:76:26: fatal error: superlu_defs.h: No such file or directory
 #include "superlu_defs.h"
                          ^

SuperLUDist is not enabled as shown by:

• https://testing.sandia.gov/cdash/viewConfigure.php?buildid=3181474

and is not provided by the SEMS Env.

The checkin-test-sems.sh script should produce the exact same build error. I just used my own do-configure script so that I could set the parallel build options for 'make dashboard' to be fast.

[ibaned]

@bartlettroscoe sorry about the build error, and taking so long to fix it.
I just pushed commit f7c3d5a to the PR branch which I think helps. I was able to do a build of Trilinos with SuperLUDist disabled after that patch.
Could you please take another look when possible? Thanks!

[bartlettroscoe]

I added the commit a68dd29 to make the formatting more consistent with the Thyra Coding and Documentation Guidelines and tweaked a little documentation.

I am not going to bother removing the usage of raw pointers in the function bodies because many of the virtual functions have yet to be refactored to remove the usage of raw pointers. But there is going to be a significant refactoring of Thyra taking place in the near future so we can address this at that point.

I am going to go ahead and merge and push this from my local machine. Then we will see what happens with automated testing.

[bartlettroscoe]

When trying to push yesterday, I got some Teko test failures. I posted these to CDash as shown at:

• https://testing.sandia.gov/cdash/index.php?project=Trilinos&parentid=3206464

These tests seem to all show the failure:

ERROR: Could not find inverse type "Amesos2" required by inverse name "Amesos2"
 
 
 p=0: *** Caught standard std::exception of type 'std::logic_error' :
 
  /ascldap/users/rabartl/Trilinos.base/Trilinos/packages/teko/src/Teko_InverseLibrary.cpp:199:
  
  Throw number = 1
  
  Throw test that evaluated to true: !(false)
  
  Error!

I will take a look and see if I can figure out how to fix this.

[bartlettroscoe]

The fix in Teko was trivial (a one-line change as seen in 3f89a95) . I rebased the branch to clean up the history and forced pushed it to the remote branch in ibaned/Trilinos. I am now running ./checkin-test-sems.sh --no-rebase --do-all --push to test and push.

[bartlettroscoe]

Was merged and pushed using ./checkin-test-sems.sh --no-rebase --do-all --push. YOu can see the test and push log at:

• https://software.sandia.gov/pipermail/trilinos-checkin-tests/2017-November/002812.html

Now we just watch the nightly testing to see what happens and fix issues as they come up.

[eric-c-cyr]

@bartlettroscoe The only comment I have on the change is if Amesos2 is not available, Teko will think it is. Is there a way that can be protected with #ifdef conditionals? What does Stratimikos do to protect a lack of Amesos2?

[hkthorn]

@bartlettroscoe @ibaned

This change looks to be causing an issue with a regular build of mine to test Belos. The cmake error is:

... ...
Processing enabled package: Stratimikos (Libs, Tests, Examples)
CMake Error at cmake/tribits/core/utils/AssertDefined.cmake:79 (MESSAGE):
Error, the variable Amesos2_ENABLE_KLU2 is not defined!
Call Stack (most recent call first):
packages/stratimikos/src/CMakeLists.txt:16 (ASSERT_DEFINED)

-- Stratimikos_ForwardSolverAsPreconditioner: NOT added test because TPL_ENABLE_MPI='OFF' and COMM='mpi'!
Processing enabled package: NOX (Libs, Tests, Examples)

No ETI support requested by packages.

Set up for creating a distribution ...

Finished configuring Trilinos!

-- Configuring incomplete, errors occurred!
See also "/Users/hkthorn/Trilinos/Trilinos/CMAKE_SERIAL_BELOS/CMakeFiles/CMakeOutput.log".
See also "/Users/hkthorn/Trilinos/Trilinos/CMAKE_SERIAL_BELOS/CMakeFiles/CMakeError.log".

I see that there should be an adapter for Amesos2, but it should not be required to build Stratimikos.

[ibaned]

@hkthorn the adapter isn't required, but it looks like I assumed a variable would always be defined that probably isn't defined if Amesos2 is disabled... I created pull request #1973 in an attempt to fix this, please let me know what you think.

[bartlettroscoe]

@bartlettroscoe The only comment I have on the change is if Amesos2 is not available, Teko will think it is. Is there a way that can be protected with #ifdef conditionals? What does Stratimikos do to protect a lack of Amesos2?

Use the variable Stratimikos_ENABLE_Amesos2. Same for all of the optional dependencies.

IF (Stratimikos_ENABLE_Amesos2)
   # Do whatever ...
ENDIF()