Trilinos icon indicating copy to clipboard operation
Trilinos copied to clipboard

Address issues with the TriBITS update from PR #11380

Open bartlettroscoe opened this issue 1 year ago • 12 comments

Description

There ware some possible errors caused by the recent snapshot of TriBITS into Trilinos from PR #11380. One definite case was a change in the behavior for disabling a TPL and having that disable downstream TPLs (which broke Albany Trilinos configures where HDF5 was explicitly disabled by they wanted Netcdf enabled, see #11426).

There may be other problems as well.

The revert PR #11428 was created and merged which reverted the merge of PR #11380.

Tasks

  • [x] Update TriBITS to make TPL dependencies optional to address #11426 (see TriBITSPub/TriBITS#557 and TriBITSPub/TriBITS#558)
  • [x] Bring back support for Trilinos_ASSERT_MISSING_PACKAGES and properly deprecate it with a deprecation warning to address below and https://github.com/trilinos/Trilinos/issues/11426#issuecomment-1364150183 (see TriBITSPub/TriBITS#558)
  • [x] Create new Trilinos branch 'tribits-snapshot-2022-12-12-again' that starts with a revert of the revert commit from PR #11428
  • [x] Update TriBITS snapshot and merge into branch 'tribits-snapshot-2022-12-12-again'
  • [x] Post new PR for 'tribits-snapshot-2022-12-12-again' ... See #11458
  • [x] Ask Albany and Nalu developers to test branch 'tribits-snapshot-2022-12-12-again' if they want
  • [x] Merge PR #11458
  • [ ] Try to reproduce and address #11425
  • [ ] Verify that no new errors are reported by customers

bartlettroscoe avatar Dec 24 '22 13:12 bartlettroscoe

EMPIRE was also seeing a configure failure similar to what @spdomin noted in #11426 with the tribits push from last week:

02:06:42  Setting up major user options ...
02:06:42  
02:06:42  CMake Error at cmake/tribits/core/package_arch/TribitsGlobalMacros.cmake:744 (message):
02:06:42    Error, Trilinos_ASSERT_MISSING_PACKAGES= 'ON' is set and is no longer
02:06:42    supported! Please set Trilinos_ASSERT_DEFINED_DEPENDENCIES=FATAL_ERROR
02:06:42    instead!
02:06:42  Call Stack (most recent call first):
02:06:42    cmake/tribits/core/package_arch/TribitsProjectImpl.cmake:130 (tribits_define_global_options_and_define_extra_repos)
02:06:42    cmake/tribits/core/package_arch/TribitsProject.cmake:92 (tribits_project_impl)
02:06:42    CMakeLists.txt:144 (TRIBITS_PROJECT)
02:06:42  
02:06:42  
02:06:42  -- Configuring incomplete, errors occurred!

rppawlo avatar Jan 03 '23 14:01 rppawlo

@rppawlo, @spdomin,

Responding to above and https://github.com/trilinos/Trilinos/issues/11426#issuecomment-1364150183 for fatal error on setting Trilinos_ASSERT_MISSING_PACKAGES ...

This change was highlighted in the changelog here

I could bring back support for Trilinos_ASSERT_MISSING_PACKAGES and properly deprecate it with a deprecation warning. That would smooth upgrades. I will look into that. (But then you have to check for cases where the old option and new option are both set and perhaps to inconsistent values.)

bartlettroscoe avatar Jan 03 '23 18:01 bartlettroscoe

@bartlettroscoe - you don't have to properly deprecate for empire. empire is not setting that flag anywhere. It is being set by the atdm build scripts and needs to be fixed in trilinos:

rppawlo@s1057859 cmake % git grep ASSERT_MISSING
std/atdm/ATDMDevEnvSettings.cmake:ATDM_SET_CACHE(Trilinos_ASSERT_MISSING_PACKAGES ON CACHE BOOL)

rppawlo avatar Jan 03 '23 18:01 rppawlo

It is being set by the atdm build scripts and needs to be fixed in trilinos

@rppawlo, that is good to know. (Not sure how that setting got in there.)

bartlettroscoe avatar Jan 03 '23 18:01 bartlettroscoe

@spdomin, what configuration of Trilinos is producing the Trilinos_ASSERT_MISSING_PACKAGES error you reported in https://github.com/trilinos/Trilinos/issues/11426#issuecomment-1364150183? Is this the Spack configuration?

bartlettroscoe avatar Jan 04 '23 01:01 bartlettroscoe

@spdomin, what configuration of Trilinos is producing the Trilinos_ASSERT_MISSING_PACKAGES error you reported in #11426 (comment)? Is this the Spack configuration?

@bartlettroscoe, no, this was my standard Trilinos config using the config file noted in this push: https://github.com/NaluCFD/Nalu/commit/c4f24d4937a2fab398b1849bb9b749cbfe2ef558

See line 83 build/do-configTrilinos_release

spdomin avatar Jan 06 '23 16:01 spdomin

@spdomin, are you okay with the solution to just remove -D Trilinos_ASSERT_MISSING_PACKAGES=OFF from you configure script in https://github.com/NaluCFD/Nalu/commit/c4f24d4937a2fab398b1849bb9b749cbfe2ef558?

