MueLu: Failing tests with Tpetra deprecated code disabled

[tjfulle]

Bug Report

@trilinos/muelu

Part of: #5602

Description

With Tpetra's deprecated code disabled, many MueLu tests fail for GlobalOrdinal=[int,long long int]

If GlobalOrdinal=int

See the full configuration at #5602 (comment)

• MueLu_BlockedTransfer_Tpetra_MPI_4: see issue Xpetra/Tpetra: MatrixMatrixMultiply fails with Tpetra deprecated code OFF #5629
• MueLu_DriverEpetra_CircNspDependency_MPI_4: fix in PR MueLu: fixing isorropia test before removal of Tpetra deprecated code issue #5614 #5631
• MueLu_DriverEpetra_IsorropiaPoisson_MPI_4: fix in PR MueLu: fixing isorropia test before removal of Tpetra deprecated code issue #5614 #5631
• MueLu_ParameterListInterpreterTpetra_MPI_1
• MueLu_UnitTestsEpetra_MPI_1: see issue Xpetra/Tpetra: MatrixMatrixMultiply fails with Tpetra deprecated code OFF #5629
• MueLu_UnitTestsIntrepid2Tpetra_MPI_1: assigned to @csiefer2 in MueLu: Pcoarsen factory uses dynamic CrsGraph allocation #5625
• MueLu_UnitTestsIntrepid2Tpetra_MPI_4: assigned to @csiefer2 in MueLu: Pcoarsen factory uses dynamic CrsGraph allocation #5625
• MueLu_UnitTestsTpetra_MPI_1: partial fix in MueLu: fixing the Zoltan unit-test for static allocation of Tpetra::CrsGraph see issue #5614 #5620 and issue Xpetra/Tpetra: MatrixMatrixMultiply fails with Tpetra deprecated code OFF #5629
• MueLu_UnitTestsTpetra_MPI_4: partial fix in MueLu: fixing the Zoltan unit-test for static allocation of Tpetra::CrsGraph see issue #5614 #5620 and issue Xpetra/Tpetra: MatrixMatrixMultiply fails with Tpetra deprecated code OFF #5629
• MueLu_VarDofDriver_MPI_1: fix in MueLu: fixing the number of non-zeros per row in unsmoosh factory #5628
• MueLu_VarDofDriver_MPI_2: fix in MueLu: fixing the number of non-zeros per row in unsmoosh factory #5628

If GlobalOrdinal=long long int

See the full configuration at: #5602 (comment)

• MueLu_UnitTestsTpetra_MPI_1 (Failed): partial fix in MueLu: fixing the Zoltan unit-test for static allocation of Tpetra::CrsGraph see issue #5614 #5620 and issue Xpetra/Tpetra: MatrixMatrixMultiply fails with Tpetra deprecated code OFF #5629
• MueLu_UnitTestsTpetra_MPI_4 (Failed): partial fix in MueLu: fixing the Zoltan unit-test for static allocation of Tpetra::CrsGraph see issue #5614 #5620 and issue Xpetra/Tpetra: MatrixMatrixMultiply fails with Tpetra deprecated code OFF #5629
• MueLu_UnitTestsIntrepid2Tpetra_MPI_1 (Failed): assigned to @csiefer2 in MueLu: Pcoarsen factory uses dynamic CrsGraph allocation #5625
• MueLu_UnitTestsIntrepid2Tpetra_MPI_4 (Failed): assigned to @csiefer2 in MueLu: Pcoarsen factory uses dynamic CrsGraph allocation #5625
• MueLu_BlockedTransfer_Tpetra_MPI_4 (Failed): see issue Xpetra/Tpetra: MatrixMatrixMultiply fails with Tpetra deprecated code OFF #5629
• MueLu_VarDofDriver_MPI_1 (Failed): fix in MueLu: fixing the number of non-zeros per row in unsmoosh factory #5628
• MueLu_VarDofDriver_MPI_2 (Failed): fix in MueLu: fixing the number of non-zeros per row in unsmoosh factory #5628

[lucbv]

@tjfulle I will give it a look, after re-configuring according to the instructions in #5602 I notice that a lot of packages are enabled. I will at least disable SEACAS, STK and PANZER to save on build time, do you see any reason for it to change the results of the build/tests?

[tjfulle]

@lucbv no reason to enable everything. The configuration is based on PR testing configuration, which is overkill for fixing these tests, I am sure. I would enable only what you need.

[tjfulle]

But do note, if you work off of trilinos:develop, it may not build. the branch referenced in #5602 has a bunch of fixes that have not made it back to develop (yet)

[lucbv]

I see, I have a build that is half-way through so far so I will let it continue and if it fails I'll switch to your branch to fix the tests.
I am working on this "in the background" so I might not be lightning fast... let me know if there is great urgency? Based on all the failing tests you report I am guessing I have a bit of time though ; )

[tjfulle]

No urgency. I suspect that one or two fixes will fix all the tests.

Since you are only building through MueLu you may not need to use my branch - the fixes are mostly for packages downstream of MueLu

[lucbv]

No I got unlucky and some belos Epetra adapter died on me so I added your fork as a remote and I am building off of your branch.
How do you want me to submit my fixes though: should I do a PR to your fork or patch Trilinos/develop?
My guess is the later since it might be cleaner?

[tjfulle]

Note that the configuration I built with disables Epetra (leading to the previous issue with Galeri). It may not build with Epetra enabled.

