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

Tpetra: CrsGraph & CrsMatrix still use deprecated enum value DynamicProfile #5117

Closed
mhoemmen opened this issue May 7, 2019 · 9 comments
Closed
Assignees
Labels
impacting: configure or build The issue is primarily related to configuring or building pkg: Tpetra type: bug The primary issue is a bug in Trilinos code or tests

Comments

@mhoemmen
Copy link
Contributor

mhoemmen commented May 7, 2019

Bug Report

@trilinos/tpetra

Description

When I disable deprecated code in Tpetra, I get the following build errors:

In file included from .../Trilinos/packages/tpetra/core/src/Tpetra_CrsGraph_SerialWrapperNode.cpp:52:
.../Trilinos/packages/tpetra/core/src/Tpetra_CrsGraph_decl.hpp:2582:59: error: use of undeclared identifier
      'DynamicProfile'; did you mean 'StaticProfile'?
    return rcp (new graph_type (map, maxNumEntriesPerRow, DynamicProfile, params));
                                                          ^~~~~~~~~~~~~~
                                                          StaticProfile
.../Trilinos/packages/tpetra/core/src/Tpetra_ConfigDefs.hpp:131:5: note: 'StaticProfile' declared here
    StaticProfile
    ^
In file included from .../Trilinos/packages/tpetra/core/src/Tpetra_CrsGraph_SerialWrapperNode.cpp:54:
In file included from .../Trilinos/packages/tpetra/core/src/Tpetra_CrsGraph_def.hpp:66:
In file included from .../Trilinos/packages/tpetra/core/src/Tpetra_Import_Util2.hpp:64:
.../Trilinos/packages/tpetra/core/src/Tpetra_CrsMatrix_decl.hpp:5039:43: error: use of undeclared identifier
      'DynamicProfile'; did you mean 'StaticProfile'?
                                          DynamicProfile, params));
                                          ^~~~~~~~~~~~~~
                                          StaticProfile

When I fix those and enable tests, I get the following build errors:

.../Trilinos/packages/tpetra/core/test/CrsGraph/CrsGraph_insertGlobalIndicesFiltered.cpp:137:16: error: no member
      named 'DynamicProfile' in namespace 'Tpetra'; did you mean 'StaticProfile'?
      {Tpetra::DynamicProfile, Tpetra::StaticProfile};
       ~~~~~~~~^~~~~~~~~~~~~~
               StaticProfile
.../Trilinos/packages/tpetra/core/src/Tpetra_ConfigDefs.hpp:131:5: note: 'StaticProfile' declared here
    StaticProfile
    ^
...
.../Trilinos/packages/tpetra/core/test/CrsGraph/CrsGraph_UnitTests1.cpp:51:17: error: no member named
      'DynamicProfile' in namespace 'Tpetra'; did you mean 'StaticProfile'?
  using Tpetra::DynamicProfile;
        ~~~~~~~~^~~~~~~~~~~~~~
                StaticProfile
.../Trilinos/packages/tpetra/core/src/Tpetra_ConfigDefs.hpp:131:5: note: 'StaticProfile' declared here
    StaticProfile
    ^
.../Trilinos/packages/tpetra/core/test/CrsGraph/CrsGraph_UnitTests1.cpp:143:25: error: use of undeclared identifier
      'DynamicProfile'; did you mean 'StaticProfile'?
      GRAPH graph(map,1,DynamicProfile);
                        ^~~~~~~~~~~~~~
                        StaticProfile
.../Trilinos/packages/tpetra/core/test/CrsGraph/CrsGraph_UnitTests1.cpp:53:17: note: 'StaticProfile' declared here
  using Tpetra::StaticProfile;
                ^
.../Trilinos/packages/tpetra/core/test/CrsGraph/CrsGraph_UnitTests1.cpp:459:29: error: use of undeclared identifier
      'DynamicProfile'; did you mean 'StaticProfile'?
      GRAPH graph(map,map,0,DynamicProfile);
                            ^~~~~~~~~~~~~~
                            StaticProfile
.../Trilinos/packages/tpetra/core/test/CrsGraph/CrsGraph_UnitTests1.cpp:53:17: note: 'StaticProfile' declared here
  using Tpetra::StaticProfile;
                ^
.../Trilinos/packages/tpetra/core/test/CrsGraph/CrsGraph_UnitTests1.cpp:474:29: error: use of undeclared identifier
      'DynamicProfile'; did you mean 'StaticProfile'?
      GRAPH graph(map,map,0,DynamicProfile);
                            ^~~~~~~~~~~~~~
                            StaticProfile
.../Trilinos/packages/tpetra/core/test/CrsGraph/CrsGraph_UnitTests1.cpp:53:17: note: 'StaticProfile' declared here
  using Tpetra::StaticProfile;
                ^

