Skip to content

Commit

Permalink
ObjectPath XML errors feedback (#2341)
Browse files Browse the repository at this point in the history
1. Prevents crashing when PathCollection::objectPath is wrong (thus, solving the case 2.1 of the issue #2320)

2. Adds a context in case of a wrong PackCollection or FieldSpecification objectPath so we know from which object the error comes from (thus, solving the case 2.2 of the issue #2320).
In order to output the name of the Group with a wrong objectPath, this PR propose to use the GEOSX_THROW macros and try-catch to add a context to the originally thrown exception (inserting a line starting with ***** ERROR:):

***** ERROR: initialPressure has a wrong objectPath. The following error have been thrown:
***** LOCATION: /appli_PITSI/MelvinRey/GEOSX_repos/GEOSX/src/coreComponents/mesh/MeshObjectPath.cpp:225
***** Controlling expression (should be false): !foundMatch
***** Rank 0: elementRegionsGroup doesn't have a child named badRegion.
elementRegionsGroup have the following children: reservoir, wellRegion

3. Fixes Group::getPath() so it no longer starts with m/ but with / (that m was the end of Problem which seemed wrongly cut). This fix will help a bit to output clearer path errors (this was the only use of getPath()). The returned path would be of the following form: /Tasks/myPackCollection (the leading / symbolizing it is an absolute path)

A `testGroupPath` was added to validate:
- the getPath() output,
- the getGroupByPath() input,
- if the Group hierarchy keeps being as we expect.
  • Loading branch information
MelReyCG authored Apr 25, 2023
1 parent b177414 commit f72a0da
Show file tree
Hide file tree
Showing 14 changed files with 434 additions and 114 deletions.
19 changes: 19 additions & 0 deletions src/coreComponents/codingUtilities/StringUtilities.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,25 @@ array1d< T > fromStringToArray( string const & str )
template< typename T >
string toMetricPrefixString( T const & value );

/**
* @brief Compute the length of a constant string at compile-time.
*/
// TODO c++17: this function is to remove in favor of std::string_view
constexpr size_t cstrlen( char const * const str )
{
if( str )
{
char const * ptr = str;
for(; *ptr != '\0'; ++ptr )
{}
return ptr - str;
}
else
{
return 0;
}
}

} // namespace stringutilities
} // namespace geos

Expand Down
32 changes: 32 additions & 0 deletions src/coreComponents/common/Logger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,42 @@
// Source includes
#include "Logger.hpp"
#include "Path.hpp"
#include "codingUtilities/StringUtilities.hpp"

namespace geos
{

/**
* @brief Insert an exception message in another one.
* @param originalMsg original exception message (i.e. thrown from LVARRAY_THROW or GEOSX_THROW)
* @param msgToInsert message to insert at the top of the originalMsg
*/
std::string InsertExMsg( std::string const & originalMsg, std::string const & msgToInsert )
{
std::string newMsg( originalMsg );

size_t insertPos = 0;
// for readability purposes, we try to insert the message after the "***** Rank N: " or after "***** " instead of at the top.
static auto constexpr rankLogStart = "***** Rank ";
static auto constexpr rankLogEnd = ": ";
static auto constexpr simpleLogStart = "***** ";
if( ( insertPos = newMsg.find( rankLogStart ) ) != std::string::npos )
{
insertPos = newMsg.find( rankLogEnd, insertPos + stringutilities::cstrlen( rankLogStart ) )
+ stringutilities::cstrlen( rankLogEnd );
}
else if( ( insertPos = newMsg.find_last_of( simpleLogStart ) ) != std::string::npos )
{
insertPos += stringutilities::cstrlen( simpleLogStart );
}
newMsg.insert( insertPos, msgToInsert );
return newMsg;
}

InputError::InputError( std::exception const & subException, std::string const & msgToInsert ):
std::runtime_error( InsertExMsg( subException.what(), msgToInsert ) )
{}

namespace logger
{

Expand Down
7 changes: 7 additions & 0 deletions src/coreComponents/common/Logger.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,13 @@ struct InputError : public std::runtime_error
InputError( char const * const what ):
std::runtime_error( what )
{}

/**
* @brief Construct an InputError from an underlying exception.
* @param subException An exception to base this new one on.
* @param msgToInsert The error message. It will be inserted into the one inside of subException.
*/
InputError( std::exception const & subException, std::string const & msgToInsert );
};

/**
Expand Down
50 changes: 47 additions & 3 deletions src/coreComponents/dataRepository/Group.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ string Group::getPath() const
{
// In the Conduit node heirarchy everything begins with 'Problem', we should change it so that
// the ProblemManager actually uses the root Conduit Node but that will require a full rebaseline.
string const noProblem = getConduitNode().path().substr( std::strlen( dataRepository::keys::ProblemManager ) - 1 );
string const noProblem = getConduitNode().path().substr( stringutilities::cstrlen( dataRepository::keys::ProblemManager ) );
return noProblem.empty() ? "/" : noProblem;
}

Expand Down Expand Up @@ -267,6 +267,32 @@ string Group::dumpInputOptions() const
return rval;
}

string Group::dumpSubGroupsNames() const
{
if( numSubGroups() == 0 )
{
return getName() + " has no children.";
}
else
{
return "The children of " + getName() + " are: " +
"{ " + stringutilities::join( getSubGroupsNames(), ", " ) + " }";
}
}

string Group::dumpWrappersNames() const
{
if( numWrappers() == 0 )
{
return getName() + " has no wrappers.";
}
else
{
return "The wrappers of " + getName() + " are: " +
"{ " + stringutilities::join( getWrappersNames(), ", " ) + " }";
}
}

void Group::deregisterGroup( string const & name )
{
GEOS_ERROR_IF( !hasGroup( name ), "Group " << name << " doesn't exist." );
Expand Down Expand Up @@ -610,8 +636,10 @@ Group const & Group::getBaseGroupByPath( string const & path ) const
break;
}
}
GEOS_ERROR_IF( !foundTarget,
"Could not find the specified path from the starting group." );
GEOS_THROW_IF( !foundTarget,
"Could not find the specified path start.\n"<<
"Specified path is " << path,
std::domain_error );
}

string::size_type currentPosition;
Expand Down Expand Up @@ -650,5 +678,21 @@ PyTypeObject * Group::getPythonType() const
{ return geos::python::getPyGroupType(); }
#endif

std::vector< string > Group::getSubGroupsNames() const
{
std::vector< string > childrenNames;
childrenNames.reserve( numSubGroups() );
forSubGroups( [&]( Group const & subGroup ){ childrenNames.push_back( subGroup.getName() ); } );
return childrenNames;
}

std::vector< string > Group::getWrappersNames() const
{
std::vector< string > wrappersNames;
wrappersNames.reserve( numWrappers() );
forWrappers( [&]( WrapperBase const & wrapper ){ wrappersNames.push_back( wrapper.getName() ); } );
return wrappersNames;
}

} /* end namespace dataRepository */
} /* end namespace geos */
45 changes: 41 additions & 4 deletions src/coreComponents/dataRepository/Group.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,16 @@ class Group
*/
string dumpInputOptions() const;

/**
* @brief @return a comma separated string containing all sub groups name.
*/
string dumpSubGroupsNames() const;

/**
* @brief @return a comma separated string containing all wrappers name.
*/
string dumpWrappersNames() const;

///@}

//START_SPHINX_INCLUDE_REGISTER_GROUP
Expand Down Expand Up @@ -323,7 +333,11 @@ class Group
T & getGroup( KEY const & key )
{
Group * const child = m_subGroups[ key ];
GEOS_THROW_IF( child == nullptr, "Group " << getPath() << " doesn't have a child " << key, std::domain_error );
GEOS_THROW_IF( child == nullptr,
"Group " << getPath() << " has no child named " << key << std::endl
<< dumpSubGroupsNames(),
std::domain_error );

return dynamicCast< T & >( *child );
}

Expand All @@ -334,7 +348,11 @@ class Group
T const & getGroup( KEY const & key ) const
{
Group const * const child = m_subGroups[ key ];
GEOS_THROW_IF( child == nullptr, "Group " << getPath() << " doesn't have a child " << key, std::domain_error );
GEOS_THROW_IF( child == nullptr,
"Group " << getPath() << " has no child named " << key << std::endl
<< dumpSubGroupsNames(),
std::domain_error );

return dynamicCast< T const & >( *child );
}

Expand Down Expand Up @@ -380,6 +398,11 @@ class Group
*/
localIndex numSubGroups() const { return m_subGroups.size(); }

/**
* @return An array containing all sub groups keys
*/
std::vector< string > getSubGroupsNames() const;

/**
* @brief Check whether a sub-group exists.
* @param name the name of sub-group to search for
Expand Down Expand Up @@ -1047,7 +1070,11 @@ class Group
WrapperBase const & getWrapperBase( KEY const & key ) const
{
WrapperBase const * const wrapper = m_wrappers[ key ];
GEOS_THROW_IF( wrapper == nullptr, "Group " << getPath() << " doesn't have a child " << key, std::domain_error );
GEOS_THROW_IF( wrapper == nullptr,
"Group " << getPath() << " has no wrapper named " << key << std::endl
<< dumpWrappersNames(),
std::domain_error );

return *wrapper;
}

Expand All @@ -1058,7 +1085,11 @@ class Group
WrapperBase & getWrapperBase( KEY const & key )
{
WrapperBase * const wrapper = m_wrappers[ key ];
GEOS_THROW_IF( wrapper == nullptr, "Group " << getPath() << " doesn't have a child " << key, std::domain_error );
GEOS_THROW_IF( wrapper == nullptr,
"Group " << getPath() << " has no wrapper named " << key << std::endl
<< dumpWrappersNames(),
std::domain_error );

return *wrapper;
}

Expand Down Expand Up @@ -1090,6 +1121,11 @@ class Group
indexType numWrappers() const
{ return m_wrappers.size(); }

/**
* @return An array containing all wrappers keys
*/
std::vector< string > getWrappersNames() const;

///@}

/**
Expand Down Expand Up @@ -1244,6 +1280,7 @@ class Group

/**
* @brief Return the path of this Group in the data repository.
* Starts with '/' followed by the hierarchy of the children of the "Problem" in which the Group is.
* @return The path of this group in the data repository.
*/
string getPath() const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,14 @@ FieldSpecificationBase::getCatalog()

void FieldSpecificationBase::setMeshObjectPath( Group const & meshBodies )
{
m_meshObjectPaths = std::make_unique< MeshObjectPath >( m_objectPath, meshBodies );
try
{
m_meshObjectPaths = std::make_unique< MeshObjectPath >( m_objectPath, meshBodies );
}
catch( InputError const & e )
{
throw InputError( e, getName() + " has a wrong objectPath: " + m_objectPath + "\n" );
}
}


Expand Down
Loading

0 comments on commit f72a0da

Please sign in to comment.