Feel free to push to my branch (you should have write permissions as a Trilinos developer), or open a PR against trilinos:develop. I plan to split the changes I have already made in my branch out by package and open separate PRs for each, so it would probably be easiest to work with trilinos:develop.

[lucbv]

@tjfulle so I looked at this test: MueLu_UnitTestsTpetra_MPI_1 and I see that two of the unit-tests are failing because of a call to Tpetra::MatrixMatrix::Multiply which does not seem compute the number of non-zeros of the output matrix correctly and dies because of the following error:

 p=0: *** Caught standard std::exception of type 'std::runtime_error' :
 
  /home/lberge/Research/Trilinos_lucbv/packages/tpetra/core/src/Tpetra_CrsGraph_def.hpp:2299:
  
  Throw number = 1
  
  Throw test that evaluated to true: (numInserted == Teuchos::OrdinalTraits<size_t>::invalid())
  
  Tpetra::CrsGraph<int, long long, Kokkos::Compat::KokkosDeviceWrapperNode<Kokkos::Serial, Kokkos::HostSpace> >::insertGlobalIndicesImpl: There is not enough capacity to insert indices in to row 0. The upper bound on the number of entries in this row must be increased to accommodate one or more of the new indices.

For info here is the full gdb backtrace I have for this throw: matmatmult_backtrace.txt

I am looking at another unit-test (Zoltan Build) that fails due to bad memory allocation in a matrix.
This one is on us (@trilinos/muelu) so I'll look into fixing it.

[tjfulle]

The runtime error is from DynamicProfile being deprecated. DynamicProfile is deprecated so any graph/matrix that MueLu constructs will have to be given an upper size limit for entries in a row. Does that make sense?

[lucbv]

Sure but how do I guess that number for a matrix that is the result of the multiplication of two matrices?
I would have assumed that in that case Tpetra does the calculation for me, otherwise I might as well re-implement the symbolic phase of MatMatMult?

[tjfulle]

@csiefer2 @jhux2 isn’t there an overload of MMM for this use case?

[cgcgcg]

@tjfulle Is it possible that C_estimate_nnz in TpetraExt_MatrixMatrix_def.hpp is not doing the correct thing when one of the matrices only has one entry per row?

[lucbv]

@tjfulle I did a bit of investigation already into the wrapper we are using and I can't imagine that it does the wrong thing are almost every single test in MueLu would fail.
This error is more likely due to a bad estimation of the number of non-zero per row from the Tpetra implementation of the MMM.
@mhoemmen do you recall what the expectations are on matrix C computed with MMM as C=A*B?

[tjfulle]

In preparation for removing DynamicProfile I changed Tpetra’s import/export to have the ability to change a graph’s structure - I thought this was being used by MMM so that the target of MMM could be altered to accommodate unknown entries, but I’m unsure. I’m unfamiliar with MMM, @csiefer2 should know

[lucbv]

@tjfulle the above PR #5628 should fix the issue with the variable dofs per node test.
It was relatively straightforward so I don't expect problems with the auto-tester.
Moving on to the BlockedTransfer test.

[tjfulle]

Thanks @lucbv!

[lucbv]

So the BlockedTransfer test fails because it uses the Petrov-Galerkin prolongator factory which uses the MatMatMult operation from Xpetra/Tpetra.
We might want to create a seperate issue for that to track it properly between MueLu/Xpetra and Tpetra.
This pretty much concludes the work for GlobalOrdinal=long long int in the sense that everything is fully triaged, root cause for failures are clearly identified and work has been assigned appropriately.

[mhoemmen]

@mhoemmen do you recall what the expectations are on matrix C computed with MMM as C=A*B?

Not off-hand. @csiefer2 is the expert here.

[lucbv]

@tjfulle almost everything is now covered one way or another.
The main thing will be to take care of the MMM kernel but this will wait until next week when @csiefer2 is back from travel.
Regarding the MueLu_ParameterListInterpreterTpetra_MPI_1 I'm too tired to look at it now so it'll probably wait until next week.
Once PR #5631 is merged, would you mind re-running the tests with all the changes to check that things are good on you end?

[tjfulle]

Thanks @lucbv! I’m on travel till early next week, I’ll look then

[csiefer2]

Huh. That's weird. Will look at this MMM & Intrepid issues next week.

[brian-kelley]

All the checkboxes are filled! There's only one new MueLu test failure:
MueLu_DriverEpetra_CircNspDependency_MPI_4. And this probably doesn't have anything to do with our deprecation stuff:

Laplace2D: Build (MueLu::CoarseMapFactory)
Striding info = {}   [default]
Strided block id = -1   [default]
Domain GID offsets = {0}   [default]

Laplace2D: Build (MueLu::RebalanceTransferFactory)
MueLu_Driver: preconditioner setup crashed w/ message:/home/nightlyTesting/OPENMPI-1.10.1_RELEASE_TPETRA_DEPRECATED_CODE_OFF_ENABLE_DOWNSTREAM_GO_INT/Trilinos/packages/muelu/src/Interface/../MueCentral/MueLu_Level.hpp:197:

Throw number = 3

Throw test that evaluated to true: !IsKey(fac, ename)

"P" not found
MueLu_Driver: Not solving system due to crash in preconditioner setup

[brian-kelley]

Closing since the deprecated removal PR is ready to go in next week.