NodalScalarKernel not working with boundary name starting with numbers

[lingzou]

Bug Description

NodalScalarKernel (and its derived classes) accepts boundary (vector of BoundaryName) as one of its inputs. From the boundary name, the code accesses the nodelist via moose mesh API call:

nodelist = _mesh.getNodeList(_mesh.getBoundaryID(boundary_name));

in turn, which calls a function from MooseMeshUtils

BoundaryID
getBoundaryID(const BoundaryName & boundary_name, const MeshBase & mesh)
{
  BoundaryID id = Moose::INVALID_BOUNDARY_ID;
  std::istringstream ss(boundary_name);

  if (!(ss >> id))
    id = mesh.get_boundary_info().get_id_by_name(boundary_name);

  return id;
}

If, in a moose mesh, a boundary name starts with number, e.g., 123_xyz, this line if (!(ss >> id)) incorrectly detects that 123 is the boundary id, returns the incorrect id to _mesh.getNodeList, which in turn triggers the following error:

*** ERROR ***
The following error occurred in the object "mesh", of type "FileMesh".

Unable to nodeset ID: 123.

Stack frames: 19
0: 0   libmesh_opt.0.dylib                 0x00000001131a6c83 libMesh::print_trace(std::__1::basic_ostream<char, std::__1::char_traits<char> >&) + 1091
1: 1   libmoose-opt.0.dylib                0x00000001126890e3 moose::internal::mooseErrorRaw(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >) + 755
2: 2   libmoose-opt.0.dylib                0x00000001125e86ef callMooseErrorRaw(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >&, MooseApp*) + 111
3: 3   libmoose-opt.0.dylib                0x000000011178acd9 void MooseObject::mooseError<char const (&) [23], short&, char>(char const (&) [23], short&, char&&) const + 409
4: 4   libmoose-opt.0.dylib                0x000000011178aae5 MooseMesh::getNodeList(short) const + 197
5: 5   libmoose-opt.0.dylib                0x000000011186008d NodalScalarKernel::NodalScalarKernel(InputParameters const&) + 1245
6: 6   libmoose-opt.0.dylib                0x00000001118c6d95 NodalEqualValueConstraint::NodalEqualValueConstraint(InputParameters const&) + 37
7: 7   libmoose-opt.0.dylib                0x00000001118c6b45 std::__1::shared_ptr<MooseObject> Registry::build<NodalEqualValueConstraint, MooseObject>(InputParameters const&) + 69
8: 8   libmoose-opt.0.dylib                0x00000001125ec52a Factory::create(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, InputParameters const&, unsigned int, bool) + 378
9: 9   libmoose-opt.0.dylib                0x00000001121de0a4 std::__1::shared_ptr<ScalarKernelBase> Factory::create<ScalarKernelBase>(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, InputParameters const&, unsigned int) + 52
10: 10  libmoose-opt.0.dylib                0x00000001121ddf45 NonlinearSystemBase::addScalarKernel(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, InputParameters&) + 53
11: 11  libmoose-opt.0.dylib                0x0000000111d3c49a FEProblemBase::addScalarKernel(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, InputParameters&) + 602
12: 12  libmoose-opt.0.dylib                0x000000011209e62f ActionWarehouse::executeActionsWithAction(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) + 1359
13: 13  libmoose-opt.0.dylib                0x00000001120cf738 ActionWarehouse::executeAllActions() + 280
14: 14  libmoose-opt.0.dylib                0x000000011265edd2 MooseApp::runInputFile() + 98
15: 15  libsam-opt.0.dylib                  0x000000010d921785 SamApp::runInputFile() + 37
16: 16  libmoose-opt.0.dylib                0x000000011265ab09 MooseApp::run() + 905
17: 17  sam-opt                             0x000000010cbbdbec main + 124
18: 18  dyld                                0x000000011a82552e start + 462
application called MPI_Abort(MPI_COMM_WORLD, 1) - process 0
[unset]: write_line error; fd=-1 buf=:cmd=abort exitcode=1
:
system msg for write_line failure : Bad file descriptor

Steps to Reproduce

The zip file includes an input file and a mesh file to reproduce the error.
2d.zip

(Note that this might not be the best test case, since NodalEqualValueConstraint does not really work with boundaries, but it is enough to trigger the error as described above. FYI, if the boundary bug is fixed, this input will still error out because NodalEqualValueConstraint wants to see two nodes).

Impact

This bug can be avoided by not using boundary names starting with numbers.

[lingzou]

@dogrady2 FYI

[GiudGiud]

Thanks for reporting the problem!

@lingzou are you working on a fix?

If not,
@lindsayad I think this should be classified in the NEAMS projects for TA

[lingzou]

@GiudGiud No, I am not working a fix. I should be able to fix the getBoundaryID function, but feel like it may need someone who is more experienced with lower-level MOOSE APIs to work on it.

[lindsayad]

@roystgnr I read over https://en.cppreference.com/w/cpp/locale/num_get/get and frankly don't understand why a fail bit is not set when parsing "123_xyz". Do you understand why no fail bit is set?

A new check we could implement is https://stackoverflow.com/a/8889045 but what sucks is that we are then iterating over the string twice if the check passes

[roystgnr]

We're using istream::operator<<'? Then I'd expect no fail bit to be set just as if we were parsing "123,xyz" - operator<<` greedily grabs as many characters as it can convert, but if it sees more characters it can't convert then it leaves them on the stream for another type conversion later.

I'm not actually sure why I can't find an "official" C++ method to do this check ... even std::stoi, which throws an exception if it can't make a conversion, seems to have a parameter devoted specifically to "convert the beginning of the string and then tell me how much is left that you couldn't convert" behavior.

[roystgnr]

I wouldn't stress about iterating over short strings twice, though. It'll already be in L1 cache for the first iteration and at that point futzing around more with it is cheap.