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

fix: Standardisation of incorrect sub-group creation errors #3418

Open
wants to merge 34 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
e712814
🐛 bugfix: Proper error when detecting child that is not a xml elements
MelReyCG Oct 24, 2024
afd945e
💄 small error edit for uniformisation
MelReyCG Oct 24, 2024
d6ea052
💡 Added note on all empty createChild()
MelReyCG Oct 24, 2024
e4c1c41
✏️ typo fix
MelReyCG Oct 25, 2024
814a2b3
Re rewrite of ObjectCatalog errors for bad children :
MelReyCG Oct 25, 2024
2449139
added ability to get context from a cpp source file (typically for te…
MelReyCG Oct 25, 2024
eee2c11
Updated ElementRegionManager error message + refactored how ElementRe…
MelReyCG Oct 25, 2024
bbe8d7f
Updated WellSolverBase error message
MelReyCG Oct 25, 2024
0d26bb2
🐛 bugfix
MelReyCG Oct 25, 2024
549146f
⚰️ wrong & dead code removal
MelReyCG Oct 30, 2024
311c867
📝 object catalog factory docs update
MelReyCG Oct 30, 2024
aa63944
Prefering more contextualized catalog error over custom ones
MelReyCG Oct 30, 2024
9b4f59e
Updated WellGeneratorBase error message
MelReyCG Oct 30, 2024
1f26cd4
standardize group name processing
MelReyCG Nov 7, 2024
d39dde6
regex validation fail message improvement
MelReyCG Nov 7, 2024
099c536
bugfix
MelReyCG Nov 7, 2024
8c779df
Merge remote-tracking branch 'origin/develop' into bugfix/rey/bad-chi…
MelReyCG Nov 7, 2024
263ee4c
uncrustify
MelReyCG Nov 8, 2024
b330a2f
📝 Group::processInputName docs
MelReyCG Nov 8, 2024
d7f3345
📝 doc add & updates
MelReyCG Nov 8, 2024
ab1aea4
Merge branch 'develop' into bugfix/rey/bad-child-error
MelReyCG Nov 8, 2024
20f3620
📝 doc update
MelReyCG Nov 8, 2024
dd5d83d
uncrustify
MelReyCG Nov 8, 2024
8444dc7
Merge branch 'develop' into bugfix/rey/bad-child-error
MelReyCG Nov 12, 2024
27b4253
Merge branch 'develop' into bugfix/rey/bad-child-error
MelReyCG Nov 14, 2024
0358a96
Merge remote-tracking branch 'origin/develop' into bugfix/rey/bad-chi…
MelReyCG Dec 13, 2024
1693985
Merge remote-tracking branch 'origin/develop' into bugfix/rey/bad-chi…
MelReyCG Dec 18, 2024
1ad796f
properly finishing merge with develop
MelReyCG Dec 18, 2024
c3c1499
Merge branch 'develop' into bugfix/rey/bad-child-error
MelReyCG Jan 6, 2025
cfeeb44
std::string -> string
MelReyCG Jan 6, 2025
1f36c84
merge fixes
MelReyCG Jan 6, 2025
7aacc51
schema file
MelReyCG Jan 6, 2025
0e2efa9
uncrustify
MelReyCG Jan 6, 2025
95659ad
🐛 oups
MelReyCG Jan 7, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions examples/ObjectCatalog/Base.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Copyright (c) 2016-2024 Lawrence Livermore National Security LLC
* Copyright (c) 2018-2024 TotalEnergies
* Copyright (c) 2018-2024 The Board of Trustees of the Leland Stanford Junior University
* Copyright (c) 2023-2024 Chevron
* Copyright (c) 2023-2024 Chevron
* Copyright (c) 2019- GEOS/GEOSX Contributors
* All rights reserved
*
Expand Down Expand Up @@ -46,7 +46,7 @@ class Parameter
class Base
{
public:
Base( int junk, double const & junk2, Parameter& pbv )
Base( int junk, double const & junk2, Parameter & pbv )
{
GEOS_LOG( "calling Base constructor with arguments (" << junk << " " << junk2 << ")" );
}
Expand All @@ -56,14 +56,20 @@ class Base
GEOS_LOG( "calling Base destructor" );
}

using CatalogInterface = dataRepository::CatalogInterface< Base, int, double const &, Parameter& >;
static CatalogInterface::CatalogType& getCatalog()
using CatalogInterface = dataRepository::CatalogInterface< Base, int, double const &, Parameter & >;
static CatalogInterface::CatalogType & getCatalog()
{
static CatalogInterface::CatalogType catalog;
return catalog;
}

virtual std::string const getName() const = 0;

static DataContext const & getDataContext()
{
static DataFileContext const context = DataFileContext( "Base Test Class", __FILE__, __LINE__ );
return context;
}
};

