Trilinos merge requestshttps://gitlab.osti.gov/jmwille/Trilinos/-/merge_requests2018-09-11T17:56:43Zhttps://gitlab.osti.gov/jmwille/Trilinos/-/merge_requests/3398(WIP) Tpetra: Remove template parameters2018-09-11T17:56:43ZJames Willenbring(WIP) Tpetra: Remove template parameters*Created by: mhoemmen*
@trilinos/tpetra
**THIS IS A WORK IN PROGRESS.** Please don't merge this yet.
## Description
Start removing the LocalOrdinal, GlobalOrdinal, and Node template parameters from Tpetra. **THIS IS A WORK I...*Created by: mhoemmen*
@trilinos/tpetra
**THIS IS A WORK IN PROGRESS.** Please don't merge this yet.
## Description
Start removing the LocalOrdinal, GlobalOrdinal, and Node template parameters from Tpetra. **THIS IS A WORK IN PROGRESS.** I've added new versions of the following classes that no longer have any of these template parameters:
- Details::Directory and subclasses
- Directory
- Map
- ImportExportData
- Transfer
- Export
- Import
I wanted these classes to build alongside the current Tpetra classes, so I could test and compare them, and have a gradual porting path. (Note use of `::Tpetra` to pull in enums etc. from the Tpetra namespace, that I didn't want to duplicate. My goal is to avoid duplication whenever possible.) To prevent namespace drama, all new classes live in an entirely new namespace (TpetraNew), that is not nested in the Tpetra namespace.
I currently have tests for Map and Directory. The tests build and pass on my laptop. Testing Export and Import will take a bit more work, since very few of the current tests for these classes work in isolation. (Most of them depend on MultiVector, Vector, etc. and exercise DistObject.)
This was not very hard to do; it was mostly just tedious. The most important change is that the "decl" and "def" header files go away. Instead, there's a true header file (e.g., `Tpetra_Map.hpp`) and a .cpp file that actually has the implementation in it (no more explicit template instantiation (ETI)). We will still need ETI machinery for classes templated on Scalar or Packet, but we will be able to simplify the CMake logic for ETI quite a bit.
## Next steps
The next class to convert is DistObject. That will be the first class to convert that will retain a template parameter (Packet).
I can defer changing Distributor, since it does not depend on the usual three template parameters; however, I have some future plans to hide Distributor's implementation that I can discuss elsewhere.
After DistObject will come RowGraph and CrsGraph. After those, will come Operator, then all of the other DistObject subclasses and the various functions that work with them. (The latter two will need to happen simultaneously -- e.g., for nonmember functions that work on CrsMatrix.)
## Risks
1. _Build size and time increase_: We could avoid this by making TpetraNew a new Tpetra subpackage. That way, it would be a separate library, and users could avoid linking to it.
2. _Merging with other work_, such as @DrBooom 's incoming changes.
3. _Huge amount of work downstream_ to use this: We could mitigate and make this a gradual porting exercise with conversion functions, e.g., between old and new versions of the same data structure.
## Related Issues
* Closes #57, #2548
https://gitlab.osti.gov/jmwille/Trilinos/-/merge_requests/4400Xpetra: fixed dependency on Tpetra instantations2019-04-12T20:25:54ZJames WillenbringXpetra: fixed dependency on Tpetra instantations*Created by: brian-kelley*
Satisfy dependency of Xpetra EpetraMap on
Tpetra::Details::FixedHashTable with int/long and
int/longlong as LO/GO. These dependencies always exist
as long as Tpetra is enabled, but regardless of whether
th...*Created by: brian-kelley*
Satisfy dependency of Xpetra EpetraMap on
Tpetra::Details::FixedHashTable with int/long and
int/longlong as LO/GO. These dependencies always exist
as long as Tpetra is enabled, but regardless of whether
these LO/GO combinations are enabled in Tpetra ETI.
<!---
Be sure to select `develop` as the `base` branch against which to create this
pull request. Only pull requests against `develop` will undergo Trilinos'
automated testing. Pull requests against `master` will be ignored.
Provide a general summary of your changes in the Title above. If this pull
request pertains to a particular package in Trilinos, it's worthwhile to start
the title with "PackageName: ".
Note that anything between these delimiters is a comment that will not appear
in the pull request description once created. Most areas in this message are
commented out and can be easily added by removing the comment delimiters.
Please make sure to mark:
* Reviewers
* Assignees
* Labels
Replace <teamName> below with the appropriate Trilinos package/team name.
-->
@trilinos/xpetra
## Description
<!--- Please describe your changes in detail. -->
Xpetra_EpetraMap.cpp now includes the def of
FixedHashTable so that these instantiations can be made, even if int/long
and/or int/longlong haven't been enabled in ETI. The include is only done if
Tpetra and Xpetra kokkos refactor are both enabled, because Xpetra::EpetraMap
only uses FixedHashTable in this case.
## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
This fixes an "undefined symbol" error during the final link of Albany (or any other app executable). Without this code change, the only workaround is to enable both TPETRA_INST_INT_LONG and TPETRA_INST_INT_LONG_LONG in configuration (huge code size increase). Only the instantiation of FixedHashTable is needed, not all of Tpetra.
<!---
If applicable, let us know how this merge request is related to any other open
issues or pull requests:
## Related Issues
* Closes
* Blocks
* Is blocked by
* Follows
* Precedes
* Part of
* Composed of
-->
* Related to #4038
## How Has This Been Tested?
<!---
Please describe in detail how you tested your changes. Include details of your
testing environment and the tests you ran to see how your change affects other
areas of the code. Consider including configure, build, and test log files.
-->
Tested by successfully building Trilinos/Albany without TPETRA_INST_INT_LONG.
<!---
## Screenshots
Not obligatory, but is there anything pertinent that we should see?
-->
<!---
Go over all the following points, and put an `x` in all the boxes that apply.
If you are unsure about any of these, please ask—we are here to help.
-->
## Checklist
- [ ] My commit messages mention the appropriate GitHub issue numbers.
- [x] My code follows the code style of the affected package(s).
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [ ] I have read the [code contribution guidelines](../blob/master/CONTRIBUTING.md) for this project.
- [ ] I have added tests to cover my changes.
- [ ] All new and existing tests passed.
- [x] No new compiler warnings were introduced.
- [ ] These changes break backwards compatibility.
<!---
## Additional Information
Anything else we need to know in evaluating this merge request?
-->