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

Tpetra: Removal of deprecated code meta issue #5602

Closed
8 of 14 tasks
tjfulle opened this issue Jul 30, 2019 · 17 comments
Closed
8 of 14 tasks

Tpetra: Removal of deprecated code meta issue #5602

tjfulle opened this issue Jul 30, 2019 · 17 comments
Labels
CLOSED_DUE_TO_INACTIVITY Issue or PR has been closed by the GitHub Actions bot due to inactivity. MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. pkg: Tpetra TpetraRF type: bug The primary issue is a bug in Trilinos code or tests

Comments

@tjfulle
Copy link
Contributor

tjfulle commented Jul 30, 2019

Bug Report

@trilinos/tpetra

Description

Meta issue to track work to remove deprecated code from @trilinos/tpetra

The work (so far) concentrates on two configurations, each with deprecated code disabled and all downstream packages enabled:

  1. Tpetra's global ordinal = int (TPETRA_INST_INT_INT:BOOL=ON)
  2. Tpetra's global ordinal = the default value (long long int)

Each configuration has build and/or test problems outlined in the following comments.

Related PRs

Related Issues

Supersedes

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

tjfulle commented Jul 30, 2019

Status for GlobalOrdinal=int, LocalOrdinal=int

Configuration and Build

Note: the branch tjfulle:issue-5536 is being updated with this issue, the configuration below is known to build with this branch. Build errors are likely on trilinos:develop

Note: the configuration below is being updated as issues are resolved.

With deprecated code disabled and using the default Tpetra GlobalOrdinal, the
build fails due to inconsistencies between Tpetra's global ordinal and Epetra.
For instance, Xpetra fails to build because it requires that Tpetra's global
ordinal be int when Epetra is enabled. The solution is to disable Epetra
support in this and other packages, or set the global ordinal to int, as in
the configuration below.

Note, the configuration options file below requires that
${TRILINOS_DIR}/cmake/std/sems/PullRequestGCC7.3.0TestingEnv.sh be sourced.

#!/bin/sh
rm -rf CMake*
  cmake
  -G "Ninja" \
  -D Trilinos_ENABLE_Tpetra:BOOL=ON \
  -D Tpetra_INST_INT_INT:BOOL=ON \
  -D Trilinos_CONFIGURE_OPTIONS_FILE:FILEPATH=${TRILINOS_DIR}/cmake/std/PullRequestLinuxCommonTestingSettings.cmake \
  -DTpetra_ENABLE_DEPRECATED_CODE:BOOL=OFF \
  -DXpetra_ENABLE_DEPRECATED_CODE:BOOL=OFF \
  -D Trilinos_ENABLE_STK:BOOL=OFF \
  -D ROL_ENABLE_EXAMPLES:BOOL=OFF \
  ${TRILINOS_DIR}

Failing tests

  • Amesos2_GappedMtxGIDs-1proc_MPI_1
  • Amesos2_KLU2_UnitTests_MPI_2
  • Belos_resolve_gmres_hb_1_MPI_4
  • Ifpack2_AdditiveSchwarz_MPI_4
  • MueLu_BlockedTransfer_Tpetra_MPI_4
  • MueLu_DriverEpetra_CircNspDependency_MPI_4
  • MueLu_DriverEpetra_IsorropiaPoisson_MPI_4
  • MueLu_ParameterListInterpreterTpetra_MPI_1
  • MueLu_UnitTestsEpetra_MPI_1
  • MueLu_UnitTestsIntrepid2Tpetra_MPI_1
  • MueLu_UnitTestsIntrepid2Tpetra_MPI_4
  • MueLu_UnitTestsTpetra_MPI_1
  • MueLu_UnitTestsTpetra_MPI_4
  • MueLu_VarDofDriver_MPI_1
  • MueLu_VarDofDriver_MPI_2
  • PanzerDiscFE_LinearObjFactory_Tests_MPI_2
  • Rythmos_IntegratorBuilder_ConvergenceTest_MPI_1
  • ShyLU_DDFROSch_test_thyra_xpetra_laplace_TLP_IPOUHarmonic_DIM2_DPN2_ORD0_TPETRA_MPI_4
  • ShyLU_DDFROSch_test_thyra_xpetra_laplace_TLP_IPOUHarmonic_DIM2_DPN2_ORD1_TPETRA_MPI_4
  • Stokhos_TpetraCrsMatrixUQPCEUnitTest_Serial_MPI_4
  • Teko_testdriver_tpetra_MPI_1
  • Teko_testdriver_tpetra_MPI_4
  • 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
  • Xpetra_Cloner_UnitTests_MPI_4
  • Zoltan2_GraphModel2ndAdjsFromAdjs_MPI_4
  • Zoltan2_Metric_MPI_4
  • Zoltan2_TaskMapper_MPI_4
  • Zoltan2_TaskMappingProblemTest_MPI_4
  • Zoltan2_componentMetrics_MPI_4
  • Zoltan2_pamgenMeshAdapterTest_ghosthg_MPI_4
  • Zoltan2_simplePamgenTest_MPI_3

