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

Amesos2,Tpetra: Remove HAVE_STD_NEW_COUNT_SYNTAX macro #3752

Closed
mhoemmen opened this issue Oct 29, 2018 · 0 comments
Closed

Amesos2,Tpetra: Remove HAVE_STD_NEW_COUNT_SYNTAX macro #3752

mhoemmen opened this issue Oct 29, 2018 · 0 comments

Comments

@mhoemmen
Copy link
Contributor

mhoemmen commented Oct 29, 2018

@trilinos/amesos2 @trilinos/tpetra

$ git grep HAVE_STD_NEW_COUNT_SYNTAX
packages/amesos2/cmake/Amesos2_config.h.in:#cmakedefine HAVE_STD_NEW_COUNT_SYNTAX
packages/tpetra/core/cmake/TpetraCore_config.h.in:#cmakedefine HAVE_STD_NEW_COUNT_SYNTAX

Nothing in Trilinos uses the HAVE_STD_NEW_COUNT_SYNTAX macro, in either package. No CMake code in Trilinos defines this macro. Furthermore, since both packages enable exactly the same macro, the macro could only possibly be defined if exactly one of the two packages is enabled. Otherwise, a build error would result. However, the Dashboard tests and users regularly build with both packages enabled. This suggests that neither package actually ever defines this macro.

git blame on the Tpetra file shows that the macro definition is almost 10 years old:

551d96784e4 packages/tpetra/cmake/Tpetra_config.h.in          (Roscoe A. Bartlett   2008-11-18 05:23:16 +0000   5) /* define if new form of std::count is supported */
c642d67612f packages/tpetra/cmake/Tpetra_config.h.in          (Roscoe A. Bartlett   2008-11-24 03:44:00 +0000   6) #cmakedefine HAVE_STD_NEW_COUNT_SYNTAX

My guess is that Ross added this macro for a legitimate purpose (likely relating to a particular Standard Template Library implementation) long ago. Someone else must have removed the CMake option that would have enabled this code, once the option was no longer useful. However, whoever did that never changed TpetraCore_config.h.in to remove the macro corresponding to that CMake option.

The Amesos2 file may have been a copy-and-paste directly from Tpetra. It never caused a build error, since the option must never have been enabled as of first introduction of Amesos2 into Trilinos, in 2011. git blame shows that this part of the Amesos2 file has not changed since Eric Bavier checked in it, in 2010. (The fact that Eric changed the surrounding lines of the file in 2011 supports the copy-and-paste theory.)

All of this strongly suggests that we can safely remove the macro.

mhoemmen added a commit that referenced this issue Apr 20, 2019
Amesos2,Tpetra: Fix #3752 (remove HAVE_STD_NEW_COUNT_SYNTAX macro)
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Apr 21, 2019
…s:develop' (a6a844d).

* trilinos-develop:
  Amesos2,Tpetra: Fix trilinos#3752 (remove HAVE_STD_NEW_COUNT_SYNTAX macro)
  Tempus: Add accessor to WrapperModelEvaluator
  Tpetra: Fix trilinos#4962
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Apr 22, 2019
…s:develop' (a6a844d).

* trilinos-develop:
  Automatic snapshot commit from tribits at 3b02ce8
  Amesos2,Tpetra: Fix trilinos#3752 (remove HAVE_STD_NEW_COUNT_SYNTAX macro)
  Tempus: Add accessor to WrapperModelEvaluator
  Tpetra: Fix trilinos#4962
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Apr 23, 2019
…s:develop' (a6a844d).

* trilinos-develop:
  MueLu: fix compiler warning
  MueLu Maxwell test: Allow running without M0inv
  MueLu: split files to reduce library size
  MAPVAR: Fix bad format statement
  MueLu: Fix Maxwell test
  MueLu Maxwell test: Add flags for Belos solver selection and preconditioner use
  MueLu Maxwell test: remove nnodes and nedges parameters
  MueLu: Fixing dump issue identified by Ed Love
  Fix some typos (ATDV-155, trilinos#4977)
  Reduce number of parallel build processes for CUDA+RDC+PT (ATDV-155)
  Allow proper overriding of parallel build levels and document them
  Automatic snapshot commit from tribits at 3b02ce8
  Amesos2,Tpetra: Fix trilinos#3752 (remove HAVE_STD_NEW_COUNT_SYNTAX macro)
  Tempus: Add accessor to WrapperModelEvaluator
  Tempus: Tempus Output After Passing Output Time
  Tpetra: Fix trilinos#4962
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Apr 24, 2019
…s:develop' (a6a844d).

* trilinos-develop:
  Amesos2: Make ifdef / endif pairs more clear
  Amesos2: Fix trilinos#4992
  Automatic snapshot commit from tribits at 1d1334b
  MueLu: Fixing Helmholtz bug
  MueLu: fix compiler warning
  MueLu Maxwell test: Allow running without M0inv
  MueLu: split files to reduce library size
  MAPVAR: Fix bad format statement
  MueLu: Fix Maxwell test
  MueLu Maxwell test: Add flags for Belos solver selection and preconditioner use
  MueLu Maxwell test: remove nnodes and nedges parameters
  MueLu: Fixing dump issue identified by Ed Love
  Fix some typos (ATDV-155, trilinos#4977)
  Reduce number of parallel build processes for CUDA+RDC+PT (ATDV-155)
  Allow proper overriding of parallel build levels and document them
  Automatic snapshot commit from tribits at 3b02ce8
  Amesos2,Tpetra: Fix trilinos#3752 (remove HAVE_STD_NEW_COUNT_SYNTAX macro)
  Tempus: Add accessor to WrapperModelEvaluator
  Tempus: Tempus Output After Passing Output Time
  Tpetra: Fix trilinos#4962
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Apr 24, 2019
…s:develop' (a6a844d).

* trilinos-develop:
  Amesos2: Make ifdef / endif pairs more clear
  Amesos2: Fix trilinos#4992
  Automatic snapshot commit from tribits at 1d1334b
  MueLu: Fixing Helmholtz bug
  MueLu: fix compiler warning
  MueLu Maxwell test: Allow running without M0inv
  MueLu: split files to reduce library size
  MAPVAR: Fix bad format statement
  MueLu: Fix Maxwell test
  MueLu Maxwell test: Add flags for Belos solver selection and preconditioner use
  MueLu Maxwell test: remove nnodes and nedges parameters
  MueLu: Fixing dump issue identified by Ed Love
  Fix some typos (ATDV-155, trilinos#4977)
  Reduce number of parallel build processes for CUDA+RDC+PT (ATDV-155)
  Allow proper overriding of parallel build levels and document them
  Automatic snapshot commit from tribits at 3b02ce8
  Amesos2,Tpetra: Fix trilinos#3752 (remove HAVE_STD_NEW_COUNT_SYNTAX macro)
  Tempus: Add accessor to WrapperModelEvaluator
  Tempus: Tempus Output After Passing Output Time
  Tpetra: Fix trilinos#4962
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant