trilinos / Trilinos

Primary repository for the Trilinos Project
https://trilinos.org/
Other
1.22k stars 568 forks source link

Panzer: ThyraEpetraAdapters does not enable and causes the build to fail #11402

Closed GrahamBenHarper closed 1 year ago

GrahamBenHarper commented 1 year ago

Bug Report

I noticed my nightly build for MrHyDE (which uses Trilinos on develop as a library) now fails due to a recent change in Panzer, which I suspect is related to #11383. The failure is as follows:

In file included from /ascldap/users/MrHyDE/nightly/Trilinos/Trilinos-source/packages/panzer/disc-fe/src/lof/Panzer_BlockedEpetraLinearObjFactory.hpp:57,
                 from /ascldap/users/MrHyDE/nightly/Trilinos/Trilinos-source/packages/panzer/mini-em/src/eqn_sets/MiniEM_AuxiliaryEquationSet_MassMatrix_impl.hpp:25,
                 from /ascldap/users/MrHyDE/nightly/Trilinos/Trilinos-source/packages/panzer/mini-em/src/eqn_sets/MiniEM_AuxiliaryEquationSet_MassMatrix.cpp:4:
/ascldap/users/MrHyDE/nightly/Trilinos/Trilinos-source/packages/panzer/disc-fe/src/lof/Panzer_EpetraLinearObjContainer.hpp:61:10: fatal error: Thyra_EpetraLinearOp.hpp: No such file or directory
   61 | #include "Thyra_EpetraLinearOp.hpp"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

Description

After some additional digging, I found out my build no longer enables ThyraEpetraAdapters as Panzer does not require it, but Panzer attempts to build certain Epetra objects that require ThyraEpetraAdapters anyway.

Steps to Reproduce

Trilinos Develop SHA: 1e393b02bb34862a63983b954e51de8b76fc73d1 Here's the relevant lines from the configure script I was using

  -D Trilinos_ENABLE_Amesos:BOOL=ON \
  -D Trilinos_ENABLE_Amesos2:BOOL=ON \
  -D Trilinos_ENABLE_AztecOO:BOOL=ON \
  -D Trilinos_ENABLE_Belos:BOOL=ON \
  -D Trilinos_ENABLE_Compadre:BOOL=ON \
  -D Trilinos_ENABLE_FEI:BOOL=ON \
  -D Trilinos_ENABLE_Ifpack:BOOL=ON \
  -D Trilinos_ENABLE_ML:BOOL=ON \
  -D Trilinos_ENABLE_MueLu:BOOL=ON \
  -D Trilinos_ENABLE_Pamgen:BOOL=ON \
  -D Trilinos_ENABLE_Panzer:BOOL=ON \
    -D Panzer_ENABLE_TESTS:BOOL=OFF \
    -D Panzer_ENABLE_EXAMPLES:BOOL=OFF \
    -D Panzer_ENABLE_EXPLICIT_INSTANTIATION:BOOL=ON \
  -D Trilinos_ENABLE_ROL:BOOL=ON \
  -D Trilinos_ENABLE_SEACAS:BOOL=ON \
    -D SEACASExodus_ENABLE_MPI:BOOL=OFF \
  -D Trilinos_ENABLE_Shards:BOOL=ON \
  -D Trilinos_ENABLE_STK:BOOL=ON \
    -D STK_ENABLE_TESTS:BOOL=OFF \
  -D Trilinos_ENABLE_Stratimikos:BOOL=ON \
  -D Trilinos_ENABLE_Teko:BOOL=ON \
  -D Trilinos_ENABLE_Teuchos:BOOL=ON \
  -D Trilinos_ENABLE_Zoltan:BOOL=ON \

Here's my final list of enabled packages:

Final set of enabled top-level packages:  Gtest Kokkos Teuchos KokkosKernels RTOp Sacado Epetra Zoltan Shards Triutils Tpetra TrilinosSS Thyra Xpetra AztecOO Galeri Amesos Pamgen Zoltan2Core Ifpack ML Belos Amesos2 S
EACAS Anasazi Ifpack2 Stratimikos FEI Teko Intrepid2 Compadre STK Phalanx NOX MueLu Rythmos ROL Piro Panzer 39