@tjfulle
Copy link
Contributor Author

tjfulle commented Jul 30, 2019

Status with GlobalOrdinal=<default> and LocalOrdinal=<default>

Configuration and Build

Note: the branch tjfulle:issue-5536 is being updated with this issue, the configuration below is known to build with this branch. Build errors are likely on trilinos:develop

Note: the configuration below is being updated as issues are resolved.

Like the previous comment, with deprecated code disabled, all downstream packages enabled, and using the default Tpetra GlobalOrdinal, the build fails for a variety of reasons. Many packages need to be disabled to get a build to completion.

Note, the configuration options file below requires that
${TRILINOS_DIR}/cmake/std/sems/PullRequestGCC7.3.0TestingEnv.sh be sourced.

Final (Working) Configuration

/projects/sems/install/rhel6-x86_64/sems/utility/cmake/3.12.2/bin/cmake \
  -G "Ninja" \
  -D Trilinos_CONFIGURE_OPTIONS_FILE:FILEPATH=$TRILINOS_DIR/cmake/std/PullRequestLinuxCommonTestingSettings.cmake \
  -D Trilinos_ENABLE_Tpetra:BOOL=ON \
  -D Trilinos_ENABLE_Epetra:BOOL=OFF \
  -D Trilinos_ENABLE_Moertel:BOOL=OFF \
  -D Trilinos_ENABLE_ShyLU_DDFROSch:BOOL=OFF \
  -D Trilinos_ENABLE_Stokhos:BOOL=OFF \
  -D ROL_ENABLE_EXAMPLES:BOOL=OFF \
  -D Tpetra_ENABLE_DEPRECATED_CODE:BOOL=OFF \
  -D Xpetra_ENABLE_DEPRECATED_CODE:BOOL=OFF \
  -D Rythmos_ENABLE_TESTS:BOOL=OFF \
  -D Trilinos_ENABLE_FEI:BOOL=OFF \
  -D Trilinos_ENABLE_Panzer:BOOL=OFF \
 $TRILINOS_DIR

Failing Tests

  • Zoltan2_GraphModel2ndAdjsFromAdjs_MPI_4 (Failed)
  • Zoltan2_Metric_MPI_4 (Failed)
  • Zoltan2_componentMetrics_MPI_4 (Failed)
  • Zoltan2_TaskMapper_MPI_4 (Failed)
  • Zoltan2_TaskMappingProblemTest_MPI_4 (Failed)
  • Zoltan2_pamgenMeshAdapterTest_ghosthg_MPI_4 (Failed)
  • Zoltan2_simplePamgenTest_MPI_3 (Failed)
  • Amesos2_GappedMtxGIDs-1proc_MPI_1 (Failed)
  • Ifpack2_AdditiveSchwarz_MPI_4 (Failed)
  • MueLu_UnitTestsTpetra_MPI_1 (Failed)
  • MueLu_UnitTestsTpetra_MPI_4 (Failed)
  • MueLu_UnitTestsIntrepid2Tpetra_MPI_1 (Failed)
  • MueLu_UnitTestsIntrepid2Tpetra_MPI_4 (Failed)
  • MueLu_BlockedTransfer_Tpetra_MPI_4 (Failed)
  • MueLu_VarDofDriver_MPI_1 (Failed)
  • MueLu_VarDofDriver_MPI_2 (Failed)

@brian-kelley
Copy link
Contributor

brian-kelley commented Aug 2, 2019

@tjfulle I fixed the Ifpack2_AdditiveSchwarz_MPI_4 tests, and made a pull request into your branch "issue-5536". Is that the right way to merge it, or should I PR straight to origin/develop?

tjfulle#2

Edit: nvm, closed that one, I'll wait until #5590 gets merged and then make a PR in the main repo.
Edit2: This is in #5640, which will be merged when autotesting finishes.

@mhoemmen
Copy link
Contributor

mhoemmen commented Aug 2, 2019

PR #5573 merged, so Stokhos builds now with Tpetra deprecated code OFF and GO=int OFF.

@GeoffDanielson
Copy link
Contributor

GeoffDanielson commented Aug 2, 2019

@tjfulle , I am unable to make Belos_resolve_gmres_hb_1_MPI_4 fail with GO = LO = int

@GeoffDanielson
Copy link
Contributor

GeoffDanielson commented Aug 2, 2019

@tjfulle I am also unable to make Rythmos_IntegratorBuilder_ConvergenceTest_MPI_1 fail with GO = LO = int.

@kddevin
Copy link
Contributor

kddevin commented Aug 2, 2019

@tjfulle I am running tempus now, but not seeing failures in some of the tests that were reported as failures. Perhaps the rebase from develop pulled in solutions.

I will report all tempus results once them run to completion; the tempus tests take a long time on my workstation.

Zoltan2 fixes are in PR #5645 and will soon be merged with develop (I hope).

tjfulle added a commit that referenced this issue Aug 19, 2019
tjfulle added a commit that referenced this issue Aug 20, 2019
tjfulle added a commit that referenced this issue Aug 20, 2019
* Zoltan2: use ZOLTAN2_HAVE_EPETRA.

Closes: #5751
Part of: #5602

* only disable epetra in zoltan2 if user explicitly requested

* some versions of cmake did not like the last conditional
@kddevin
Copy link
Contributor

kddevin commented Aug 26, 2019

Hey, @tjfulle : what's the current status? If needed, we can ask @DrBooom to help with remaining failures.

@tjfulle
Copy link
Contributor Author

tjfulle commented Aug 26, 2019

I’ll gather more details, off the top of my head, some ROL examples and MueLu tests fail with deprecated code off and default ordinals in Tpetra. A large remaining issue is that this is with Epetra disabled. I’m not sure many apps are ready to fully abandon Epetra (@trilinos/panzer for example). With Tpetra_INST_INT_INT=ON there are other failures I haven’t looked at since the initial diagnostic (and might now be better with the many PRs for this issue).

If @DrBooom can look in to test failures with deprecated code off and Tpetra_INST_INT_INT=ON, I’ll start looking in to being able to enable Epetra when Tpetra ordinals are their default values

@rppawlo
Copy link
Contributor

rppawlo commented Aug 26, 2019

Right now, panzer requires both epetra and tpetra but can use any single GO that is enabled in tpetra, so it should not hold things up. If the GO is not appropriate for Epetra, the workaround for ATDM builds is to disable epetra support in the xpetra and muelu packages.Then both stacks build with any tpetra GO type. I'll be making the panzer requirement on epetra optional early next FY. Ross has an ATDM build running nightly that enables a single GO (long long) and builds the entire stack including panzer, epetra and tpetra. If there is anything on the panzer side you encounter, please let me know, but I think we are clear on the deprecated tpetra code.

@tjfulle
Copy link
Contributor Author

tjfulle commented Aug 26, 2019