When I fix those build errors, I get the following:

.../Trilinos/packages/tpetra/core/test/CrsGraph/CrsGraph_UnitTests_Swap.cpp:415:15: error: no member named
      'DynamicProfile' in namespace 'Tpetra'; did you mean 'StaticProfile'?
using Tpetra::DynamicProfile;
      ~~~~~~~~^~~~~~~~~~~~~~
              StaticProfile

When I fix that, I get the following:

.../Trilinos/packages/tpetra/core/test/CrsGraph/CrsGraph_Issue601.cpp:100:52: error: no member named
      'DynamicProfile' in namespace 'Tpetra'; did you mean 'StaticProfile'?
    Tpetra::ProfileType profileTypes[] = { Tpetra::DynamicProfile, Tpetra::StaticProfile };
                                           ~~~~~~~~^~~~~~~~~~~~~~
                                                   StaticProfile
.../Trilinos/packages/tpetra/core/src/Tpetra_ConfigDefs.hpp:131:5: note: 'StaticProfile' declared here
    StaticProfile
    ^
.../Trilinos/packages/tpetra/core/test/CrsGraph/CrsGraph_Issue601.cpp:107:39: error: no member named
      'DynamicProfile' in namespace 'Tpetra'; did you mean 'StaticProfile'?
          << ((profileType == Tpetra::DynamicProfile) ? "Dynamic" : "Static")
                              ~~~~~~~~^~~~~~~~~~~~~~
                                      StaticProfile

I get a few more after this in other tests in TpetraCore.

Steps to Reproduce

  1. SHA1: 2b9c4fe
@mhoemmen mhoemmen added type: bug The primary issue is a bug in Trilinos code or tests impacting: configure or build The issue is primarily related to configuring or building pkg: Tpetra labels May 7, 2019
@mhoemmen mhoemmen changed the title PackageName: General Summary of the Bug Tpetra: CrsGraph & CrsMatrix still use deprecated enum value DynamicProfile May 7, 2019
mhoemmen pushed a commit to mhoemmen/Trilinos that referenced this issue May 7, 2019
@trilinos/tpetra

TpetraCore's tests and examples build, but some tests still fail.
@mhoemmen
Copy link
Contributor Author

mhoemmen commented May 7, 2019

When I fixed build errors, some Tpetra tests failed. Here is my PR with the attempt: #5118 (don't merge it yet!).

@tjfulle tjfulle self-assigned this May 7, 2019
tjfulle added a commit to tjfulle/Trilinos that referenced this issue May 9, 2019
@trilinos/tpetra
Issue: trilinos#5117

- Fixes build errors in Tpetra's `CrsMatrix::createCrsMatrix` and
  `CrsGraph::createCrsGraph` associated with `DynamicProfile`
- Fixes many build errors in Tpetra's tests

Still has one build error:

```
duplicate symbol typeinfo name for Tpetra::Experimental::BlockMultiVector<double, int, long long, Kokkos::Compat::KokkosDeviceWrapperNode<Kokkos::Serial, Kokkos::HostSpace> > in:
    packages/tpetra/core/src/CMakeFiles/tpetra.dir/Tpetra_Experimental_BlockMultiVector.cpp.o
    packages/tpetra/core/src/CMakeFiles/tpetra.dir/Tpetra_Experimental_BlockMultiVector_DOUBLE_INT_LONG_LONG_SERIAL.cpp.o
duplicate symbol typeinfo for Tpetra::Experimental::BlockMultiVector<double, int, long long, Kokkos::Compat::KokkosDeviceWrapperNode<Kokkos::Serial, Kokkos::HostSpace> > in:
    packages/tpetra/core/src/CMakeFiles/tpetra.dir/Tpetra_Experimental_BlockMultiVector.cpp.o
    packages/tpetra/core/src/CMakeFiles/tpetra.dir/Tpetra_Experimental_BlockMultiVector_DOUBLE_INT_LONG_LONG_SERIAL.cpp.o
ld: 2 duplicate symbols for architecture x86_64
collect2: error: ld returned 1 exit status
ninja: build stopped: subcommand failed.
```
@tjfulle
Copy link
Contributor

tjfulle commented May 9, 2019

Hmmm, after fixing the build errors (see branch tjfulle/Trilinos@c6d04b22) associated with DynamicProfile, I get a link error:

duplicate symbol typeinfo name for Tpetra::Experimental::BlockMultiVector<double, int, long long, Kokkos::Compat::KokkosDeviceWrapperNode<Kokkos::Serial, Kokkos::HostSpace> > in:
    packages/tpetra/core/src/CMakeFiles/tpetra.dir/Tpetra_Experimental_BlockMultiVector.cpp.o
    packages/tpetra/core/src/CMakeFiles/tpetra.dir/Tpetra_Experimental_BlockMultiVector_DOUBLE_INT_LONG_LONG_SERIAL.cpp.o
