trilinos / Trilinos

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

Tpetra: Use of ETI macros should not require enclosing them in Tpetra namespace #3230

Open mhoemmen opened 6 years ago

mhoemmen commented 6 years ago

@trilinos/tpetra

Tpetra currently requires that its explicit template instantiation (ETI) macros be used inside the Tpetra namespace. For example:

namespace Tpetra {
  SOME_ETI_MACRO_GOES_HERE(...); // just a representation
};

This hinders moving Tpetra classes around and hiding this from users with aliases.

Motivation and Context

I'm working on #2548 and #57. As part of that, I've been applying the following procedure to Tpetra classes:

For all Tpetra classes CLASS that take 3 or 4 template parameters:

  1. Move Tpetra class ${CLASS} from Tpetra namespace to Tpetra::Classes namespace.
  2. Add forward declaration header file Tpetra_${CLASS}_fwd.hpp, that forward-declares Tpetra::Class::${CLASS}, and adds a template alias to that class in the Tpetra namespace.
  3. Change ETI macro for ${CLASS}, to put the instantiation in the Classes namespace (Tpetra expects the macros to be used in the Tpetra namespace).
  4. Fix any forward declarations in downstream classes.

This was going fine, until I got to Tpetra::CrsMatrix. The problem is that Stokhos actually specializes CrsMatrix for various Scalar types. That was working fine for MultiVector, but not for CrsMatrix. As I looked through Stokhos' ETI macros for its specializations of Tpetra classes (for its own Scalar types, specifically UQ PCE), I noticed that they all had to be used inside the Tpetra namespace. Some macros actually needed to be used in the Tpetra::Classes namespace, and others just in the Tpetra namespace. It was a mess that I eventually was able to work around, but it showed how requiring macros to be used in a particular namespace was leading to trouble.

Possible Solution

Put the namespaces themselves in the macro definition:

#define TPETRA_FOO_INST(SC, LO, GO, NT) \
  namespace Tpetra { namespace Classes { template class Foo<SC, LO, GO, NT>; } }
github-actions[bot] commented 3 years ago

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.

mhoemmen commented 3 years ago

@csiefer2