bartlettroscoe avatar Jan 06 '23 16:01 bartlettroscoe

@rppawlo, @spdomin, @ikalash, @GrahamBenHarper, @lxmota,

Do any of you want to try to test against the update of TriBITS and Trilinos in the new PR #11458 with Nalu or Albany before this PR gets merged? The issues discovered in the previous TriBITS snapshot merge from PR #11380 should be addressed as listed in above. I will run some ATDM Trilinos builds before I try to merge but I was not going to test against APP codes. (But if there are easy up-to-date instructions on running a smoke test against a branch of Trilinos and the APP's suite against Trilinos 'develop' is clean, then I am willing to give it a try.)

bartlettroscoe avatar Jan 11 '23 17:01 bartlettroscoe

@rppawlo, @spdomin, @ikalash, @GrahamBenHarper, @lxmota,

FYI: PR #11458 just merged. Please comment here if you see any problems with your integration testing of Trilinos 'develop' with you codes.

bartlettroscoe avatar Jan 19 '23 21:01 bartlettroscoe

@bartlettroscoe it looks like the TriBITS configure still doesn't pick up the MueLu/Xpetra dependency on Thyra properly for #11425. I'm letting it run a clean build again, but I suspect since the list of enabled packages has not changed and the build logic has not changed, the result is the same. Interestingly, it looks like it sets the Xpetra dependencies twice in a row.

Edit: the build indeed failed in the same place

-- Setting ThyraCore_ENABLE_TeuchosCore=ON since Trilinos_ENABLE_ThyraCore=ON AND Trilinos_ENABLE_TeuchosCore=ON
-- Setting ThyraCore_ENABLE_TeuchosParameterList=ON since Trilinos_ENABLE_ThyraCore=ON AND Trilinos_ENABLE_TeuchosParameterList=ON
-- Setting ThyraCore_ENABLE_TeuchosComm=ON since Trilinos_ENABLE_ThyraCore=ON AND Trilinos_ENABLE_TeuchosComm=ON
-- Setting ThyraCore_ENABLE_TeuchosNumerics=ON since Trilinos_ENABLE_ThyraCore=ON AND Trilinos_ENABLE_TeuchosNumerics=ON
-- Setting ThyraCore_ENABLE_RTOp=ON since Trilinos_ENABLE_ThyraCore=ON AND Trilinos_ENABLE_RTOp=ON
-- Setting ThyraTpetraAdapters_ENABLE_ThyraCore=ON since Trilinos_ENABLE_ThyraTpetraAdapters=ON AND Trilinos_ENABLE_ThyraCore=ON
-- Setting ThyraTpetraAdapters_ENABLE_Tpetra=ON since Trilinos_ENABLE_ThyraTpetraAdapters=ON AND Trilinos_ENABLE_Tpetra=ON
-- Setting Xpetra_ENABLE_Teuchos=ON since Trilinos_ENABLE_Xpetra=ON AND Trilinos_ENABLE_Teuchos=ON
-- Setting Xpetra_ENABLE_KokkosCore=ON since Trilinos_ENABLE_Xpetra=ON AND Trilinos_ENABLE_KokkosCore=ON
-- Setting Xpetra_ENABLE_KokkosContainers=ON since Trilinos_ENABLE_Xpetra=ON AND Trilinos_ENABLE_KokkosContainers=ON
-- Setting Xpetra_ENABLE_Tpetra=ON since Trilinos_ENABLE_Xpetra=ON AND Trilinos_ENABLE_Tpetra=ON
-- Setting Xpetra_ENABLE_KokkosKernels=ON since Trilinos_ENABLE_Xpetra=ON AND Trilinos_ENABLE_KokkosKernels=ON
-- NOT setting Xpetra_ENABLE_Thyra=ON since Thyra is NOT enabled at this point!
-- Setting Xpetra_ENABLE_BLAS=ON since Trilinos_ENABLE_Xpetra=ON AND TPL_ENABLE_BLAS=ON
-- Setting Xpetra_ENABLE_LAPACK=ON since Trilinos_ENABLE_Xpetra=ON AND TPL_ENABLE_LAPACK=ON
-- NOTE: Xpetra_ENABLE_Tpetra=ON is already set!
-- NOTE: Xpetra_ENABLE_KokkosKernels=ON is already set!
-- NOT setting Xpetra_ENABLE_Thyra=ON since Thyra is NOT enabled at this point!
...
-- NOT setting MueLu_ENABLE_Thyra=ON since Thyra is NOT enabled at this point!
-- Setting MueLu_ENABLE_ThyraTpetraAdapters=ON since Trilinos_ENABLE_MueLu=ON AND Trilinos_ENABLE_ThyraTpetraAdapters=ON

GrahamBenHarper avatar Jan 19 '23 22:01 GrahamBenHarper

@bartlettroscoe it looks like the TriBITS configure still doesn't pick up the MueLu/Xpetra dependency on Thyra properly for #11425.

Okay, let me give it another try to reproduce this failure.

bartlettroscoe avatar Jan 23 '23 18:01 bartlettroscoe

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[bot] avatar Feb 24 '24 12:02 github-actions[bot]

This issue was closed due to inactivity for 395 days.

github-actions[bot] avatar Mar 27 '24 12:03 github-actions[bot]