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

Tempus: Test failures with Tpetra's deprecated code disabled #5616

Closed
Tracked by #5602
tjfulle opened this issue Jul 31, 2019 · 23 comments
Closed
Tracked by #5602

Tempus: Test failures with Tpetra's deprecated code disabled #5616

tjfulle opened this issue Jul 31, 2019 · 23 comments
Assignees
Labels
pkg: Tempus TpetraRF type: bug The primary issue is a bug in Trilinos code or tests

Comments

@tjfulle
Copy link
Contributor

tjfulle commented Jul 31, 2019

Bug Report

@trilinos/tempus

Part of: #5602

Description

With Tpetra's deprecated code disabled, many Tempus tests fail for GlobalOrdinal=int:

  • Tempus_BDF2_Combined_FSA_MPI_1
  • Tempus_BDF2_MPI_1
  • Tempus_BackwardEuler_Combined_FSA_MPI_1
  • Tempus_BackwardEuler_MPI_1
  • Tempus_BackwardEuler_Staggered_FSA_MPI_1
  • Tempus_DIRK_ASA_MPI_1
  • Tempus_DIRK_Combined_FSA_SDIRK_5_Stage_4th_Order_MPI_1
  • Tempus_DIRK_Combined_FSA_SDIRK_5_Stage_5th_Order_MPI_1
  • Tempus_ExplicitRK_ASA_MPI_1
  • Tempus_HHTAlpha_MPI_1
  • Tempus_IMEX_RK_Combined_FSA_MPI_1
  • Tempus_IMEX_RK_Partitioned_Combined_FSA_Partitioned_IMEX_RK_1st_Order_MPI_1
  • Tempus_IMEX_RK_Partitioned_Staggered_FSA_Partitioned_IMEX_RK_1st_Order_MPI_1
  • Tempus_IMEX_RK_Staggered_FSA_MPI_1
  • Tempus_Newmark_MPI_1

See the full configuration for GlobalOrdinal=int at #5602 (comment)

Note: if you work off of the Trilinos develop branch, you will likely get build errors using the above linked configurations. The branch tjfulle:issue-5536 is currently working.

@tjfulle tjfulle added type: bug The primary issue is a bug in Trilinos code or tests pkg: Tempus TpetraRF labels Jul 31, 2019
@tjfulle
Copy link
Contributor Author

tjfulle commented Jul 31, 2019

@kddevin can you assign the Trilinos developer that was going to look at this?

@ccober6
Copy link
Contributor

ccober6 commented Aug 1, 2019

How did this get through testing and get committed?

@kddevin We need this fixed in the next day or two as it is holding up my development and develop on EMPIRE. If this is not possible, the offending commit needs to be reverted as soon as possible. Sorry but this is really a problem.

@tjfulle
Copy link
Contributor Author

tjfulle commented Aug 1, 2019

@ccober6 - sorry for the confusion - these tests don't fail with Trilinos' current default PR settings. They fail when Tpetra's deprecated code is disabled. As part of #5602, we are preparing packages downstream from Tpetra for the removal of the deprecated code.

@ccober6
Copy link
Contributor

ccober6 commented Aug 1, 2019

Hmm... I pulled develop yesterday, and seemed to get errors related to this?! This there some other problem?

@tjfulle
Copy link
Contributor Author

tjfulle commented Aug 1, 2019

I'm not sure about any other problems. The tests above fail when Tpetra_ENABLE_DEPRECATED_CODE:BOOL=OFF. In the coming weeks, @trilinos/tpetra plans to remove deprecated code - hence the push to clean up packages downstream of Tpetra.

@ccober6
Copy link
Contributor

ccober6 commented Aug 1, 2019

Ok sounds good. I will take a closer look at the failures with my develop build. Thanks for the clarification!

@ccober6
Copy link
Contributor

ccober6 commented Aug 1, 2019

So ... here is what I am seeing on my Mac OS 10.14 with Trilinos develop (90b0f99)

/Users/ccober/myOffice/Trilinos/packages/xpetra/src/BlockedMultiVector/Xpetra_BlockedMultiVector_def.hpp:56:1: error: 
      no template named 'BlockedMultiVector'
BlockedMultiVector<Scalar, LocalOrdinal, GlobalOrdinal, Node>::
^

and

/Users/ccober/myOffice/Trilinos/packages/xpetra/src/BlockedVector/Xpetra_BlockedVector_def.hpp:153:5: error: 
      'BlockedMultiVector' is not a class, namespace, or enumeration
    BlockedMultiVector::replaceLocalValue(myRow, 0, value);
    ^
/Users/ccober/myOffice/Trilinos/packages/xpetra/src/BlockedCrsMatrix/Xpetra_MapExtractor_decl.hpp:69:56: note: 
      'BlockedMultiVector' declared here
  template<class S, class LO, class GO, class N> class BlockedMultiVector;

Is just a problem on the Mac? This is a clean build and I have attached my configure and build logs.

build_tempus_s1037643_mac_gcc.sh.log

configure.log

build.log

@tjfulle
Copy link
Contributor Author

tjfulle commented Aug 1, 2019

I haven't seen that particular error. @william76 could this be related to ETI work in @trilnos/xpetra ?