Final set of enabled packages:  Gtest KokkosCore KokkosContainers KokkosAlgorithms Kokkos TeuchosCore TeuchosParser TeuchosParameterList TeuchosComm TeuchosNumerics TeuchosRemainder TeuchosKokkosCompat TeuchosKokkosC
omm Teuchos KokkosKernels RTOp Sacado Epetra Zoltan Shards Triutils TpetraCore Tpetra TrilinosSS ThyraCore ThyraTpetraAdapters Thyra Xpetra AztecOO Galeri Amesos Pamgen Zoltan2Core Ifpack ML Belos Amesos2 SEACASExodu
s SEACASExodus_for SEACASExoIIv2for32 SEACASNemesis SEACASIoss SEACASChaco SEACASAprepro_lib SEACASSupes SEACASSuplib SEACASSuplibC SEACASSuplibCpp SEACASSVDI SEACASPLT SEACASAlgebra SEACASAprepro SEACASBlot SEACASCo
njoin SEACASEjoin SEACASEpu SEACASCpup SEACASExo2mat SEACASExodiff SEACASExomatlab SEACASExotxt SEACASExo_format SEACASEx1ex2v2 SEACASFastq SEACASGjoin SEACASGen3D SEACASGenshell SEACASGrepos SEACASExplore SEACASMapv
arlib SEACASMapvar SEACASMapvar-kd SEACASMat2exo SEACASNas2exo SEACASZellij SEACASNemslice SEACASNemspread SEACASNumbers SEACASSlice SEACASTxtexo SEACASEx2ex1v2 SEACAS Anasazi Ifpack2 Stratimikos FEI Teko Intrepid2 C
ompadre STKUtil STKCoupling STKMath STKSimd STKNGP_TEST STKTopology STKMesh STKIO STKSearch STKTransfer STKTools STKBalance STKUnit_test_utils STKSearchUtil STKUnit_tests STKDoc_tests STKExprEval STKEmend STK Phalanx
 NOX MueLu Rythmos ROL Piro PanzerCore PanzerDofMgr PanzerDiscFE PanzerAdaptersSTK PanzerMiniEM Panzer 120

Final set of non-enabled top-level packages:  TrilinosFrameworkTests TrilinosATDMConfigTests MiniTensor EpetraExt Domi Isorropia Pliris ShyLU_Node Komplex TriKota Intrepid Percept Krino Moertel Zoltan2Sphynx Zoltan2 
ShyLU_DD ShyLU Tempus Stokhos PyTrilinos NewPackage Adelus TrilinosCouplings Pike TrilinosBuildStats TrilinosInstallTests 27

Final set of non-enabled packages:  TrilinosFrameworkTests TrilinosATDMConfigTests KokkosSimd MiniTensor EpetraExt TpetraTSQR Domi ThyraEpetraAdapters ThyraEpetraExtAdapters Isorropia Pliris ShyLU_NodeHTS ShyLU_NodeT
acho ShyLU_NodeBasker ShyLU_NodeFastILU ShyLU_Node SEACASExotec2 Komplex TriKota Intrepid Percept Krino Moertel Zoltan2Sphynx Zoltan2 ShyLU_DDFROSch ShyLU_DDCore ShyLU_DDCommon ShyLU_DD ShyLU Tempus Stokhos PanzerExp
rEval PyTrilinos NewPackage Adelus TrilinosCouplings PikeBlackBox PikeImplicit Pike TrilinosBuildStats TrilinosInstallTests 4

I believe this can be fixed by adding something like the following to my configure script

  -D Trilinos_ENABLE_Thyra:BOOL=ON \
    -D Thyra_ENABLE_EpetraAdapters:BOOL=ON \

However, I'm waiting for the build to finish in order to confirm. The more important thing is that Panzer with Epetra support on develop seems to configure properly and then result in a build failure unless ThyraEpetraAdapters is explicitly enabled by the user.

