sandialabs / Albany

Sandia National Laboratories' Albany multiphysics code
Other
282 stars 89 forks source link

Failing builds due to ParameterList routine call #1074

Closed ikalash closed 2 months ago

ikalash commented 2 months ago

We are getting an error due to an alleged erroneous call to a PL routine:

/.../repos/Albany/src/Albany_StateManager.cpp(65): error: no instance of overloaded function "Teuchos::ParameterList::set" matches the argument list

Curiously, this is showing up right now in the Weaver build only on the CDash (https://sems-cdash-son.sandia.gov/cdash/viewBuildError.php?buildid=204332) but also in both my spack nightly builds, which are non-GPU.

Could someone please look into this? Did some PL routines get deprecated?

bartgol commented 2 months ago

Looking at Teuchos git history, I noticed they recently changed something in ParameterList, with this PR. They messed a bit with the overloads of set, and I think something is now not working if the template argument is const. The following reproducer does indeed not compile.

void foo (const std::string& s, Teuchos::ParameterList& p)
{
  p.set<const std::string>("str",s);
}

int main (int    /* argc */,
          char** /* argv */)
{
  std::string s = "hello";
  Teuchos::ParameterList p;
  foo (s,p);
  return 0;
}

The error message I get is somewhat more verbose than what we see on cdash:

/storage/workdir/quick_test/main.cpp:16:35: error: no matching function for call to 'Teuchos::ParameterList::set<const string>(const char [4], const string&)'
   16 |   p.set<const std::string>("str",s);
      |                                   ^
In file included from /storage/workdir/quick_test/main.cpp:10:
/storage/workdir/trilinos/trilinos-install/groppello/gcc/opt/develop-serial/include/Teuchos_ParameterList.hpp:934:16: note: candidate: 'Teuchos::ParameterList& Teuchos::ParameterList::set(const string&, T&&, const string&, const Teuchos::RCP<const Teuchos::ParameterEntryValidator>&) [with T = const std::__cxx11::basic_string<char>; std::string = std::__cxx11::basic_string<char>]' (near match)
  934 | ParameterList& ParameterList::set(
      |                ^~~~~~~~~~~~~
/storage/workdir/trilinos/trilinos-install/groppello/gcc/opt/develop-serial/include/Teuchos_ParameterList.hpp:934:16: note:   conversion of argument 2 would be ill-formed:

/storage/workdir/quick_test/main.cpp:16:34: error: cannot bind rvalue reference of type 'const std::__cxx11::basic_string<char>&&' to lvalue of type 'const string' {aka 'const std::__cxx11::basic_string<char>'}
   16 |   p.set<const std::string>("str",s);

Imho, it makes little sense to specify a const template argument. I'm assuming the goal was to prevent downstream code from changing the value. But if the ParameterList is passed around as const, it will already be not allowed to change the value. Also, since the template arg should be already deduced from the input string, I don't think we need to specify the template arg at all...

I'm not sure if they indeed wanted to prevent ppl from using const T as a template argument, but I don't really care: we can simply rm the const (or the template arg altogether).