duplicate symbol typeinfo for Tpetra::Experimental::BlockMultiVector<double, int, long long, Kokkos::Compat::KokkosDeviceWrapperNode<Kokkos::Serial, Kokkos::HostSpace> > in:
    packages/tpetra/core/src/CMakeFiles/tpetra.dir/Tpetra_Experimental_BlockMultiVector.cpp.o
    packages/tpetra/core/src/CMakeFiles/tpetra.dir/Tpetra_Experimental_BlockMultiVector_DOUBLE_INT_LONG_LONG_SERIAL.cpp.o
ld: 2 duplicate symbols for architecture x86_64
collect2: error: ld returned 1 exit status
ninja: build stopped: subcommand failed.

@mhoemmen
Copy link
Contributor Author

@tjfulle Where does this link error occur? -- on what file?

@tjfulle
Copy link
Contributor

tjfulle commented May 15, 2019

I have a branch that now builds. Many tests fail because export/import operations that resize the target matrix are not yet supported (as they are for CrsGraph, see #4147). The tests could be made to pass by allocating sufficient space to the target matrices, but a longer term solution is to modify CrsMatrix to allow a target matrix (with static profile) to expand during import/export, which is being tracked in #5164

@mhoemmen
Copy link
Contributor Author

mhoemmen commented Jun 4, 2019

Given that #5268 merged into develop 3 days ago and is probably in master by now, can we consider #5164 fixed?

@kddevin
Copy link
Contributor

kddevin commented Jun 5, 2019

This issue duplicates #5065.
PR #5326 is a step toward resolving it.

@bartlettroscoe
Copy link
Member

This is vomiting hundreds of warnings all over the SPARC build:

/scratch/rabartl/SPARC.base/sparc/Trilinos/spack-rhel6_gnu-7.2.0_serial_static_opt/include/Tpetra_CrsMatrix_decl.hpp:5039:43: warning: ‘Tpetra::DynamicProfile’ is deprecated [-Wdeprecated-declarations]
                                           DynamicProfile, params));
                                           ^~~~~~~~~~~~~~

There is nothing they can do to avoid these warnings since they are in Trilinos.

We need to have a policy that if someone is going to deprecate something in Trilinos then they need to build all downstream Trilinos packages and remove usage of all deprecated usage in those packages.

@kddevin
Copy link
Contributor

kddevin commented Jun 13, 2019

We are deprecating DynamicProfile construction in Tpetra. We have removed many downstream uses (e.g., #4350 #4351 #4411 #5326 #5268 #5130 #4701). Some cases remain (e.g., #5065 and this issue). The warning generation will continue to reduce over the next weeks. Thanks for your patience.

@bartlettroscoe
Copy link
Member

Can't these particular deprecated warnings be turned off by default until all of the usages of these deprecated warnings can be removed from Trilinos? Then Tpetra and other developers can turn back on these particular warnings at will to keep working on getting rid if their usage downstream in Trilinos. I can show how this would work. It is very little CMake and C++ preprocessor code.

What we are doing right now is training users of Trilinos to ignore deprecated warnings which defeats the purpose of deprecated warnings.

jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Jun 20, 2019
…s:develop' (d1e19b9).

* trilinos-develop:
  Error out if trying to set Kokkos Pthreads backend (TRIL-272)
  tpetra:  changes to reduce the number of deprecated warnings issued trilinos#5117
  Tpetra: Fix trilinos#5408 (build warning in CrsGraph)
  Tpetra::CrsMatrix: Fix trilinos#5407
  Teuchos::ParameterList: Fix 2 shadowing warnings
  Tpetra: Add benchmark for CrsMatrix::sumIntoLocalValues
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Jun 20, 2019
…s:develop' (d1e19b9).

* trilinos-develop:
  Error out if trying to set Kokkos Pthreads backend (TRIL-272)
  tpetra:  changes to reduce the number of deprecated warnings issued trilinos#5117
  Tpetra: Fix trilinos#5408 (build warning in CrsGraph)
  Tpetra::CrsMatrix: Fix trilinos#5407
  Teuchos::ParameterList: Fix 2 shadowing warnings
  Tpetra: Add benchmark for CrsMatrix::sumIntoLocalValues
@kddevin kddevin closed this as completed Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impacting: configure or build The issue is primarily related to configuring or building pkg: Tpetra type: bug The primary issue is a bug in Trilinos code or tests
Projects
None yet
Development

No branches or pull requests

4 participants