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

Panzer: build fails with Epetra disabled #5592

Closed
Tracked by #5602
tjfulle opened this issue Jul 30, 2019 · 16 comments
Closed
Tracked by #5602

Panzer: build fails with Epetra disabled #5592

tjfulle opened this issue Jul 30, 2019 · 16 comments
Assignees
Labels
AT: WIP Causes the PR autotester to not test the PR. (Remove to allow testing to occur.) pkg: Panzer 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/panzer

When Trilinos_ENABLE_Epetra:BOOL=OFF is defined, I get the following build error:

packages/panzer/dof-mgr/src/Panzer_GlobalIndexer_EpetraUtilities.hpp:52:10: fatal error: Epetra_Vector.h: No such file or directory
 #include <Epetra_Vector.h>

Full configure:

/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 Trilinos_ENABLE_Galeri:BOOL=OFF \
  -D Rythmos_ENABLE_TESTS:BOOL=OFF \
  -D Trilinos_ENABLE_FEI:BOOL=OFF \
  -D MueLu_ENABLE_TESTS:BOOL=OFF \
  $TRILINOS_DIR
@tjfulle tjfulle added the type: bug The primary issue is a bug in Trilinos code or tests label Jul 30, 2019
@rppawlo
Copy link
Contributor

rppawlo commented Jul 30, 2019

yes - panzer currently requires both the type 1 and type 2 stacks to be enabled. It's on the list of things to fix in FY20. What is the timeline/priority for this?

@tjfulle
Copy link
Contributor Author

tjfulle commented Jul 30, 2019

@rppawlo, by type 1 and 2, are you referring to Epetra and Tpetra? If so, I disabled Epetra because many packages assume that if Epetra is enabled then Tpetra’s GO=int - which is not true by default. Those build failures will eventually be addressed in the PR. So long as Panzer is ok with different GO in [ET]petra, or Trilinos is built with (non-default) GO=int in Tpetra, it should be fine.

@tjfulle
Copy link
Contributor Author

tjfulle commented Jul 30, 2019

Just noticed I’m commenting in this issue and not PR #5590 - which was the genesis for this issue

@rppawlo
Copy link
Contributor

rppawlo commented Jul 30, 2019

@tjfulle - yes - type 1 is epetra/amesos/ifpack/ml and type 2 is tpetra/amesos2/ifpack2/muelu. Panzer can use any GO for tpetra, however we always assume epetra is 32-bit. Unfortunately, almost all of our testing is based on epetra, so it will be a lot of work to disable complete type 1 stack and still have adequate test converage of panzer. Most of panzer is agnostic to epetra/tpetra but uses epetra for assembly. Its not hard to change but is a lot of busy work. I can move up priority if needed. Was targeting FY20/Q2

@tjfulle
Copy link
Contributor Author

tjfulle commented Jul 30, 2019

I don’t know that it is a priority for Tpetra. I just wanted to build Tpetra+downstream with deprecated code off (using default ordinal types). Many packages downstream of Tpetra fail to build in this case, so disabling Epetra and any other packages that fail to build with Epetra disabled was my first pass.

Perhaps Panzer can add some CMake logic to catch this at configure, rather than build, time?

@rppawlo
Copy link
Contributor

rppawlo commented Jul 30, 2019

It should catch this at configure. Epetra is a required package in panzer's disc-fe subpackage. I'll take a look at this.

@rppawlo rppawlo self-assigned this Jul 30, 2019
@rppawlo rppawlo added AT: WIP Causes the PR autotester to not test the PR. (Remove to allow testing to occur.) pkg: Panzer labels Jul 30, 2019
@mhoemmen
Copy link
Contributor

Panzer can use any GO for tpetra, however we always assume epetra is 32-bit.

This is a bit of a problem for @trilinos/xpetra and @trilinos/muelu. The current plan is that if you enable Epetra support in Xpetra and MueLu, then you must enable GO=int. This excludes enabling both Epetra and Tpetra with deprecated code disabled in Tpetra.

@tjfulle
Copy link
Contributor Author

tjfulle commented Jul 30, 2019

@mhoemmen does it? Can’t we disable deprecated code and explicitly set Tpetra_INST_INT_INT?

@rppawlo
Copy link
Contributor

rppawlo commented Jul 30, 2019

This is a bit of a problem for @trilinos/xpetra and @trilinos/muelu.

We have been disabling Epetra support in Xpetra and Muelu in our builds. We only use xpetra/muelu for type 2 stack runs. We never call that code with epetra. I think ATDM might have adopted these settings already. we should check with @bartlettroscoe .

@bartlettroscoe
Copy link
Member

The settings ATDM is using can be seen in:

which includes:

ATDM_SET_CACHE(Tpetra_INST_INT_INT OFF CACHE BOOL)

Any deviation from the ATDM settings is may not work so my advice is to just use the ATDM Trilinos configuration if you want to be productive.

@tjfulle
Copy link
Contributor Author

tjfulle commented Jul 30, 2019

@bartlettroscoe said

Any deviation from the ATDM settings is may not work so my advice is to just use the ATDM Trilinos configuration if you want to be productive.

That is probably true, but @trilinos/tpetra plans to remove deprecated code soon and many packages don’t yet build if deprecated code is not enabled. This, and many other issues, were opened when build failures occurred with deprecated code disabled but otherwise default test settings.

@rppawlo
Copy link
Contributor

rppawlo commented Jul 30, 2019

So to follow up - we are able to build using a single GO type and are able to jump between the type 1 and type 2 stacks at runtime. It is really useful in certain applications to do performance testing comparisons of the two stacks from the same executable. This would be a painful change for some applications that currently assume both stacks must be built built, but we could do it.

@bartlettroscoe
Copy link
Member

NOTE: several non-ATDM Trilinos packages don't currently build with Tpetra_INST_INT_INT=OFF. See, for example this build (which @fryeguy52 will create a GitHub issue for when he has the time as per ATDV-188).

@tjfulle
Copy link
Contributor Author

tjfulle commented Jul 30, 2019

@rppawlo, when the dust settles, I hope to have things fixed to the point where (with deprecated code disabled and eventually removed) the current default test settings will just work. To get there many packages need to be updated to work with any Tpetra GO and Epetra - or fail at configure time if they can’t. Direct comparison between the two stacks may require building Tpetra with GO=int (which will not be built by default in the coming weeks)

@mhoemmen
Copy link
Contributor

Direct comparison between the two stacks may require building Tpetra with GO=int

Right, this is the only way that MueLu plans to permit enabling both Epetra and Tpetra support. However, one could always enable Epetra but disable support for Epetra in Amesos2, Xpetra, and MueLu.

@tjfulle
Copy link
Contributor Author

tjfulle commented Jul 30, 2019

... but disable support for Epetra in Amesos2, Xpetra, and MueLu.

Not quite yet, I still have 3-4 packages that won’t build with deprecated code disabled - but we’re getting there!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AT: WIP Causes the PR autotester to not test the PR. (Remove to allow testing to occur.) pkg: Panzer type: bug The primary issue is a bug in Trilinos code or tests
Projects
None yet
Development

No branches or pull requests

4 participants