#endif
12 changes: 8 additions & 4 deletions examples/ObjectCatalog/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Copyright (c) 2016-2024 Lawrence Livermore National Security LLC
* Copyright (c) 2018-2024 TotalEnergies
* Copyright (c) 2018-2024 The Board of Trustees of the Leland Stanford Junior University
* Copyright (c) 2023-2024 Chevron
* Copyright (c) 2023-2024 Chevron
* Copyright (c) 2019- GEOS/GEOSX Contributors
* All rights reserved
*
Expand All @@ -31,10 +31,14 @@ int main( int argc, char *argv[] )


GEOS_LOG( "Attempting to create a Derived1 object" );
std::unique_ptr<Base> derived1 = Base::CatalogInterface::Factory( "derived1", junk, junk2, param );
std::unique_ptr< Base > derived1 = Base::CatalogInterface::factory( "derived1",
Base::getDataContext(),
junk, junk2, param );
GEOS_LOG( "Attempting to create a Derived2 object" );
std::unique_ptr<Base> derived2 = Base::CatalogInterface::Factory( "derived2", junk, junk3, param );
std::unique_ptr< Base > derived2 = Base::CatalogInterface::factory( "derived2",
Base::getDataContext(),
junk, junk3, param );

Base::CatalogInterface::catalog_cast<Derived1>( *(derived2.get()));
Base::CatalogInterface::catalog_cast< Derived1 >( *(derived2.get()));
GEOS_LOG( "EXITING MAIN" );
}
6 changes: 3 additions & 3 deletions src/coreComponents/common/format/StringUtilities.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ string join( CONTAINER const & container, S const & delim = S() )
* @return a string containing input values concatenated with a delimiter
*/
template< typename IT, typename S, typename LAMBDA >
string joinLamda( IT first, IT last, S const & delim, LAMBDA formattingFunc )
string joinLambda( IT first, IT last, S const & delim, LAMBDA formattingFunc )
{
if( first == last )
{
Expand All @@ -114,9 +114,9 @@ string joinLamda( IT first, IT last, S const & delim, LAMBDA formattingFunc )
* @return a string containing input values concatenated with a delimiter
*/
template< typename CONTAINER, typename S, typename LAMBDA >
string joinLamda( CONTAINER const & container, S const & delim, LAMBDA formattingFunc )
string joinLambda( CONTAINER const & container, S const & delim, LAMBDA formattingFunc )
{
return joinLamda( std::begin( container ), std::end( container ), delim, formattingFunc );
return joinLambda( std::begin( container ), std::end( container ), delim, formattingFunc );
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/coreComponents/constitutive/ConstitutiveBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ ConstitutiveBase::deliverClone( string const & name,
Group * const parent ) const
{
std::unique_ptr< ConstitutiveBase >
newModel = ConstitutiveBase::CatalogInterface::factory( this->getCatalogName(), name, parent );
newModel = ConstitutiveBase::CatalogInterface::factory( this->getCatalogName(), getDataContext(),
name, parent );

newModel->forWrappers( [&]( WrapperBase & wrapper )
{
Expand Down
3 changes: 2 additions & 1 deletion src/coreComponents/constitutive/ConstitutiveManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ ConstitutiveManager::~ConstitutiveManager()
Group * ConstitutiveManager::createChild( string const & childKey, string const & childName )
{
GEOS_LOG_RANK_0( GEOS_FMT( "{}: adding {} {}", getName(), childKey, childName ) );
std::unique_ptr< ConstitutiveBase > material = ConstitutiveBase::CatalogInterface::factory( childKey, childName, this );
std::unique_ptr< ConstitutiveBase > material =
ConstitutiveBase::CatalogInterface::factory( childKey, getDataContext(), childName, this );
return &registerGroup< ConstitutiveBase >( childName, std::move( material ) );
}

Expand Down
17 changes: 13 additions & 4 deletions src/coreComponents/dataRepository/DataContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace dataRepository
{


DataContext::DataContext( string const & targetName ):
DataContext::DataContext( string_view targetName ):
m_targetName( targetName )
{}

Expand All @@ -35,12 +35,12 @@ std::ostream & operator<<( std::ostream & os, DataContext const & ctx )
return os;
}

DataContext::ToStringInfo::ToStringInfo( string const & targetName, string const & filePath, size_t line ):
DataContext::ToStringInfo::ToStringInfo( string_view targetName, string_view filePath, size_t line ):
m_targetName( targetName ),
m_filePath( filePath ),
m_line( line )
{}
DataContext::ToStringInfo::ToStringInfo( string const & targetName ):
DataContext::ToStringInfo::ToStringInfo( string_view targetName ):
m_targetName( targetName )
{}

Expand Down Expand Up @@ -83,6 +83,15 @@ DataFileContext::DataFileContext( xmlWrapper::xmlNode const & targetNode,
m_offset( attPos.offset )
{}

DataFileContext::DataFileContext( string_view targetName, string_view file, size_t line ):
DataContext( targetName ),
m_typeName( "C++ Source File" ),
m_filePath( file ),
m_line( line ),
m_offsetInLine( 0 ),
m_offset( 0 )
{}

string DataFileContext::toString() const
{
if( m_line != xmlWrapper::xmlDocument::npos )
Expand All @@ -95,7 +104,7 @@ string DataFileContext::toString() const
}
else
{
return GEOS_FMT( "{} (Source file not found)", m_targetName );
return GEOS_FMT( "{} ({})", m_targetName, ( m_filePath.empty() ? "Source file not found" : m_filePath ) );
}
}

Expand Down
14 changes: 11 additions & 3 deletions src/coreComponents/dataRepository/DataContext.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class DataContext
* @brief Construct a new DataContext object.
* @param targetName the target object name
*/
DataContext( string const & targetName );
DataContext( string_view targetName );

/**
* @brief Destroy the DataContext object
Expand Down Expand Up @@ -95,12 +95,12 @@ class DataContext
* @param filePath the input file path where the target is declared.
* @param line the line in the file where the target is declared.
*/
ToStringInfo( string const & targetName, string const & filePath, size_t line );
ToStringInfo( string_view targetName, string_view filePath, size_t line );
/**
* @brief Construct a new ToStringInfo object from a DataContext that has no input file info.
* @param targetName the target name.
*/
ToStringInfo( string const & targetName );
ToStringInfo( string_view targetName );
/**
* @return true if a location has been found to declare the target in an input file.
*/
Expand Down Expand Up @@ -142,6 +142,14 @@ class DataFileContext final : public DataContext
DataFileContext( xmlWrapper::xmlNode const & targetNode, xmlWrapper::xmlAttribute const & att,
xmlWrapper::xmlAttributePos const & attPos );

/**
* @brief Constructs the file context of a Group from a C++ source file.
* @param targetName The name of the target Group.
* @param file The name of the source file.
* @param line The line number in the source file.
*/
DataFileContext( string_view targetName, string_view file, size_t line );

/**
* @return the target object name followed by the the file and line declaring it.
*/
Expand Down
103 changes: 57 additions & 46 deletions src/coreComponents/dataRepository/Group.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,18 +135,64 @@ string Group::getPath() const
return noProblem.empty() ? "/" : noProblem;
}

string Group::processInputName( xmlWrapper::xmlNode const & targetNode,
xmlWrapper::xmlNodePos const & targetNodePos,
string_view parentNodeName,
xmlWrapper::xmlNodePos const & parentNodePos,
std::set< string > & siblingNames )
{
GEOS_THROW_IF( targetNode.type() != xmlWrapper::xmlNodeType::node_element,
GEOS_FMT( "Error in node named \"{}\" ({}): GEOS XML nodes cannot contain "
"text data nor anything but XML nodes.\nErroneous content: \"{}\"\n",
parentNodeName, parentNodePos.toString(),
stringutilities::trimSpaces( targetNode.value() ) ),
InputError );

string targetNodeName;
try
{ // read & validate the name attribute
xmlWrapper::readAttributeAsType( targetNodeName, "name",
rtTypes::getTypeRegex< string >( rtTypes::CustomTypes::groupName ),
targetNode, string( "" ) );
} catch( std::exception const & ex )
{
xmlWrapper::processInputException( ex, "name", targetNode, targetNodePos );
}

if( targetNodeName.empty() )
{ // if there is no name attribute, we use the node tag as a node name
targetNodeName = targetNode.name();
}
else
{ // Make sure names are not duplicated by checking all previous siblings
GEOS_THROW_IF( siblingNames.count( targetNodeName ) != 0,
GEOS_FMT( "Error at node named \"{}\" ({}): "
"An XML block cannot contain children with duplicated names.\n",
targetNodeName, targetNodePos.toString() ),
InputError );
siblingNames.insert( targetNodeName );
}

return targetNodeName;
}

void Group::processInputFileRecursive( xmlWrapper::xmlDocument & xmlDocument,
xmlWrapper::xmlNode & targetNode )
{
xmlWrapper::xmlNodePos nodePos = xmlDocument.getNodePosition( targetNode );
processInputFileRecursive( xmlDocument, targetNode, nodePos );
xmlWrapper::xmlNodePos targetNodePos = xmlDocument.getNodePosition( targetNode );
processInputFileRecursive( xmlDocument, targetNode, targetNodePos );
}
void Group::processInputFileRecursive( xmlWrapper::xmlDocument & xmlDocument,
xmlWrapper::xmlNode & targetNode,
xmlWrapper::xmlNodePos const & nodePos )
xmlWrapper::xmlNodePos const & targetNodePos )
{
xmlDocument.addIncludedXML( targetNode );

if( targetNodePos.isFound() )
{
m_dataContext = std::make_unique< DataFileContext >( targetNode, targetNodePos );
}

// Handle the case where the node was imported from a different input file
// Set the path prefix to make sure all relative Path variables are interpreted correctly
string const oldPrefix = std::string( Path::getPathPrefix() );
Expand All @@ -157,40 +203,12 @@ void Group::processInputFileRecursive( xmlWrapper::xmlDocument & xmlDocument,
}

// Loop over the child nodes of the targetNode
array1d< string > childNames;
std::set< string > childNames;
for( xmlWrapper::xmlNode childNode : targetNode.children() )
{
xmlWrapper::xmlNodePos childNodePos = xmlDocument.getNodePosition( childNode );

// Get the child tag and name
string childName;

try
{
xmlWrapper::readAttributeAsType( childName, "name",
rtTypes::getTypeRegex< string >( rtTypes::CustomTypes::groupName ),
childNode, string( "" ) );
} catch( std::exception const & ex )
{
xmlWrapper::processInputException( ex, "name", childNode, childNodePos );
}

if( childName.empty() )
{
childName = childNode.name();
}
else
{
// Make sure child names are not duplicated
GEOS_ERROR_IF( std::find( childNames.begin(), childNames.end(), childName ) != childNames.end(),
GEOS_FMT( "Error: An XML block cannot contain children with duplicated names.\n"
"Error detected at node {} with name = {} ({}:l.{})",
childNode.path(), childName, xmlDocument.getFilePath(),
xmlDocument.getNodePosition( childNode ).line ) );

childNames.emplace_back( childName );
}

string const childName = processInputName( childNode, childNodePos,
getName(), targetNodePos, childNames );
// Create children
Group * newChild = createChild( childNode.name(), childName );
if( newChild == nullptr )
Expand All @@ -203,7 +221,7 @@ void Group::processInputFileRecursive( xmlWrapper::xmlDocument & xmlDocument,
}
}

processInputFile( targetNode, nodePos );
processInputFile( targetNode, targetNodePos );

// Restore original prefix once the node is processed
Path::setPathPrefix( oldPrefix );
Expand All @@ -212,12 +230,6 @@ void Group::processInputFileRecursive( xmlWrapper::xmlDocument & xmlDocument,
void Group::processInputFile( xmlWrapper::xmlNode const & targetNode,
xmlWrapper::xmlNodePos const & nodePos )
{
if( nodePos.isFound() )
{
m_dataContext = std::make_unique< DataFileContext >( targetNode, nodePos );
}


std::set< string > processedAttributes;
for( std::pair< string const, WrapperBase * > & pair : m_wrappers )
{
Expand All @@ -233,10 +245,10 @@ void Group::processInputFile( xmlWrapper::xmlNode const & targetNode,
if( !xmlWrapper::isFileMetadataAttribute( attributeName ) )
{
GEOS_THROW_IF( processedAttributes.count( attributeName ) == 0,
GEOS_FMT( "XML Node at '{}' with name={} contains unused attribute '{}'.\n"
GEOS_FMT( "Error in {}: XML Node at '{}' contains unused attribute '{}'.\n"
"Valid attributes are:\n{}\nFor more details, please refer to documentation at:\n"
"http://geosx-geosx.readthedocs-hosted.com/en/latest/docs/sphinx/userGuide/Index.html",
targetNode.path(), m_dataContext->toString(), attributeName,
getDataContext(), targetNode.path(), attributeName,
dumpInputOptions() ),
InputError );
}
Expand Down Expand Up @@ -264,11 +276,10 @@ void Group::registerDataOnMeshRecursive( Group & meshBodies )

Group * Group::createChild( string const & childKey, string const & childName )
{
GEOS_ERROR_IF( !(CatalogInterface::hasKeyName( childKey )),
"KeyName ("<<childKey<<") not found in Group::Catalog" );
GEOS_LOG_RANK_0( "Adding Object " << childKey<<" named "<< childName<<" from Group::Catalog." );
return &registerGroup( childName,
CatalogInterface::factory( childKey, childName, this ) );
CatalogInterface::factory( childKey, getDataContext(),
childName, this ) );
}

void Group::printDataHierarchy( integer const indent ) const
Expand Down
Loading
Loading