Skip to content

Conversation

@klevzoff
Copy link
Collaborator

@klevzoff klevzoff commented Aug 3, 2020

This PR works around the following problem:

ParallelTopologyChange.hpp includes SurfaceGenerator.hpp. When GEOSX is compiled with TrilinosTpetra as selected LAI, it therefore transitively includes TpetraVector and TpetraMatrix, which include forward declaration headers from Tpetra. However, because PTP does not have either linearAlgebra or physicsSolvers as its dependency, BLT/Cmake does not place Trilinos install dir on its include paths, and the compiler fails to find those Tpetra headers. I simply added linearAlgebra as a dependency for now (adding physicsSolvers would create a circular dependency).

Couple notes:

  1. This was not an issue with Epetra-based interface (or Hypre/Petsc) because we did not include any Epetra headers in EpetraVector or EpetraMatrix, instead simply forward declaring the types ourselves. This is significantly more difficult to do with Tpetra, because the types are templates with a bunch of default parameters that are defined deep in Tpetra headers at config time. It would be unwise for us to try replicate all those namespaces/types. Luckily, Tpetra provides forward declaration headers for every public type (e.g. Tpetra_Vector_fwd.hpp), and we have to include those.
  2. This workaround is not great, because obviously PTP does not have a dependency on linearAlgebra. Root of the issue is that it includes SurfaceGenerator.hpp only for the definition of ModifiedObjectLists. A better solution would be to put ModifiedObjectLists in its own header and forward declare it in both of the above. Or at least move definition to ParallelTopologyChange.hpp. Let me know your preferences.

Related to GEOS-DEV/GEOS#1086

@klevzoff klevzoff self-assigned this Aug 3, 2020
@klevzoff klevzoff mentioned this pull request Aug 3, 2020
3 tasks
@klevzoff klevzoff marked this pull request as ready for review August 8, 2020 06:19
@klevzoff klevzoff requested a review from rrsettgast August 8, 2020 06:19
…ing Tpetra headers included in PTP via SurfaceGenerator. This change should be reverted if PTP is moved into main code.
@klevzoff klevzoff force-pushed the feature/klevzoff/tpetra branch from 4417a17 to f67d3e5 Compare September 11, 2020 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants