#74: Belos: Add PCPG tpetra test

[cwschilly]

Fixes #74

[cwschilly]

I have not found a suitable alternative to Belos::EpetraPrecOp. This was used in the Epetra test to create a Belos preconditioned operator from the preconditioner, which is necessary because Belos expects an operator to apply the preconditioner with Apply() NOT ApplyInverse().

As a result, I have (temporarily) removed the preconditioner from the test.

Note: This thread may help resolve the problem

_Follow-Up: Consider Belos::MueLuOp<> as a replacement for Belos::EpetraPrecOp_

[cwschilly]

I templated the run function so that we can test it on different scalar types, but it fails with float. Per our last meeting, I won't pursue the failure--it's commented out for now

[cwschilly]

@stmcgovern This PR is ready for review; we'll have to decide if we want to pursue incorporating a MueLu preconditioner (it may require asking a MueLu or Belos developer for help)

[cwschilly]

This test passes on MPI builds now; I will likely have to make some small changes to get it to pass GPU builds as well

[cwschilly]

The test passes on both MPI and GPU builds

[cwschilly]

The build fails because of the circular dependency between MueLu and Belos that I've introduced by using a MueLu preconditioner (as opposed to the ML preconditioner used by the Epetra test). This build failure can be bypassed by adding this flag to the configuration: Trilinos_ASSERT_DEFINED_DEPENDENCIES=IGNORE

[cwschilly]

@trilinos/belos @jennloe

Would you please take a look at this PR? In using a MueLu preconditioner for the Tpetra version of the PCPG test (instead of the ML preconditioner in the Epetra test), I've introduced a circular dependency between MueLu and Belos. What is the best way to handle this?

[jennloe]

@cwschilly We've run into this issue before trying to make Belos tests with ifpack2 preconditioners.

For context: Belos/epetra tests were able to use ML as a dependency because ML used AztecOO for testing, not Belos.

I think the best way to resolve this is simply to remove the MueLu preconditioner from the Belos test. If there is a pressing reason to test PCPG with MueLu (I don't know of any off the top of my head; MueLu has pretty extensive testing already), then we can put an additional test in the MueLu directory.

@cgcgcg @hkthorn Any additional comments on this? Do you agree with my suggested path forward?

[cgcgcg]

Would it be possible to do something similar to how Ifpack2 was handled but for MueLu? As far as I know ML is registered in the default solver factory. It might make sense to do the same for MueLu. This could involve moving the MueLu adapter from packages/muelu/adapters to Stratimikos, but that should be pretty easy to do.

@cwschilly Can you give more details about the way this bugs out?

[cwschilly]

Thanks for your responses @jennloe @cgcgcg . Here's the error when I try to build with the MueLu as an optional test dependency:

CMake Error at cmake/tribits/core/package_arch/TribitsReadDepsFilesCreateDepsGraph.cmake:733 (message):
  Error, the package 'MueLu' is listed as a dependency of the package 'Belos'
  but the package 'MueLu' is either not defined or is listed later in the
  package order.  This may also be an attempt to create a circular dependency
  between the packages 'MueLu' and 'Belos' (which is not allowed).  Check the
  spelling of 'MueLu' or see how it is listed in a call to
  tribits_repository_define_packages() in relation to 'Belos'.  To
  ignore/disable the undefined package 'MueLu', set the cache variable
  Trilinos_ASSERT_DEFINED_DEPENDENCIES=IGNORE.

Setting the prescribed flag works, but that doesn't seem like a viable solution.

[cgcgcg]

I see. I guess the Belos team needs to evaluate what the best path forward is. Does it make sense to run this with a simpler preconditioner? Or should this test be moved somewhere else?

[cwschilly]

@stmcgovern This PR is ready for review; I commented out all preconditioning for the time being

[cwschilly]

Thank you for your review, @hkthorn ! I returned to using 1e-10 for the tolerance to keep things as simple as possible. Another option to allow for future implementation of other scalar types is to include a third parameter to the run function (ie run<double>(argc,argv,tol))