Edit: Despite the configure complaining Thyra_ENABLE_EpetraAdapters was incorrect/unused, it seems that manually enabling Thyra is enough to fix the build.

GrahamBenHarper commented 1 year ago

Whoops, @trilinos/panzer @rppawlo

sebrowne commented 1 year ago

I think it makes more sense to change ThyraEpetraAdapters to a required dep package in packages/panzer/disc-fe/cmake/Dependencies.cmake, or to resolve the includes to check if ThyraEpetraAdapters was in fact enabled or not.

That being said, I skimmed the linked PR and don't see how it would cause ThyraEpetraAdapters to not be enabled any more. That kind of thing would have to be in Dependencies.cmake somewhere (I think).

GrahamBenHarper commented 1 year ago

@sebrowne I think the goal is to make a lot of the dependencies on Epetra optional. However, doing so means the logic is flaky. Basically, the logic for most of these packages looks like this: if Epetra is enabled, then we must also enable Epetra-related subpackages like ThyraEpetraAdapters and ThyraEpetraExtAdapters.

However, this isn't being picked up. Trilinos is enabling Epetra and Epetra support in Panzer/Teko, but at configure time it's not enabling the necessary subpackages like ThyraEpetraAdapters/ThyraEpetraExtAdapters, causing the build to eventually fail when it reaches the point where it needs that subpackage. I don't know how to do this within the Tribits framework, but really we need logic like this:

rppawlo commented 1 year ago

I'm working on this now.

cgcgcg commented 1 year ago

Thanks @rppawlo . I attempted #11407 over the weekend, but something is wrong.

rppawlo commented 1 year ago

I think the issue is that packages like panzer can't enable code unless both optional packages (Epetra and ThyraEpetraAdapters) are enabled. So instead of keying off whether one package is enabled (Epetra), the ifdef used to turn off code needs to check for all optional packages it needs. We do this in nox:

COMBINED_OPTION( NOX_ENABLE_THYRA_EPETRA_ADAPTERS
  DEP_OPTIONS_NAMES NOX_ENABLE_ThyraCore NOX_ENABLE_ThyraEpetraAdapters NOX_ENABLE_ThyraEpetraExtAdapters
  DOCSTR "Enables support for the Thyra Epetra(Ext) adapters "
    " Defaults to ON if the Thyra Epetra(Ext) adapters are both enabled,"
    " otherwise defaults to OFF"
  )

I think changing to similar logic in teko and panzer (and maybe other downstream packages) is needed.

GrahamBenHarper commented 1 year ago

Thanks @rppawlo!

rppawlo commented 1 year ago

@GrahamBenHarper @ikalash @glhenni

This is taking a little longer than expected. It's not just panzer and teko but nox and piro as well. We can't look at what packages are already enabled and then force another upstream package to be enabled. It's too late as all the other upstream packages have already been configured. The build should definitely not fail though. The fix is easy for panzer and teko, but nox and piro have some pretty complex dependency cases in the testing. Hope to have it in by today.

One simple fix to get the same behavior as before is the add the flag Trilinos_ENABLE_ALL_OPTIONAL_PACKAGES=ON or in the case of Charon, tcad-charon_ENABLE_ALL_OPTIONAL_PACKAGES=ON. Since Epetra is now optional, if you do want to manually enable packages, enabling Thyra is enough as @GrahamBenHarper mentioned above. If you enable the meta-package, all non-experimental optional subpackages get enabled as well.

ikalash commented 1 year ago

Thanks @rppawlo for working on this and for the update. @GrahamBenHarper 's fix suggested above worked for me in the spack build. I will keep that as a workaround so the more correct fix involving ifdefs is not urgent for me.

GrahamBenHarper commented 1 year ago

I think this is resolved by #11415. I'm not sure why Github didn't automatically close this when the PR had the correct "Closes #XXXXX" in the description. I'll reopen if I see any issues, but it also seems like we're now able to go Epetra-free with MrHyDE now :)