@kddevin
Copy link
Contributor

kddevin commented Aug 1, 2019

This sounds like a problem in the Xpetra instantiation work that @william76 and @trilinos/xpetra are doing. It would be unrelated to the Tpetra deprecation.

@kddevin
Copy link
Contributor

kddevin commented Aug 1, 2019

I think @brian-kelley will be helping with the Tpetra deprecation work.

@william76
Copy link
Contributor

@ccober6 @kddevin
The cause of the error is a fun chain of included files with BlockedMultiVector and MultiVectorFactory.

I believe I have this fixed already in my branch for PR #5605 that I'm working on related to the bug reported in #5591 but I haven't let that PR go in because tracking down the errors with ETI off has been more stubborn than I'd like.

Also, building without ETI takes a long time :(

Hopefully I can get this PR in later today or tomorrow.

@mhoemmen
Copy link
Contributor

mhoemmen commented Aug 1, 2019

Also, building without ETI takes a long time :(

Now y'all know how the Sierra developers feel ;-)

@kddevin
Copy link
Contributor

kddevin commented Aug 1, 2019

Maybe we can move further ETI discussion elsewhere, as it isn't related to the deprecation issue here.

@kddevin
Copy link
Contributor

kddevin commented Aug 2, 2019

@tjfulle I am having difficulty reproducing the deprecation failures you are seeing in @trilinos/tempus
I am building with
-D Trilinos_ENABLE_Tpetra=ON
-D Tpetra_INST_INT_INT=ON
-D Tpetra_ENABLE_DEPRECATED_CODE=OFF
-D Xpetra_ENABLE_DEPRECATED_CODE=OFF

My tempus tests are running now; several that were reported as failing are now passing. Perhaps your rebase from develop pulled in fixes to the test failures.

So @trilinos/tempus team, please wait until we confirm or refute the failures before spending time on this issue.

@william76
Copy link
Contributor

I merged in #5605 which should fix the BlockedMultiVector is not a class, namespace, or enumeration issue @ccober6 mentioned.

The recent Xpetra classes that I pushed through were BlockedMultiVector, BlockedVector, MultiVectorFactory and MapExtractor. If you see more of these kinds of errors sneaking through that are hitting those classes please send in an issue and @mention me and the MueLu team.

@tjfulle
Copy link
Contributor Author

tjfulle commented Aug 2, 2019

@kddevin - maybe one of the other fixes had the happy consequence of fixing these too? I ran the GO=int tests a couple weeks ago and haven’t looked at them since all the recent work.

@ccober6
Copy link
Contributor

ccober6 commented Aug 2, 2019

Ok sounds good. FYI, as of a few minutes ago, I was still seeing the next set of errors on develop. Thoughts?

/Users/ccober/myOffice/Trilinos_Second/packages/ifpack2/src/Ifpack2_BandedContainer_def.hpp:279:39: error: 
      cannot initialize object parameter of type 'const
      Ifpack2::Container<Tpetra::RowMatrix<double, int, long long,
      Kokkos::Compat::KokkosDeviceWrapperNode<Kokkos::Serial, Kokkos::HostSpace>
      > >' with an expression of type
      'Ifpack2::BandedContainer<Tpetra::RowMatrix<double, int, long long,
      Kokkos::Compat::KokkosDeviceWrapperNode<Kokkos::Serial, Kokkos::HostSpace>
      >, double>'
      ArrayView<const LO> blockRows = this->getBlockRows(i);
                                      ^~~~

@william76
Copy link
Contributor

@tjfulle would these new errors be related to the fix you have in process in #5638 or some of the Tpetra deprecation stuff?

@kddevin
Copy link
Contributor

kddevin commented Aug 2, 2019

@brian-kelley Can you take a look at the compilation Curt is seeing above? It is unrelated to Tpetra deprecation, but this github issue has gone way off course anyway. :)

@brian-kelley
Copy link
Contributor

@ccober6 @kddevin I'm looking at it now, with a mac/clang build. It doesn't happen in linux/GCC, and there are many calls to getBlockRows() that look exactly the same as that one. But I'll fix it ASAP.

@brian-kelley
Copy link
Contributor

@ccober6 I can't seem to replicate the Ifpack2 build error. I'm on up-to-date develop (a4f99ce), on AppleClang 10.0.0 with OpenMPI 4.0.1. I took the configure script you linked and didn't get the error, then I added the lines to disable Xpetra/Tpetra deprecated code and still didn't get it. If I roll back to 90b0f99 then I get the BlockedMultiVector error you ran into first. Can you link the exact version and configure script?

@ccober6
Copy link
Contributor

ccober6 commented Aug 5, 2019

@brian-kelley Ok, I just rebuilt with a4f99ce on my Mac and it compiled and ran correctly. My previous build was 1bfa7b1 (between a4f99ce and 90b0f99). :( But I guess it does not matter at this point. Thanks for the help!

@william76
Copy link
Contributor

@tjfulle it looks like this might have been resolved, should this issue be closed out?

@ccober6 ccober6 closed this as completed Feb 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: Tempus TpetraRF type: bug The primary issue is a bug in Trilinos code or tests
Projects
None yet
Development

No branches or pull requests

6 participants