Thanks @rppawlo, I don’t think Panzer is a hold up. I’ll have to look at the ATDM config. My definition of done is to enable Tpetra+ALL downstream packages with deprecated code off and otherwise the standard PR config. Perhaps this config enables more than ATDM and that’s the source of the failures? We’re getting to the bottom of it...

@brian-kelley
Copy link
Contributor

brian-kelley commented Sep 26, 2019

There are now 4 nightly builds with deprecated off:

@tjfulle If you're not already working on these, I can look at the ROL build errors in GO long long today and tomorrow.

kddevin added a commit that referenced this issue Oct 9, 2019
…c profile.

Dynamic profile construction is deprecated.
Added loops to count number of entries per row to be inserted into graph.
See github #5602
kddevin added a commit that referenced this issue Oct 9, 2019
the number of nonzeros per row in this case is constant for each row.
rppawlo added a commit that referenced this issue Oct 15, 2019
panzer:  change dynamic profile Tpetra::CrsGraph construction to static profile for #5602
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Oct 16, 2019
…s:develop' (8fe9e32).

* trilinos-develop:
  TrilinosCouplings: fixed wrong type in BlockRelaxation paramlist
  MueLu: remerge separate libraries
  Fixed shadow warning
  Use XHOSTTYPE Windows to skip tests on windows
  EXODUS: Fix for loop
  Automatic snapshot commit from seacas at cd7b73e53c
  Tempus: Add Output Control to TimeStepControl.
  panzer:  trilinos#5602 changed dynamic profile CrsGraph construction to static profile
  panzer:  an easy fix from dynamic profile to static profile trilinos#5602 the number of nonzeros per row in this case is constant for each row.
  panzer:  change dynamic profile Tpetra::CrsGraph construction to static profile. Dynamic profile construction is deprecated. Added loops to count number of entries per row to be inserted into graph. See github trilinos#5602
  IOSS: A couple pamgen fixes (test and compilation warning)
  Automatic snapshot commit from seacas at 83f74bc141
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Oct 16, 2019
…s:develop' (8fe9e32).

* trilinos-develop:
  TrilinosCouplings: fixed wrong type in BlockRelaxation paramlist
  MueLu: remerge separate libraries
  Fixed shadow warning
  Use XHOSTTYPE Windows to skip tests on windows
  EXODUS: Fix for loop
  Automatic snapshot commit from seacas at cd7b73e53c
  Tempus: Add Output Control to TimeStepControl.
  panzer:  trilinos#5602 changed dynamic profile CrsGraph construction to static profile
  panzer:  an easy fix from dynamic profile to static profile trilinos#5602 the number of nonzeros per row in this case is constant for each row.
  panzer:  change dynamic profile Tpetra::CrsGraph construction to static profile. Dynamic profile construction is deprecated. Added loops to count number of entries per row to be inserted into graph. See github trilinos#5602
  IOSS: A couple pamgen fixes (test and compilation warning)
  Automatic snapshot commit from seacas at 83f74bc141
@github-actions
Copy link

This issue has had no activity for 365 days and is marked for closure. It will be closed after an additional 30 days of inactivity.
If you would like to keep this issue open please add a comment and/or remove the MARKED_FOR_CLOSURE label.
If this issue should be kept open even with no activity beyond the time limits you can add the label DO_NOT_AUTOCLOSE.
If it is ok for this issue to be closed, feel free to go ahead and close it. Please do not add any comments or change any labels or otherwise touch this issue unless your intention is to reset the inactivity counter for an additional year.

@github-actions github-actions bot added the MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. label Sep 25, 2021
@github-actions
Copy link

github-actions bot commented Dec 1, 2021

This issue was closed due to inactivity for 395 days.

@github-actions github-actions bot added the CLOSED_DUE_TO_INACTIVITY Issue or PR has been closed by the GitHub Actions bot due to inactivity. label Dec 1, 2021
@github-actions github-actions bot closed this as completed Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLOSED_DUE_TO_INACTIVITY Issue or PR has been closed by the GitHub Actions bot due to inactivity. MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. pkg: Tpetra 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