From 593046d9375e2c72aa3e22d288544f80681e2318 Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Wed, 15 Mar 2023 11:46:17 +0100 Subject: [PATCH 01/24] No more crash when PackCollection::objectPath is wrong (solving Issue 2320.2.1) --- .../timeHistory/HistoryCollectionBase.cpp | 180 ++++++++++-------- 1 file changed, 102 insertions(+), 78 deletions(-) diff --git a/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp b/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp index 10929f4317a..d290e5b9371 100644 --- a/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp +++ b/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp @@ -84,108 +84,132 @@ bool HistoryCollectionBase::execute( real64 const time_n, dataRepository::Group const * HistoryCollectionBase::getTargetObject( DomainPartition const & domain, string const & objectPath ) const { - // absolute objectPaths can be used to target anything in the data repo that is packable - if( objectPath[0] == '/' ) + try { - return &Group::getGroupByPath( objectPath ); - } - else // relative objectPaths use relative lookup identical to fieldSpecification to make xml input spec easier - { - string_array targetTokens = stringutilities::tokenize( objectPath, "/" ); - localIndex targetTokenLength = LvArray::integerConversion< localIndex >( targetTokens.size() ); - - dataRepository::Group const * targetGroup = nullptr; - int const numMeshBodies = domain.getMeshBodies().numSubGroups(); - - - if( numMeshBodies==1 ) + // absolute objectPaths can be used to target anything in the data repo that is packable + if( objectPath[0] == '/' ) { - string const singleMeshBodyName = domain.getMeshBody( 0 ).getName(); - if( targetTokens[0] != singleMeshBodyName ) - { - ++targetTokenLength; - targetTokens.insert( 0, &singleMeshBodyName, (&singleMeshBodyName)+1 ); - } + return &Group::getGroupByPath( objectPath ); } - else + else // relative objectPaths use relative lookup identical to fieldSpecification to make xml input spec easier { - bool bodyFound = false; - domain.forMeshBodies( [&]( MeshBody const & meshBody ) - { - if( meshBody.getName()==targetTokens[0] ) - { - bodyFound=true; - } - } ); - - GEOSX_ERROR_IF( !bodyFound, - GEOSX_FMT( "MeshBody ({}) is specified, but not found.", - targetTokens[0] ) ); - } + string_array targetTokens = stringutilities::tokenize( objectPath, "/" ); + localIndex targetTokenLength = LvArray::integerConversion< localIndex >( targetTokens.size() ); + dataRepository::Group const * targetGroup = nullptr; + int const numMeshBodies = domain.getMeshBodies().numSubGroups(); - string const meshBodyName = targetTokens[0]; - MeshBody const & meshBody = domain.getMeshBody( meshBodyName ); - - // set mesh level in path - localIndex const numMeshLevels = meshBody.getMeshLevels().numSubGroups(); - if( numMeshLevels==1 ) - { - string const singleMeshLevelName = meshBody.getMeshLevels().getGroup< MeshLevel >( 0 ).getName(); - if( targetTokens[1] != singleMeshLevelName ) + if( numMeshBodies==1 ) { - ++targetTokenLength; - targetTokens.insert( 1, &singleMeshLevelName, (&singleMeshLevelName)+1 ); + string const singleMeshBodyName = domain.getMeshBody( 0 ).getName(); + if( targetTokens[0] != singleMeshBodyName ) + { + ++targetTokenLength; + targetTokens.insert( 0, &singleMeshBodyName, (&singleMeshBodyName)+1 ); + } } else { - bool levelFound = false; - meshBody.forMeshLevels( [&]( MeshLevel const & meshLevel ) + bool bodyFound = false; + domain.forMeshBodies( [&]( MeshBody const & meshBody ) { - if( meshLevel.getName()==targetTokens[1] ) + if( meshBody.getName()==targetTokens[0] ) { - levelFound=true; + bodyFound=true; } } ); - GEOSX_ERROR_IF( !levelFound, - GEOSX_FMT( "MeshLevel ({}) is specified, but not found.", - targetTokens[1] ) ); + GEOSX_ERROR_IF( !bodyFound, + GEOSX_FMT( "MeshBody ({}) is specified, but not found.", + targetTokens[0] ) ); } - } - else if( !meshBody.getMeshLevels().hasGroup< MeshLevel >( targetTokens[1] ) ) - { - //GEOSX_LOG_RANK_0( "In TimeHistoryCollection.hpp, Mesh Level Discretization not specified, " - // "using baseDiscretizationString()." ); - string const baseMeshLevelName = MeshBody::groupStructKeys::baseDiscretizationString(); - ++targetTokenLength; - targetTokens.insert( 1, &baseMeshLevelName, (&baseMeshLevelName)+1 ); - } - string meshLevelName = targetTokens[1]; - MeshLevel const & meshLevel = meshBody.getMeshLevel( meshLevelName ); - targetGroup = &meshLevel; + string const meshBodyName = targetTokens[0]; + MeshBody const & meshBody = domain.getMeshBody( meshBodyName ); - if( targetTokens[2]== MeshLevel::groupStructKeys::elemManagerString() ) - { - ElementRegionManager const & elemRegionManager = meshLevel.getElemManager(); - string const elemRegionName = targetTokens[3]; - ElementRegionBase const & elemRegion = elemRegionManager.getRegion( elemRegionName ); - string const elemSubRegionName = targetTokens[4]; - ElementSubRegionBase const & elemSubRegion = elemRegion.getSubRegion( elemSubRegionName ); - targetGroup = &elemSubRegion; - } - else - { - for( localIndex pathLevel = 2; pathLevel < targetTokenLength; ++pathLevel ) + // set mesh level in path + localIndex const numMeshLevels = meshBody.getMeshLevels().numSubGroups(); + if( numMeshLevels==1 ) { - targetGroup = targetGroup->getGroupPointer( targetTokens[pathLevel] ); + string const singleMeshLevelName = meshBody.getMeshLevels().getGroup< MeshLevel >( 0 ).getName(); + if( targetTokens[1] != singleMeshLevelName ) + { + ++targetTokenLength; + targetTokens.insert( 1, &singleMeshLevelName, (&singleMeshLevelName)+1 ); + } + else + { + bool levelFound = false; + meshBody.forMeshLevels( [&]( MeshLevel const & meshLevel ) + { + if( meshLevel.getName()==targetTokens[1] ) + { + levelFound=true; + } + } ); + + GEOSX_ERROR_IF( !levelFound, + GEOSX_FMT( "MeshLevel ({}) is specified, but not found.", + targetTokens[1] ) ); + } + } + else if( !meshBody.getMeshLevels().hasGroup< MeshLevel >( targetTokens[1] ) ) + { + //GEOSX_LOG_RANK_0( "In TimeHistoryCollection.hpp, Mesh Level Discretization not specified, " + // "using baseDiscretizationString()." ); + + string const baseMeshLevelName = MeshBody::groupStructKeys::baseDiscretizationString(); + ++targetTokenLength; + targetTokens.insert( 1, &baseMeshLevelName, (&baseMeshLevelName)+1 ); + } + + string meshLevelName = targetTokens[1]; + MeshLevel const & meshLevel = meshBody.getMeshLevel( meshLevelName ); + targetGroup = &meshLevel; + + + if( targetTokens[2]== MeshLevel::groupStructKeys::elemManagerString() ) + { + ElementRegionManager const & elemRegionManager = meshLevel.getElemManager(); + string const elemRegionName = targetTokens[3]; + ElementRegionBase const & elemRegion = elemRegionManager.getRegion( elemRegionName ); + string const elemSubRegionName = targetTokens[4]; + ElementSubRegionBase const & elemSubRegion = elemRegion.getSubRegion( elemSubRegionName ); + targetGroup = &elemSubRegion; + } + else + { + for( localIndex pathLevel = 2; pathLevel < targetTokenLength; ++pathLevel ) + { + dataRepository::Group const * const childGroup = targetGroup->getGroupPointer( targetTokens[pathLevel] ); + if( childGroup != nullptr ) + { + targetGroup=childGroup; + } + else + { + GEOSX_THROW( targetTokens[pathLevel] << " not found in path " << + objectPath << std::endl << + "Children available in " << + stringutilities::join( targetTokens.begin(), + targetTokens.begin()+pathLevel, + '/' ) << + ": " << targetGroup->dumpChildrenName(), + std::domain_error ); + } + } } + return targetGroup; } - return targetGroup; + } + catch( std::domain_error const & e ) + { + throw std::domain_error( "\n***** ERROR: " + getName() + + " has a wrong objectPath. The following error have been thrown:" + + e.what() ); } } From cb89711c727d61e383df82de2944fdac74073ff4 Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Wed, 15 Mar 2023 11:54:47 +0100 Subject: [PATCH 02/24] - Fixing getPath() so it starts with "/" instead of "m/" - getPath() call on the Problem node now returns "Problem" --- src/coreComponents/dataRepository/Group.cpp | 4 ++-- src/coreComponents/dataRepository/Group.hpp | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/coreComponents/dataRepository/Group.cpp b/src/coreComponents/dataRepository/Group.cpp index 8cd91a9f15c..41ad42fb6ea 100644 --- a/src/coreComponents/dataRepository/Group.cpp +++ b/src/coreComponents/dataRepository/Group.cpp @@ -128,8 +128,8 @@ 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 ); - return noProblem.empty() ? "/" : noProblem; + string const noProblem = getConduitNode().path().substr( std::strlen( dataRepository::keys::ProblemManager ) ); + return noProblem.empty() ? dataRepository::keys::ProblemManager : noProblem; } void Group::processInputFileRecursive( xmlWrapper::xmlNode & targetNode ) diff --git a/src/coreComponents/dataRepository/Group.hpp b/src/coreComponents/dataRepository/Group.hpp index 4b185e34bbd..21dbca7c23d 100644 --- a/src/coreComponents/dataRepository/Group.hpp +++ b/src/coreComponents/dataRepository/Group.hpp @@ -1244,6 +1244,7 @@ class Group /** * @brief Return the path of this Group in the data repository. + * Starts with '/' followed up by the "Problem" children in which the Group is. * @return The path of this group in the data repository. */ string getPath() const; From b734b49b01692a55ad5c1ca51c6a86d883c22f33 Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Wed, 15 Mar 2023 15:53:38 +0100 Subject: [PATCH 03/24] Made group searching errors clearer. (issue 2320 case 2.2) --- src/coreComponents/dataRepository/Group.cpp | 42 ++++++++++++++++++++- src/coreComponents/dataRepository/Group.hpp | 30 +++++++++++++-- 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/src/coreComponents/dataRepository/Group.cpp b/src/coreComponents/dataRepository/Group.cpp index 41ad42fb6ea..214738cd708 100644 --- a/src/coreComponents/dataRepository/Group.cpp +++ b/src/coreComponents/dataRepository/Group.cpp @@ -268,6 +268,42 @@ string Group::dumpInputOptions() const return rval; } +string Group::dumpChildrenName() const +{ + if( getSubGroups().size() > 0 ) + { + string str = ""; + forSubGroupsIndex( [&] ( localIndex i, const Group & subGroup ) + { + str += (i==0 ? subGroup.getName() : ", "+subGroup.getName()); + } ); + return str; + } + else + { + return "No children"; + } +} + +string Group::dumpWrappersName() const +{ + if( numWrappers() > 0 ) + { + localIndex i = 0; + string str = ""; + forWrappers( [&] ( const WrapperBase & wrapper ) + { + str += (i==0 ? wrapper.getName() : ", "+wrapper.getName()); + ++i; + } ); + return str; + } + else + { + return "No children"; + } +} + void Group::deregisterGroup( string const & name ) { GEOSX_ERROR_IF( !hasGroup( name ), "Group " << name << " doesn't exist." ); @@ -611,8 +647,10 @@ Group const & Group::getBaseGroupByPath( string const & path ) const break; } } - GEOSX_ERROR_IF( !foundTarget, - "Could not find the specified path from the starting group." ); + GEOSX_THROW_IF( !foundTarget, + "Could not find the specified path start.\n"<< + "Specified path is " << path, + std::domain_error ); } string::size_type currentPosition; diff --git a/src/coreComponents/dataRepository/Group.hpp b/src/coreComponents/dataRepository/Group.hpp index 21dbca7c23d..c55def44e51 100644 --- a/src/coreComponents/dataRepository/Group.hpp +++ b/src/coreComponents/dataRepository/Group.hpp @@ -165,6 +165,16 @@ class Group */ string dumpInputOptions() const; + /** + * @brief @return a comma separated string containing all children name. + */ + string dumpChildrenName() const; + + /** + * @brief @return a comma separated string containing all children name. + */ + string dumpWrappersName() const; + ///@} //START_SPHINX_INCLUDE_REGISTER_GROUP @@ -323,7 +333,10 @@ class Group T & getGroup( KEY const & key ) { Group * const child = m_subGroups[ key ]; - GEOSX_THROW_IF( child == nullptr, "Group " << getPath() << " doesn't have a child " << key, std::domain_error ); + GEOSX_THROW_IF( child == nullptr, + "Group " << getPath() << " doesn't have a child named " << key << std::endl << + getName() << " have the following children: " << dumpChildrenName(), + std::domain_error ); return dynamicCast< T & >( *child ); } @@ -334,7 +347,10 @@ class Group T const & getGroup( KEY const & key ) const { Group const * const child = m_subGroups[ key ]; - GEOSX_THROW_IF( child == nullptr, "Group " << getPath() << " doesn't have a child " << key, std::domain_error ); + GEOSX_THROW_IF( child == nullptr, + "Group " << getPath() << " doesn't have a child named " << key << std::endl << + getName() << " have the following children: " << dumpChildrenName(), + std::domain_error ); return dynamicCast< T const & >( *child ); } @@ -1047,7 +1063,10 @@ class Group WrapperBase const & getWrapperBase( KEY const & key ) const { WrapperBase const * const wrapper = m_wrappers[ key ]; - GEOSX_THROW_IF( wrapper == nullptr, "Group " << getPath() << " doesn't have a child " << key, std::domain_error ); + GEOSX_THROW_IF( wrapper == nullptr, + "Group " << getPath() << " doesn't have a wrapper named " << key << std::endl << + getName() << " have the following wrappers: " << dumpWrappersName(), + std::domain_error ); return *wrapper; } @@ -1058,7 +1077,10 @@ class Group WrapperBase & getWrapperBase( KEY const & key ) { WrapperBase * const wrapper = m_wrappers[ key ]; - GEOSX_THROW_IF( wrapper == nullptr, "Group " << getPath() << " doesn't have a child " << key, std::domain_error ); + GEOSX_THROW_IF( wrapper == nullptr, + "Group " << getPath() << " doesn't have a wrapper named " << key << std::endl << + getName() << " have the following wrappers: " << dumpWrappersName(), + std::domain_error ); return *wrapper; } From 8d95760700f5bf8de6dae780212c1db733e6e4ce Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Wed, 15 Mar 2023 16:49:20 +0100 Subject: [PATCH 04/24] - Improved FieldSpecification::objectPath errors clarity - added exception throwing from the MeshObjectPath constructor --- .../FieldSpecificationBase.cpp | 11 +++++- src/coreComponents/mesh/MeshObjectPath.cpp | 37 +++++++++++-------- src/coreComponents/mesh/MeshObjectPath.hpp | 1 + 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/src/coreComponents/fieldSpecification/FieldSpecificationBase.cpp b/src/coreComponents/fieldSpecification/FieldSpecificationBase.cpp index 60282490e33..4685b9bc67d 100644 --- a/src/coreComponents/fieldSpecification/FieldSpecificationBase.cpp +++ b/src/coreComponents/fieldSpecification/FieldSpecificationBase.cpp @@ -97,7 +97,16 @@ 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 std::domain_error( "\n***** ERROR: " + getName() + + " has a wrong objectPath. The following error have been thrown:" + + e.what() ); + } } diff --git a/src/coreComponents/mesh/MeshObjectPath.cpp b/src/coreComponents/mesh/MeshObjectPath.cpp index 7c3cfb3bce4..187f630f503 100644 --- a/src/coreComponents/mesh/MeshObjectPath.cpp +++ b/src/coreComponents/mesh/MeshObjectPath.cpp @@ -80,14 +80,15 @@ MeshObjectPath::fillPathTokens( string const & path, int objectIndex = findObjectIndex(); - GEOSX_ERROR_IF( objectIndex==-1, - GEOSX_FMT( "Path ({}) does not contain a valid object type. " - "Must contain one of ({},{},{},{})", + GEOSX_THROW_IF( objectIndex==-1, + GEOSX_FMT( "Path {} does not contain a valid object type. " + "It must contain one of the following: {}, {}, {}, {}", path, MeshLevel::groupStructKeys::nodeManagerString(), MeshLevel::groupStructKeys::edgeManagerString(), MeshLevel::groupStructKeys::faceManagerString(), - MeshLevel::groupStructKeys::elemManagerString() ) ); + MeshLevel::groupStructKeys::elemManagerString() ), + InputError ); // No MeshBody or MeshLevels were specified. add all of them if( objectIndex==0 ) @@ -123,11 +124,12 @@ MeshObjectPath::fillPathTokens( string const & path, existingMeshBodyAndLevel += "/n"; } ); - GEOSX_ERROR_IF( !levelNameFound, - GEOSX_FMT( "Path ({}) specifies an invalid MeshBody or MeshLevel. ", - "existing MeshBodies: MeshLevels /n", + GEOSX_THROW_IF( !levelNameFound, + GEOSX_FMT( "Path {} specifies an invalid MeshBody or MeshLevel. ", + "existing MeshBodies: {} /n", path, - existingMeshBodyAndLevel ) ); + existingMeshBodyAndLevel ), + InputError ); pathTokens.insert( pathTokens.begin()+1, unidentifiedName ); } } @@ -136,11 +138,13 @@ MeshObjectPath::fillPathTokens( string const & path, objectIndex = findObjectIndex(); size_t targetTokenLength = pathTokens.size(); - GEOSX_ERROR_IF_NE_MSG( objectIndex, 2, - "Filling of MeshBody and/or MeshLevel in path has failed. Object Index should be 2" ); + GEOSX_THROW_IF_NE_MSG( objectIndex, 2, + "Filling of MeshBody and/or MeshLevel in path has failed. Object Index should be 2", + InputError ); - GEOSX_ERROR_IF( targetTokenLength < 2, - "Filling of MeshBody and/or MeshLevel in path has failed. targetTokenLength should be greater than 2" ); + GEOSX_THROW_IF( targetTokenLength < 2, + "Filling of MeshBody and/or MeshLevel in path has failed. targetTokenLength should be greater than 2", + InputError ); // now we need to fill in any missing region/subregion specifications. @@ -212,12 +216,13 @@ void processTokenRecursive( dataRepository::Group const & parentGroup, } } - GEOSX_ERROR_IF( !foundMatch, - GEOSX_FMT( "Specified name ({0}) did not find a match with a object in group ({1}). " - "Objects that are present in ({1}) are:\n{2}", + GEOSX_THROW_IF( !foundMatch, + GEOSX_FMT( "{1} doesn't have a child named {0}.\n" + "{1} have the following children: {2}", inputEntry, parentGroup.getName(), - stringutilities::join( namesInRepository, ", " ) ) ); + stringutilities::join( namesInRepository, ", " ) ), + InputError ); } } diff --git a/src/coreComponents/mesh/MeshObjectPath.hpp b/src/coreComponents/mesh/MeshObjectPath.hpp index 543e734dafc..29a16bf35f5 100644 --- a/src/coreComponents/mesh/MeshObjectPath.hpp +++ b/src/coreComponents/mesh/MeshObjectPath.hpp @@ -62,6 +62,7 @@ class MeshObjectPath * * @param path The path string * @param meshBodies The Group that contains all MeshBody objects + * @throw InputError when the input path is wrong. */ MeshObjectPath( string const path, dataRepository::Group const & meshBodies ); From cae0f901d79adef528ca7feba9641d859f274f68 Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Thu, 16 Mar 2023 11:41:51 +0100 Subject: [PATCH 05/24] Added a test for Group path methods and data hierarchy --- .../dataRepositoryTests/CMakeLists.txt | 1 + .../dataRepositoryTests/testGroupPath.cpp | 111 ++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp diff --git a/src/coreComponents/unitTests/dataRepositoryTests/CMakeLists.txt b/src/coreComponents/unitTests/dataRepositoryTests/CMakeLists.txt index dc372d7d1ef..0947beef5c0 100644 --- a/src/coreComponents/unitTests/dataRepositoryTests/CMakeLists.txt +++ b/src/coreComponents/unitTests/dataRepositoryTests/CMakeLists.txt @@ -8,6 +8,7 @@ set( dataRepository_tests testRestartExtended.cpp testPacking.cpp testWrapperHelpers.cpp + testGroupPath.cpp ) set( dependencyList gtest ) diff --git a/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp b/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp new file mode 100644 index 00000000000..1ec8a54633c --- /dev/null +++ b/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp @@ -0,0 +1,111 @@ +/* + * ------------------------------------------------------------------------------------------------------------ + * SPDX-License-Identifier: LGPL-2.1-only + * + * Copyright (c) 2018-2020 Lawrence Livermore National Security LLC + * Copyright (c) 2018-2020 The Board of Trustees of the Leland Stanford Junior University + * Copyright (c) 2018-2020 TotalEnergies + * Copyright (c) 2019- GEOSX Contributors + * All rights reserved + * + * See top level LICENSE, COPYRIGHT, CONTRIBUTORS, NOTICE, and ACKNOWLEDGEMENTS files for details. + * ------------------------------------------------------------------------------------------------------------ + */ + +// Source includes +#include "mainInterface/ProblemManager.hpp" +#include "mainInterface/initialization.hpp" +#include "mainInterface/GeosxState.hpp" + +// TPL includes +#include +#include + +// Tests the Group::getGroup() and getPath() methods +TEST( testGroupPath, testGlobalPaths ) +{ + using namespace geosx; + using namespace dataRepository; + + char const * xmlInput = + "\n" + "\n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + ""; + + std::vector< string > const groupPaths={ + "/Mesh/mesh1", + "/domain/MeshBodies/mesh1/meshLevels/Level0/ElementRegions/elementRegionsGroup/Region2", + "/domain/Constitutive/shale", + "/Events/solverApplications", + "/NumericalMethods/FiniteElements/FE1", + "/Solvers/lagsolve", + }; + + ProblemManager & problem = getGlobalState().getProblemManager(); + problem.parseInputString( xmlInput ); + + for( string const path : groupPaths ) + { + Group const & group = problem.getGroupByPath( path ); + ASSERT_STREQ( path.c_str(), group.getPath().c_str() ); + } +} + +int main( int argc, char * * argv ) +{ + ::testing::InitGoogleTest( &argc, argv ); + + geosx::GeosxState state( geosx::basicSetup( argc, argv, false ) ); + + int const result = RUN_ALL_TESTS(); + + geosx::basicCleanup(); + + return result; +} From a1f322e8a3aada8c95e94a4b087935fae84da645 Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Thu, 16 Mar 2023 11:58:09 +0100 Subject: [PATCH 06/24] indentation fix --- .../fieldSpecification/FieldSpecificationBase.cpp | 4 ++-- .../unitTests/dataRepositoryTests/CMakeLists.txt | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreComponents/fieldSpecification/FieldSpecificationBase.cpp b/src/coreComponents/fieldSpecification/FieldSpecificationBase.cpp index 4685b9bc67d..b5fb00af79e 100644 --- a/src/coreComponents/fieldSpecification/FieldSpecificationBase.cpp +++ b/src/coreComponents/fieldSpecification/FieldSpecificationBase.cpp @@ -104,8 +104,8 @@ void FieldSpecificationBase::setMeshObjectPath( Group const & meshBodies ) catch( InputError const & e ) { throw std::domain_error( "\n***** ERROR: " + getName() + - " has a wrong objectPath. The following error have been thrown:" + - e.what() ); + " has a wrong objectPath. The following error have been thrown:" + + e.what() ); } } diff --git a/src/coreComponents/unitTests/dataRepositoryTests/CMakeLists.txt b/src/coreComponents/unitTests/dataRepositoryTests/CMakeLists.txt index 0947beef5c0..3d88175d1fb 100644 --- a/src/coreComponents/unitTests/dataRepositoryTests/CMakeLists.txt +++ b/src/coreComponents/unitTests/dataRepositoryTests/CMakeLists.txt @@ -9,7 +9,7 @@ set( dataRepository_tests testPacking.cpp testWrapperHelpers.cpp testGroupPath.cpp - ) + ) set( dependencyList gtest ) From 2eff3e3abff028ff510c5355c5c263c078362fdb Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Thu, 16 Mar 2023 14:37:16 +0100 Subject: [PATCH 07/24] testMeshObjectPath now expect exceptions instead of errors --- src/coreComponents/mesh/unitTests/testMeshObjectPath.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreComponents/mesh/unitTests/testMeshObjectPath.cpp b/src/coreComponents/mesh/unitTests/testMeshObjectPath.cpp index 89f16fff224..b66406f8b19 100644 --- a/src/coreComponents/mesh/unitTests/testMeshObjectPath.cpp +++ b/src/coreComponents/mesh/unitTests/testMeshObjectPath.cpp @@ -280,7 +280,7 @@ TEST( testMeshObjectPath, invalidMeshBody ) Group const & meshBodies = testMesh.meshBodies(); { string const path = "*/level2/ElementRegions"; - EXPECT_DEATH_IF_SUPPORTED( MeshObjectPath meshObjectPath( path, meshBodies ), ".*" ); + ASSERT_THROW( MeshObjectPath meshObjectPath( path, meshBodies ), InputError ); } } @@ -291,7 +291,7 @@ TEST( testMeshObjectPath, invalidMeshLevel ) Group const & meshBodies = testMesh.meshBodies(); { string const path = "*/*/ElementRegions/{region2}"; - EXPECT_DEATH_IF_SUPPORTED( MeshObjectPath meshObjectPath( path, meshBodies ), ".*" ); + ASSERT_THROW( MeshObjectPath meshObjectPath( path, meshBodies ), InputError ); } } @@ -301,7 +301,7 @@ TEST( testMeshObjectPath, invalidMeshRegion ) Group const & meshBodies = testMesh.meshBodies(); { string const path = "*/*/ElementRegions/*/subreg2"; - EXPECT_DEATH_IF_SUPPORTED( MeshObjectPath meshObjectPath( path, meshBodies ), ".*" ); + ASSERT_THROW( MeshObjectPath meshObjectPath( path, meshBodies ), InputError ); } } From f737cca735fe243266d75cd8fe76d3d8a45f01c0 Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Fri, 17 Mar 2023 10:35:26 +0100 Subject: [PATCH 08/24] Added some doxygen throwing documentation --- src/coreComponents/mesh/ElementRegionBase.hpp | 1 + src/coreComponents/mesh/ElementRegionManager.hpp | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/coreComponents/mesh/ElementRegionBase.hpp b/src/coreComponents/mesh/ElementRegionBase.hpp index 15b17346d6c..9974a83ba24 100644 --- a/src/coreComponents/mesh/ElementRegionBase.hpp +++ b/src/coreComponents/mesh/ElementRegionBase.hpp @@ -135,6 +135,7 @@ class ElementRegionBase : public ObjectManagerBase * @tparam KEY_TYPE The type of the key used to lookup the subregion. * @param key The key to the subregion. * @return A reference to the subregion + * @throw std::domain_error if the the requested sub-region doesn't exist. */ template< typename SUBREGIONTYPE=ElementSubRegionBase, typename KEY_TYPE=void > SUBREGIONTYPE const & getSubRegion( KEY_TYPE const & key ) const diff --git a/src/coreComponents/mesh/ElementRegionManager.hpp b/src/coreComponents/mesh/ElementRegionManager.hpp index ac2aacb3751..523286552e8 100644 --- a/src/coreComponents/mesh/ElementRegionManager.hpp +++ b/src/coreComponents/mesh/ElementRegionManager.hpp @@ -225,6 +225,7 @@ class ElementRegionManager : public ObjectManagerBase * @brief Get a element region. * @param key The key of element region, either name or number. * @return Reference to const T. + * @throw std::domain_error if the requested region doesn't exist. */ template< typename T=ElementRegionBase, typename KEY_TYPE=void > T const & getRegion( KEY_TYPE const & key ) const @@ -236,6 +237,7 @@ class ElementRegionManager : public ObjectManagerBase * @brief Get a element region. * @param key The key of the element region, either name or number. * @return Reference to T. + * @throw std::domain_error if the requested region doesn't exist. */ template< typename T=ElementRegionBase, typename KEY_TYPE=void > T & getRegion( KEY_TYPE const & key ) From 5ba1d88063ab6d551a4370b3e5f0118c788aa2a6 Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Fri, 17 Mar 2023 10:41:02 +0100 Subject: [PATCH 09/24] fix : forgot to use a reference --- .../unitTests/dataRepositoryTests/testGroupPath.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp b/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp index 1ec8a54633c..21996969084 100644 --- a/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp +++ b/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp @@ -90,7 +90,7 @@ TEST( testGroupPath, testGlobalPaths ) ProblemManager & problem = getGlobalState().getProblemManager(); problem.parseInputString( xmlInput ); - for( string const path : groupPaths ) + for( string const & path : groupPaths ) { Group const & group = problem.getGroupByPath( path ); ASSERT_STREQ( path.c_str(), group.getPath().c_str() ); From f165806b4310936e44a706b9b91f438b771a9c9e Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Thu, 23 Mar 2023 18:00:48 +0100 Subject: [PATCH 10/24] getPath() call on Problem node now returns "/" (this was the behaviour before this PR) --- src/coreComponents/dataRepository/Group.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreComponents/dataRepository/Group.cpp b/src/coreComponents/dataRepository/Group.cpp index 4843aa8bc7f..c5f0dd02a91 100644 --- a/src/coreComponents/dataRepository/Group.cpp +++ b/src/coreComponents/dataRepository/Group.cpp @@ -128,7 +128,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 ) ); - return noProblem.empty() ? dataRepository::keys::ProblemManager : noProblem; + return noProblem.empty() ? "/" : noProblem; } void Group::processInputFileRecursive( xmlWrapper::xmlNode & targetNode ) From a67ae552d58ab9fbc3e946e510600d1c0aa0ee3f Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Thu, 23 Mar 2023 18:04:12 +0100 Subject: [PATCH 11/24] shorter messages --- src/coreComponents/dataRepository/Group.hpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/coreComponents/dataRepository/Group.hpp b/src/coreComponents/dataRepository/Group.hpp index c55def44e51..e4c514dcf0b 100644 --- a/src/coreComponents/dataRepository/Group.hpp +++ b/src/coreComponents/dataRepository/Group.hpp @@ -334,8 +334,8 @@ class Group { Group * const child = m_subGroups[ key ]; GEOSX_THROW_IF( child == nullptr, - "Group " << getPath() << " doesn't have a child named " << key << std::endl << - getName() << " have the following children: " << dumpChildrenName(), + "Group " << getPath() << " has no child named " << key << std::endl << + getName() << " children are: " << dumpChildrenName(), std::domain_error ); return dynamicCast< T & >( *child ); } @@ -348,8 +348,8 @@ class Group { Group const * const child = m_subGroups[ key ]; GEOSX_THROW_IF( child == nullptr, - "Group " << getPath() << " doesn't have a child named " << key << std::endl << - getName() << " have the following children: " << dumpChildrenName(), + "Group " << getPath() << " has no child named " << key << std::endl << + getName() << " children are: " << dumpChildrenName(), std::domain_error ); return dynamicCast< T const & >( *child ); } @@ -1064,8 +1064,8 @@ class Group { WrapperBase const * const wrapper = m_wrappers[ key ]; GEOSX_THROW_IF( wrapper == nullptr, - "Group " << getPath() << " doesn't have a wrapper named " << key << std::endl << - getName() << " have the following wrappers: " << dumpWrappersName(), + "Group " << getPath() << " has no wrapper named " << key << std::endl << + getName() << " wrappers are: " << dumpWrappersName(), std::domain_error ); return *wrapper; } @@ -1078,8 +1078,8 @@ class Group { WrapperBase * const wrapper = m_wrappers[ key ]; GEOSX_THROW_IF( wrapper == nullptr, - "Group " << getPath() << " doesn't have a wrapper named " << key << std::endl << - getName() << " have the following wrappers: " << dumpWrappersName(), + "Group " << getPath() << " has no wrapper named " << key << std::endl << + getName() << " wrappers are: " << dumpWrappersName(), std::domain_error ); return *wrapper; } From 9430d8fa53599d8f94c6deb51e2593b010cd7810 Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Tue, 28 Mar 2023 14:55:18 +0200 Subject: [PATCH 12/24] code style & reformulations --- src/coreComponents/dataRepository/Group.cpp | 38 ++++--------------- src/coreComponents/dataRepository/Group.hpp | 14 +++---- .../timeHistory/HistoryCollectionBase.cpp | 2 +- .../dataRepositoryTests/testGroupPath.cpp | 2 +- 4 files changed, 17 insertions(+), 39 deletions(-) diff --git a/src/coreComponents/dataRepository/Group.cpp b/src/coreComponents/dataRepository/Group.cpp index c5f0dd02a91..718411bd73a 100644 --- a/src/coreComponents/dataRepository/Group.cpp +++ b/src/coreComponents/dataRepository/Group.cpp @@ -267,40 +267,18 @@ string Group::dumpInputOptions() const return rval; } -string Group::dumpChildrenName() const +string Group::dumpChildrenNames() const { - if( getSubGroups().size() > 0 ) - { - string str = ""; - forSubGroupsIndex( [&] ( localIndex i, const Group & subGroup ) - { - str += (i==0 ? subGroup.getName() : ", "+subGroup.getName()); - } ); - return str; - } - else - { - return "No children"; - } + std::vector< string > children; + forSubGroups( [&]( Group const & subGroup ){ children.push_back( subGroup.getName() ); } ); + return "{ " + stringutilities::join( children, ", " ) + " }"; } -string Group::dumpWrappersName() const +string Group::dumpWrappersNames() const { - if( numWrappers() > 0 ) - { - localIndex i = 0; - string str = ""; - forWrappers( [&] ( const WrapperBase & wrapper ) - { - str += (i==0 ? wrapper.getName() : ", "+wrapper.getName()); - ++i; - } ); - return str; - } - else - { - return "No children"; - } + std::vector< string > wrappers; + forWrappers( [&]( WrapperBase const & wrapper ){ wrappers.push_back( wrapper.getName() ); } ); + return "{ " + stringutilities::join( wrappers, ", " ) + " }"; } void Group::deregisterGroup( string const & name ) diff --git a/src/coreComponents/dataRepository/Group.hpp b/src/coreComponents/dataRepository/Group.hpp index e4c514dcf0b..e01d3504093 100644 --- a/src/coreComponents/dataRepository/Group.hpp +++ b/src/coreComponents/dataRepository/Group.hpp @@ -168,12 +168,12 @@ class Group /** * @brief @return a comma separated string containing all children name. */ - string dumpChildrenName() const; + string dumpChildrenNames() const; /** * @brief @return a comma separated string containing all children name. */ - string dumpWrappersName() const; + string dumpWrappersNames() const; ///@} @@ -335,7 +335,7 @@ class Group Group * const child = m_subGroups[ key ]; GEOSX_THROW_IF( child == nullptr, "Group " << getPath() << " has no child named " << key << std::endl << - getName() << " children are: " << dumpChildrenName(), + "The children of " + getName() + " are: " << dumpChildrenNames(), std::domain_error ); return dynamicCast< T & >( *child ); } @@ -349,7 +349,7 @@ class Group Group const * const child = m_subGroups[ key ]; GEOSX_THROW_IF( child == nullptr, "Group " << getPath() << " has no child named " << key << std::endl << - getName() << " children are: " << dumpChildrenName(), + "The children of " + getName() + " are: " << dumpChildrenNames(), std::domain_error ); return dynamicCast< T const & >( *child ); } @@ -1065,7 +1065,7 @@ class Group WrapperBase const * const wrapper = m_wrappers[ key ]; GEOSX_THROW_IF( wrapper == nullptr, "Group " << getPath() << " has no wrapper named " << key << std::endl << - getName() << " wrappers are: " << dumpWrappersName(), + "The wrappers of " + getName() + " are: " << dumpWrappersNames(), std::domain_error ); return *wrapper; } @@ -1079,7 +1079,7 @@ class Group WrapperBase * const wrapper = m_wrappers[ key ]; GEOSX_THROW_IF( wrapper == nullptr, "Group " << getPath() << " has no wrapper named " << key << std::endl << - getName() << " wrappers are: " << dumpWrappersName(), + "The wrappers of " + getName() + " are: " << dumpWrappersNames(), std::domain_error ); return *wrapper; } @@ -1266,7 +1266,7 @@ class Group /** * @brief Return the path of this Group in the data repository. - * Starts with '/' followed up by the "Problem" children in which the Group is. + * Starts with '/' followed up by the hierarchy from the "Problem" children in which the Group is. * @return The path of this group in the data repository. */ string getPath() const; diff --git a/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp b/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp index 1ba610a59cb..98858eb3555 100644 --- a/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp +++ b/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp @@ -197,7 +197,7 @@ dataRepository::Group const * HistoryCollectionBase::getTargetObject( DomainPart stringutilities::join( targetTokens.begin(), targetTokens.begin()+pathLevel, '/' ) << - ": " << targetGroup->dumpChildrenName(), + ": " << targetGroup->dumpChildrenNames(), std::domain_error ); } } diff --git a/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp b/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp index 21996969084..d552217f8ff 100644 --- a/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp +++ b/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp @@ -78,7 +78,7 @@ TEST( testGroupPath, testGlobalPaths ) " \n" ""; - std::vector< string > const groupPaths={ + std::vector< string > const groupPaths{ "/Mesh/mesh1", "/domain/MeshBodies/mesh1/meshLevels/Level0/ElementRegions/elementRegionsGroup/Region2", "/domain/Constitutive/shale", From 74f3c183aca8d1f22fda06e412c057dba708c5af Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Tue, 28 Mar 2023 18:07:15 +0200 Subject: [PATCH 13/24] Docs rewording --- src/coreComponents/dataRepository/Group.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreComponents/dataRepository/Group.hpp b/src/coreComponents/dataRepository/Group.hpp index e01d3504093..905aefe18b2 100644 --- a/src/coreComponents/dataRepository/Group.hpp +++ b/src/coreComponents/dataRepository/Group.hpp @@ -1266,7 +1266,7 @@ class Group /** * @brief Return the path of this Group in the data repository. - * Starts with '/' followed up by the hierarchy from the "Problem" children in which the Group is. + * 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; From 52309be1bfb90497c1181cae428c7966b30e583a Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Wed, 29 Mar 2023 15:54:09 +0200 Subject: [PATCH 14/24] - modified messages after user feedback. - Added a constructor to InputError to add text in a previously caught exception. --- src/coreComponents/common/Logger.cpp | 26 +++++++++++++++++++ src/coreComponents/common/Logger.hpp | 15 +++++++++++ src/coreComponents/dataRepository/Group.hpp | 8 +++--- .../FieldSpecificationBase.cpp | 4 +-- .../timeHistory/HistoryCollectionBase.cpp | 15 +++++------ src/coreComponents/mesh/MeshObjectPath.cpp | 8 ++++-- 6 files changed, 59 insertions(+), 17 deletions(-) diff --git a/src/coreComponents/common/Logger.cpp b/src/coreComponents/common/Logger.cpp index 8e781db5b3b..eb1f520279a 100644 --- a/src/coreComponents/common/Logger.cpp +++ b/src/coreComponents/common/Logger.cpp @@ -23,6 +23,32 @@ namespace geosx { +std::string InputError::InsertExMsg( std::string const & originalMsg, std::string const & msgToInsert ) +{ + std::string newMsg( originalMsg ); + + // we try to insert after the "***** Rank N: ", then wh try after "***** ", otherwise, we insert at the top. + size_t insertPos; + if( ( insertPos = newMsg.find( "***** Rank " ) ) != std::string::npos ) + { + insertPos += 14; + } + else + { + if( ( insertPos = newMsg.find_last_of( "***** " ) ) != std::string::npos ) + { + insertPos += 6; + } + else + { + insertPos = 0; + } + } + newMsg.insert( insertPos, msgToInsert ); + + return newMsg; +} + namespace logger { diff --git a/src/coreComponents/common/Logger.hpp b/src/coreComponents/common/Logger.hpp index d27b52b1533..65a263221bd 100644 --- a/src/coreComponents/common/Logger.hpp +++ b/src/coreComponents/common/Logger.hpp @@ -476,6 +476,21 @@ 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 ): + std::runtime_error( InsertExMsg( subException.what(), msgToInsert ) ) + {} + +private: + /** + * @brief Returns an exception message in which we insert another message. + */ + static std::string InsertExMsg( std::string const & originalMsg, std::string const & msgToInsert ); }; /** diff --git a/src/coreComponents/dataRepository/Group.hpp b/src/coreComponents/dataRepository/Group.hpp index 905aefe18b2..e2e06e379ed 100644 --- a/src/coreComponents/dataRepository/Group.hpp +++ b/src/coreComponents/dataRepository/Group.hpp @@ -335,7 +335,7 @@ class Group Group * const child = m_subGroups[ key ]; GEOSX_THROW_IF( child == nullptr, "Group " << getPath() << " has no child named " << key << std::endl << - "The children of " + getName() + " are: " << dumpChildrenNames(), + ( numSubGroups()>0 ? "The children of " + getName() + " are: " + dumpChildrenNames() : getName() + " has no children." ), std::domain_error ); return dynamicCast< T & >( *child ); } @@ -349,7 +349,7 @@ class Group Group const * const child = m_subGroups[ key ]; GEOSX_THROW_IF( child == nullptr, "Group " << getPath() << " has no child named " << key << std::endl << - "The children of " + getName() + " are: " << dumpChildrenNames(), + ( numSubGroups()>0 ? "The children of " + getName() + " are: " + dumpChildrenNames() : getName() + " has no children." ), std::domain_error ); return dynamicCast< T const & >( *child ); } @@ -1065,7 +1065,7 @@ class Group WrapperBase const * const wrapper = m_wrappers[ key ]; GEOSX_THROW_IF( wrapper == nullptr, "Group " << getPath() << " has no wrapper named " << key << std::endl << - "The wrappers of " + getName() + " are: " << dumpWrappersNames(), + ( numWrappers()>0 ? "The wrappers of " + getName() + " are: " + dumpWrappersNames() : getName() + " has no wrappers." ), std::domain_error ); return *wrapper; } @@ -1079,7 +1079,7 @@ class Group WrapperBase * const wrapper = m_wrappers[ key ]; GEOSX_THROW_IF( wrapper == nullptr, "Group " << getPath() << " has no wrapper named " << key << std::endl << - "The wrappers of " + getName() + " are: " << dumpWrappersNames(), + ( numSubGroups()>0 ? "The wrappers of " + getName() + " are: " + dumpWrappersNames() : getName() + " has no wrappers." ), std::domain_error ); return *wrapper; } diff --git a/src/coreComponents/fieldSpecification/FieldSpecificationBase.cpp b/src/coreComponents/fieldSpecification/FieldSpecificationBase.cpp index b5fb00af79e..b009e9d0fde 100644 --- a/src/coreComponents/fieldSpecification/FieldSpecificationBase.cpp +++ b/src/coreComponents/fieldSpecification/FieldSpecificationBase.cpp @@ -103,9 +103,7 @@ void FieldSpecificationBase::setMeshObjectPath( Group const & meshBodies ) } catch( InputError const & e ) { - throw std::domain_error( "\n***** ERROR: " + getName() + - " has a wrong objectPath. The following error have been thrown:" + - e.what() ); + throw InputError( e, getName() + " has a wrong objectPath: " + m_objectPath + "\n" ); } } diff --git a/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp b/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp index 98858eb3555..1679f03e361 100644 --- a/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp +++ b/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp @@ -191,13 +191,14 @@ dataRepository::Group const * HistoryCollectionBase::getTargetObject( DomainPart } else { + string targetTokensStr=stringutilities::join( targetTokens.begin(), + targetTokens.begin()+pathLevel, + '/' ); GEOSX_THROW( targetTokens[pathLevel] << " not found in path " << objectPath << std::endl << - "Children available in " << - stringutilities::join( targetTokens.begin(), - targetTokens.begin()+pathLevel, - '/' ) << - ": " << targetGroup->dumpChildrenNames(), + ( targetGroup->numSubGroups()>0 ? + "Children available in " + targetTokensStr + ": " + targetGroup->dumpChildrenNames() : + "No Children available in " + targetTokensStr ), std::domain_error ); } } @@ -207,9 +208,7 @@ dataRepository::Group const * HistoryCollectionBase::getTargetObject( DomainPart } catch( std::domain_error const & e ) { - throw std::domain_error( "\n***** ERROR: " + getName() + - " has a wrong objectPath. The following error have been thrown:" + - e.what() ); + throw InputError( e, getName() + " has a wrong objectPath: " + objectPath + "\n" ); } } diff --git a/src/coreComponents/mesh/MeshObjectPath.cpp b/src/coreComponents/mesh/MeshObjectPath.cpp index e8e154232b3..6894bfdf7ca 100644 --- a/src/coreComponents/mesh/MeshObjectPath.cpp +++ b/src/coreComponents/mesh/MeshObjectPath.cpp @@ -193,12 +193,16 @@ void processTokenRecursive( dataRepository::Group const & parentGroup, NODETYPE & node, CALLBACK && cbfunc ) { - array1d< string > namesInRepository; + std::vector< string > namesInRepository; parentGroup.forSubGroups< TYPE >( [&]( TYPE const & group ) { namesInRepository.emplace_back( group.getName() ); } ); + GEOSX_THROW_IF( namesInRepository.empty(), + GEOSX_FMT( "{1} doesn't have any children.", parentGroup.getName()), + InputError ); + for( string const & inputEntry : stringutilities::tokenize( pathToken, " " ) ) { bool foundMatch = false; @@ -218,7 +222,7 @@ void processTokenRecursive( dataRepository::Group const & parentGroup, } GEOSX_THROW_IF( !foundMatch, GEOSX_FMT( "{1} doesn't have a child named {0}.\n" - "{1} have the following children: {2}", + "{1} have the following children: {{ {2} }}", inputEntry, parentGroup.getName(), stringutilities::join( namesInRepository, ", " ) ), From d0bdbbbd0e3a0bac05b92209d65601bb7be0d5a7 Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Fri, 31 Mar 2023 14:55:56 +0200 Subject: [PATCH 15/24] - code style - GEOSX_FMT param id fix --- src/coreComponents/common/Logger.cpp | 22 +++----- src/coreComponents/dataRepository/Group.cpp | 42 +++++++++++--- src/coreComponents/dataRepository/Group.hpp | 56 ++++++++++++------- .../timeHistory/HistoryCollectionBase.cpp | 11 ++-- src/coreComponents/mesh/MeshObjectPath.cpp | 8 +-- 5 files changed, 89 insertions(+), 50 deletions(-) diff --git a/src/coreComponents/common/Logger.cpp b/src/coreComponents/common/Logger.cpp index eb1f520279a..46e67d2a54b 100644 --- a/src/coreComponents/common/Logger.cpp +++ b/src/coreComponents/common/Logger.cpp @@ -27,22 +27,18 @@ std::string InputError::InsertExMsg( std::string const & originalMsg, std::strin { std::string newMsg( originalMsg ); - // we try to insert after the "***** Rank N: ", then wh try after "***** ", otherwise, we insert at the top. - size_t insertPos; - if( ( insertPos = newMsg.find( "***** Rank " ) ) != std::string::npos ) + 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 rankLogStartStr = "***** Rank "; + static auto constexpr rankLogEndStr = ": "; + static auto constexpr simpleLogStartStr = "***** "; + if( ( insertPos = newMsg.find( rankLogStartStr ) ) != std::string::npos ) { - insertPos += 14; + insertPos = newMsg.find( rankLogEndStr, insertPos + strlen( rankLogStartStr ) ) + strlen( rankLogEndStr ); } - else + else if( ( insertPos = newMsg.find_last_of( simpleLogStartStr ) ) != std::string::npos ) { - if( ( insertPos = newMsg.find_last_of( "***** " ) ) != std::string::npos ) - { - insertPos += 6; - } - else - { - insertPos = 0; - } + insertPos += strlen( simpleLogStartStr ); } newMsg.insert( insertPos, msgToInsert ); diff --git a/src/coreComponents/dataRepository/Group.cpp b/src/coreComponents/dataRepository/Group.cpp index 718411bd73a..195f831bbc2 100644 --- a/src/coreComponents/dataRepository/Group.cpp +++ b/src/coreComponents/dataRepository/Group.cpp @@ -267,18 +267,30 @@ string Group::dumpInputOptions() const return rval; } -string Group::dumpChildrenNames() const +string Group::dumpSubGroupsNames() const { - std::vector< string > children; - forSubGroups( [&]( Group const & subGroup ){ children.push_back( subGroup.getName() ); } ); - return "{ " + stringutilities::join( children, ", " ) + " }"; + if( numSubGroups() == 0 ) + { + return getName() + " has no children."; + } + else + { + return "The children of " + getName() + " are: " + + "{ " + stringutilities::join( getSubGroupsNames(), ", " ) + " }"; + } } string Group::dumpWrappersNames() const { - std::vector< string > wrappers; - forWrappers( [&]( WrapperBase const & wrapper ){ wrappers.push_back( wrapper.getName() ); } ); - return "{ " + stringutilities::join( wrappers, ", " ) + " }"; + if( numWrappers() == 0 ) + { + return getName() + " has no wrappers."; + } + else + { + return "The wrappers of " + getName() + " are: " + + "{ " + stringutilities::join( getWrappersNames(), ", " ) + " }"; + } } void Group::deregisterGroup( string const & name ) @@ -666,5 +678,21 @@ PyTypeObject * Group::getPythonType() const { return geosx::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 geosx */ diff --git a/src/coreComponents/dataRepository/Group.hpp b/src/coreComponents/dataRepository/Group.hpp index e2e06e379ed..5b2fe6415d3 100644 --- a/src/coreComponents/dataRepository/Group.hpp +++ b/src/coreComponents/dataRepository/Group.hpp @@ -166,12 +166,12 @@ class Group string dumpInputOptions() const; /** - * @brief @return a comma separated string containing all children name. + * @brief @return a comma separated string containing all sub groups name. */ - string dumpChildrenNames() const; + string dumpSubGroupsNames() const; /** - * @brief @return a comma separated string containing all children name. + * @brief @return a comma separated string containing all wrappers name. */ string dumpWrappersNames() const; @@ -333,10 +333,12 @@ class Group T & getGroup( KEY const & key ) { Group * const child = m_subGroups[ key ]; - GEOSX_THROW_IF( child == nullptr, - "Group " << getPath() << " has no child named " << key << std::endl << - ( numSubGroups()>0 ? "The children of " + getName() + " are: " + dumpChildrenNames() : getName() + " has no children." ), - std::domain_error ); + if( child == nullptr ) + { + GEOSX_THROW( "Group " << getPath() << " has no child named " << key << std::endl << + dumpSubGroupsNames(), + std::domain_error ); + } return dynamicCast< T & >( *child ); } @@ -347,10 +349,12 @@ class Group T const & getGroup( KEY const & key ) const { Group const * const child = m_subGroups[ key ]; - GEOSX_THROW_IF( child == nullptr, - "Group " << getPath() << " has no child named " << key << std::endl << - ( numSubGroups()>0 ? "The children of " + getName() + " are: " + dumpChildrenNames() : getName() + " has no children." ), - std::domain_error ); + if( child == nullptr ) + { + GEOSX_THROW( "Group " << getPath() << " has no child named " << key << std::endl << + dumpSubGroupsNames(), + std::domain_error ); + } return dynamicCast< T const & >( *child ); } @@ -396,6 +400,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 @@ -1063,10 +1072,12 @@ class Group WrapperBase const & getWrapperBase( KEY const & key ) const { WrapperBase const * const wrapper = m_wrappers[ key ]; - GEOSX_THROW_IF( wrapper == nullptr, - "Group " << getPath() << " has no wrapper named " << key << std::endl << - ( numWrappers()>0 ? "The wrappers of " + getName() + " are: " + dumpWrappersNames() : getName() + " has no wrappers." ), - std::domain_error ); + if( wrapper == nullptr ) + { + GEOSX_THROW( "Group " << getPath() << " has no wrapper named " << key << std::endl << + dumpWrappersNames(), + std::domain_error ); + } return *wrapper; } @@ -1077,10 +1088,12 @@ class Group WrapperBase & getWrapperBase( KEY const & key ) { WrapperBase * const wrapper = m_wrappers[ key ]; - GEOSX_THROW_IF( wrapper == nullptr, - "Group " << getPath() << " has no wrapper named " << key << std::endl << - ( numSubGroups()>0 ? "The wrappers of " + getName() + " are: " + dumpWrappersNames() : getName() + " has no wrappers." ), - std::domain_error ); + if( wrapper == nullptr ) + { + GEOSX_THROW( "Group " << getPath() << " has no wrapper named " << key << std::endl << + dumpWrappersNames(), + std::domain_error ); + } return *wrapper; } @@ -1112,6 +1125,11 @@ class Group indexType numWrappers() const { return m_wrappers.size(); } + /** + * @return An array containing all wrappers keys + */ + std::vector< string > getWrappersNames() const; + ///@} /** diff --git a/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp b/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp index 1679f03e361..c7bea0dcab1 100644 --- a/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp +++ b/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp @@ -191,14 +191,11 @@ dataRepository::Group const * HistoryCollectionBase::getTargetObject( DomainPart } else { - string targetTokensStr=stringutilities::join( targetTokens.begin(), - targetTokens.begin()+pathLevel, - '/' ); + string const targetTokensStr = stringutilities::join( targetTokens.begin(), + targetTokens.begin()+pathLevel, + '/' ); GEOSX_THROW( targetTokens[pathLevel] << " not found in path " << - objectPath << std::endl << - ( targetGroup->numSubGroups()>0 ? - "Children available in " + targetTokensStr + ": " + targetGroup->dumpChildrenNames() : - "No Children available in " + targetTokensStr ), + objectPath << std::endl << targetGroup->dumpSubGroupsNames(), std::domain_error ); } } diff --git a/src/coreComponents/mesh/MeshObjectPath.cpp b/src/coreComponents/mesh/MeshObjectPath.cpp index 6894bfdf7ca..d317e9f0f3d 100644 --- a/src/coreComponents/mesh/MeshObjectPath.cpp +++ b/src/coreComponents/mesh/MeshObjectPath.cpp @@ -200,7 +200,7 @@ void processTokenRecursive( dataRepository::Group const & parentGroup, } ); GEOSX_THROW_IF( namesInRepository.empty(), - GEOSX_FMT( "{1} doesn't have any children.", parentGroup.getName()), + GEOSX_FMT( "{0} doesn't have any children.", parentGroup.getName()), InputError ); for( string const & inputEntry : stringutilities::tokenize( pathToken, " " ) ) @@ -221,10 +221,10 @@ void processTokenRecursive( dataRepository::Group const & parentGroup, } } GEOSX_THROW_IF( !foundMatch, - GEOSX_FMT( "{1} doesn't have a child named {0}.\n" - "{1} have the following children: {{ {2} }}", - inputEntry, + GEOSX_FMT( "{0} doesn't have a child named {1}.\n" + "{0} have the following children: {{ {2} }}", parentGroup.getName(), + inputEntry, stringutilities::join( namesInRepository, ", " ) ), InputError ); } From 1912e66fe3d0d863d3f84e05620a5a225006da7e Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Mon, 3 Apr 2023 16:26:41 +0200 Subject: [PATCH 16/24] variable naming --- src/coreComponents/common/Logger.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/coreComponents/common/Logger.cpp b/src/coreComponents/common/Logger.cpp index 46e67d2a54b..46b30ecb50d 100644 --- a/src/coreComponents/common/Logger.cpp +++ b/src/coreComponents/common/Logger.cpp @@ -29,16 +29,16 @@ std::string InputError::InsertExMsg( std::string const & originalMsg, std::strin 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 rankLogStartStr = "***** Rank "; - static auto constexpr rankLogEndStr = ": "; - static auto constexpr simpleLogStartStr = "***** "; - if( ( insertPos = newMsg.find( rankLogStartStr ) ) != std::string::npos ) + static auto constexpr rankLogStart = "***** Rank "; + static auto constexpr rankLogEnd = ": "; + static auto constexpr simpleLogStart = "***** "; + if( ( insertPos = newMsg.find( rankLogStart ) ) != std::string::npos ) { - insertPos = newMsg.find( rankLogEndStr, insertPos + strlen( rankLogStartStr ) ) + strlen( rankLogEndStr ); + insertPos = newMsg.find( rankLogEnd, insertPos + strlen( rankLogStart ) ) + strlen( rankLogEnd ); } - else if( ( insertPos = newMsg.find_last_of( simpleLogStartStr ) ) != std::string::npos ) + else if( ( insertPos = newMsg.find_last_of( simpleLogStart ) ) != std::string::npos ) { - insertPos += strlen( simpleLogStartStr ); + insertPos += strlen( simpleLogStart ); } newMsg.insert( insertPos, msgToInsert ); From 930dd64f45bcdcbf8ec1ae3340e4ed78c36d85c8 Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Mon, 3 Apr 2023 16:31:48 +0200 Subject: [PATCH 17/24] updating "invalid MeshBody" error message. --- src/coreComponents/mesh/MeshObjectPath.cpp | 33 ++++++++++++++-------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/coreComponents/mesh/MeshObjectPath.cpp b/src/coreComponents/mesh/MeshObjectPath.cpp index d317e9f0f3d..994a8962ec5 100644 --- a/src/coreComponents/mesh/MeshObjectPath.cpp +++ b/src/coreComponents/mesh/MeshObjectPath.cpp @@ -111,25 +111,36 @@ MeshObjectPath::fillPathTokens( string const & path, { pathTokens.insert( pathTokens.begin(), "{*}" ); - string existingMeshBodyAndLevel; + // searching if the mesh level exists bool levelNameFound = false; meshBodies.forSubGroups< MeshBody >( [&]( MeshBody const & meshBody ) { - existingMeshBodyAndLevel += meshBody.getName() + ": "; meshBody.forMeshLevels( [&]( MeshLevel const & meshLevel ) { - existingMeshBodyAndLevel += meshLevel.getName() + ", "; - levelNameFound = ( unidentifiedName==meshLevel.getName() ) ? true : levelNameFound; + levelNameFound |= ( unidentifiedName==meshLevel.getName() ); } ); - existingMeshBodyAndLevel += "/n"; } ); - GEOSX_THROW_IF( !levelNameFound, - GEOSX_FMT( "Path {} specifies an invalid MeshBody or MeshLevel. ", - "existing MeshBodies: {} /n", - path, - existingMeshBodyAndLevel ), - InputError ); + if( !levelNameFound ) + { + string existingMeshBodiesAndLevels; + meshBodies.forSubGroups< MeshBody >( [&]( MeshBody const & meshBody ) + { + std::vector< string > meshLevelsNames; + existingMeshBodiesAndLevels += " MeshBody "+meshBody.getName() + ": { "; + meshBody.forMeshLevels( [&]( MeshLevel const & meshLevel ) + { + meshLevelsNames.push_back( meshLevel.getName() ); + } ); + existingMeshBodiesAndLevels += stringutilities::join( meshLevelsNames, ", " ) + " }\n"; + } ); + + GEOSX_THROW( GEOSX_FMT( "Path {0} specifies an invalid MeshBody or MeshLevel. ", + "existing MeshBodies: \n{1}\n", + path, + existingMeshBodiesAndLevels ), + InputError ); + } pathTokens.insert( pathTokens.begin()+1, unidentifiedName ); } } From aa9a930b150fe3061662bb57f9afa539823b51f8 Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Mon, 3 Apr 2023 17:31:48 +0200 Subject: [PATCH 18/24] Test to check the exception of getGroupByPath() --- .../dataRepositoryTests/testGroupPath.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp b/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp index d552217f8ff..f75aa983728 100644 --- a/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp +++ b/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp @@ -95,6 +95,24 @@ TEST( testGroupPath, testGlobalPaths ) Group const & group = problem.getGroupByPath( path ); ASSERT_STREQ( path.c_str(), group.getPath().c_str() ); } + + // test for a wrong path given to getGroupByPath() + bool trowHappened = false; + try + { + problem.getGroupByPath( "/Mesh/mesh2" ); + } + catch( const std::domain_error & e ) + { + static constexpr auto expectedMsg = "***** Controlling expression (should be false): true\n" + "***** Rank 0: Group /Mesh has no child named mesh2\n" + "The children of Mesh are: { mesh1 }"; + // checks if the exception contains the expected message + ASSERT_TRUE( string( e.what() ).find( expectedMsg ) != string::npos ); + trowHappened = true; + } + // checks if the exception has been thrown as expected + ASSERT_TRUE( trowHappened ); } int main( int argc, char * * argv ) From 026b4338dfb7f492e62d0b9e8587b109a3a0fe22 Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Mon, 3 Apr 2023 17:42:20 +0200 Subject: [PATCH 19/24] code style --- src/coreComponents/common/Logger.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreComponents/common/Logger.hpp b/src/coreComponents/common/Logger.hpp index 65a263221bd..d4ad8bf0bf7 100644 --- a/src/coreComponents/common/Logger.hpp +++ b/src/coreComponents/common/Logger.hpp @@ -490,7 +490,7 @@ struct InputError : public std::runtime_error /** * @brief Returns an exception message in which we insert another message. */ - static std::string InsertExMsg( std::string const & originalMsg, std::string const & msgToInsert ); + std::string InsertExMsg( std::string const & originalMsg, std::string const & msgToInsert ); }; /** From 7fb5afb980dbfe39674b2f3924c054d101a63d04 Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Wed, 5 Apr 2023 17:25:39 +0200 Subject: [PATCH 20/24] - Added a stringutilities::cstrlen (= compile-time strlen) - used stringutilities::cstrlen where it could be - made InsertExMsg a free function --- .../codingUtilities/StringUtilities.hpp | 18 ++++++++++++++++++ src/coreComponents/common/Logger.cpp | 18 ++++++++++++++---- src/coreComponents/common/Logger.hpp | 10 +--------- src/coreComponents/dataRepository/Group.cpp | 2 +- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/src/coreComponents/codingUtilities/StringUtilities.hpp b/src/coreComponents/codingUtilities/StringUtilities.hpp index d8e9dacf882..089e61a6913 100644 --- a/src/coreComponents/codingUtilities/StringUtilities.hpp +++ b/src/coreComponents/codingUtilities/StringUtilities.hpp @@ -207,6 +207,24 @@ 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. + */ +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 geosx diff --git a/src/coreComponents/common/Logger.cpp b/src/coreComponents/common/Logger.cpp index 46b30ecb50d..c348f4a7546 100644 --- a/src/coreComponents/common/Logger.cpp +++ b/src/coreComponents/common/Logger.cpp @@ -19,11 +19,17 @@ // Source includes #include "Logger.hpp" #include "Path.hpp" +#include "codingUtilities/StringUtilities.hpp" namespace geosx { -std::string InputError::InsertExMsg( std::string const & originalMsg, std::string const & msgToInsert ) +/** + * @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 ); @@ -34,17 +40,21 @@ std::string InputError::InsertExMsg( std::string const & originalMsg, std::strin static auto constexpr simpleLogStart = "***** "; if( ( insertPos = newMsg.find( rankLogStart ) ) != std::string::npos ) { - insertPos = newMsg.find( rankLogEnd, insertPos + strlen( rankLogStart ) ) + strlen( rankLogEnd ); + insertPos = newMsg.find( rankLogEnd, insertPos + stringutilities::cstrlen( rankLogStart ) ) + + stringutilities::cstrlen( rankLogEnd ); } else if( ( insertPos = newMsg.find_last_of( simpleLogStart ) ) != std::string::npos ) { - insertPos += strlen( simpleLogStart ); + 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 { diff --git a/src/coreComponents/common/Logger.hpp b/src/coreComponents/common/Logger.hpp index d4ad8bf0bf7..2b236a57b52 100644 --- a/src/coreComponents/common/Logger.hpp +++ b/src/coreComponents/common/Logger.hpp @@ -482,15 +482,7 @@ struct InputError : public std::runtime_error * @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 ): - std::runtime_error( InsertExMsg( subException.what(), msgToInsert ) ) - {} - -private: - /** - * @brief Returns an exception message in which we insert another message. - */ - std::string InsertExMsg( std::string const & originalMsg, std::string const & msgToInsert ); + InputError( std::exception const & subException, std::string const & msgToInsert ); }; /** diff --git a/src/coreComponents/dataRepository/Group.cpp b/src/coreComponents/dataRepository/Group.cpp index 195f831bbc2..25b401d18a2 100644 --- a/src/coreComponents/dataRepository/Group.cpp +++ b/src/coreComponents/dataRepository/Group.cpp @@ -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 ) ); + string const noProblem = getConduitNode().path().substr( std::cstrlen( dataRepository::keys::ProblemManager ) ); return noProblem.empty() ? "/" : noProblem; } From 3fc741443a811bec0a47e9f0923129f52825a8c9 Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Thu, 6 Apr 2023 16:55:24 +0200 Subject: [PATCH 21/24] fixing a wrong namespace --- src/coreComponents/dataRepository/Group.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreComponents/dataRepository/Group.cpp b/src/coreComponents/dataRepository/Group.cpp index 25b401d18a2..2e5073fa06d 100644 --- a/src/coreComponents/dataRepository/Group.cpp +++ b/src/coreComponents/dataRepository/Group.cpp @@ -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::cstrlen( dataRepository::keys::ProblemManager ) ); + string const noProblem = getConduitNode().path().substr( stringutilities::cstrlen( dataRepository::keys::ProblemManager ) ); return noProblem.empty() ? "/" : noProblem; } From 97fd7e0a3481d4c90cb96f4a7ace3f40731e80a3 Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Thu, 6 Apr 2023 17:38:26 +0200 Subject: [PATCH 22/24] added c++17 comment --- src/coreComponents/codingUtilities/StringUtilities.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreComponents/codingUtilities/StringUtilities.hpp b/src/coreComponents/codingUtilities/StringUtilities.hpp index 089e61a6913..ffd415aa3b1 100644 --- a/src/coreComponents/codingUtilities/StringUtilities.hpp +++ b/src/coreComponents/codingUtilities/StringUtilities.hpp @@ -210,6 +210,7 @@ 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 ) From 70abf6cda2577a01af69d84a8870f6943c3631ad Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Thu, 13 Apr 2023 17:27:56 +0200 Subject: [PATCH 23/24] Code factorisation (THROW_IF) --- src/coreComponents/dataRepository/Group.hpp | 44 +++++++++---------- .../dataRepositoryTests/testGroupPath.cpp | 2 +- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/src/coreComponents/dataRepository/Group.hpp b/src/coreComponents/dataRepository/Group.hpp index 5b2fe6415d3..8413493b70b 100644 --- a/src/coreComponents/dataRepository/Group.hpp +++ b/src/coreComponents/dataRepository/Group.hpp @@ -333,12 +333,11 @@ class Group T & getGroup( KEY const & key ) { Group * const child = m_subGroups[ key ]; - if( child == nullptr ) - { - GEOSX_THROW( "Group " << getPath() << " has no child named " << key << std::endl << - dumpSubGroupsNames(), - std::domain_error ); - } + GEOSX_THROW_IF( child == nullptr, + "Group " << getPath() << " has no child named " << key << std::endl + << dumpSubGroupsNames(), + std::domain_error ); + return dynamicCast< T & >( *child ); } @@ -349,12 +348,11 @@ class Group T const & getGroup( KEY const & key ) const { Group const * const child = m_subGroups[ key ]; - if( child == nullptr ) - { - GEOSX_THROW( "Group " << getPath() << " has no child named " << key << std::endl << - dumpSubGroupsNames(), - std::domain_error ); - } + GEOSX_THROW_IF( child == nullptr, + "Group " << getPath() << " has no child named " << key << std::endl + << dumpSubGroupsNames(), + std::domain_error ); + return dynamicCast< T const & >( *child ); } @@ -1072,12 +1070,11 @@ class Group WrapperBase const & getWrapperBase( KEY const & key ) const { WrapperBase const * const wrapper = m_wrappers[ key ]; - if( wrapper == nullptr ) - { - GEOSX_THROW( "Group " << getPath() << " has no wrapper named " << key << std::endl << - dumpWrappersNames(), - std::domain_error ); - } + GEOSX_THROW_IF( wrapper == nullptr, + "Group " << getPath() << " has no wrapper named " << key << std::endl + << dumpWrappersNames(), + std::domain_error ); + return *wrapper; } @@ -1088,12 +1085,11 @@ class Group WrapperBase & getWrapperBase( KEY const & key ) { WrapperBase * const wrapper = m_wrappers[ key ]; - if( wrapper == nullptr ) - { - GEOSX_THROW( "Group " << getPath() << " has no wrapper named " << key << std::endl << - dumpWrappersNames(), - std::domain_error ); - } + GEOSX_THROW_IF( wrapper == nullptr, + "Group " << getPath() << " has no wrapper named " << key << std::endl + << dumpWrappersNames(), + std::domain_error ); + return *wrapper; } diff --git a/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp b/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp index f75aa983728..5841d46334f 100644 --- a/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp +++ b/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp @@ -104,7 +104,7 @@ TEST( testGroupPath, testGlobalPaths ) } catch( const std::domain_error & e ) { - static constexpr auto expectedMsg = "***** Controlling expression (should be false): true\n" + static constexpr auto expectedMsg = "***** Controlling expression (should be false): child == nullptr\n" "***** Rank 0: Group /Mesh has no child named mesh2\n" "The children of Mesh are: { mesh1 }"; // checks if the exception contains the expected message From f9f3c730cc99bd4f91570f0274af46a7287d9bc9 Mon Sep 17 00:00:00 2001 From: MelReyCG Date: Wed, 19 Apr 2023 17:36:26 +0200 Subject: [PATCH 24/24] GEOSX -> GEOS --- .../fileIO/timeHistory/HistoryCollectionBase.cpp | 12 ++++++------ src/coreComponents/mesh/MeshObjectPath.cpp | 6 +++--- .../unitTests/dataRepositoryTests/testGroupPath.cpp | 6 +++--- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp b/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp index ffdfc8a5adf..ba32826c9ae 100644 --- a/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp +++ b/src/coreComponents/fileIO/timeHistory/HistoryCollectionBase.cpp @@ -151,9 +151,9 @@ dataRepository::Group const * HistoryCollectionBase::getTargetObject( DomainPart } } ); - GEOSX_ERROR_IF( !levelFound, - GEOSX_FMT( "MeshLevel ({}) is specified, but not found.", - targetTokens[1] ) ); + GEOS_ERROR_IF( !levelFound, + GEOS_FMT( "MeshLevel ({}) is specified, but not found.", + targetTokens[1] ) ); } } else if( !meshBody.getMeshLevels().hasGroup< MeshLevel >( targetTokens[1] ) ) @@ -194,9 +194,9 @@ dataRepository::Group const * HistoryCollectionBase::getTargetObject( DomainPart string const targetTokensStr = stringutilities::join( targetTokens.begin(), targetTokens.begin()+pathLevel, '/' ); - GEOSX_THROW( targetTokens[pathLevel] << " not found in path " << - objectPath << std::endl << targetGroup->dumpSubGroupsNames(), - std::domain_error ); + GEOS_THROW( targetTokens[pathLevel] << " not found in path " << + objectPath << std::endl << targetGroup->dumpSubGroupsNames(), + std::domain_error ); } } } diff --git a/src/coreComponents/mesh/MeshObjectPath.cpp b/src/coreComponents/mesh/MeshObjectPath.cpp index c7027219f6c..6cdaf6bfe98 100644 --- a/src/coreComponents/mesh/MeshObjectPath.cpp +++ b/src/coreComponents/mesh/MeshObjectPath.cpp @@ -210,9 +210,9 @@ void processTokenRecursive( dataRepository::Group const & parentGroup, namesInRepository.emplace_back( group.getName() ); } ); - GEOSX_THROW_IF( namesInRepository.empty(), - GEOSX_FMT( "{0} doesn't have any children.", parentGroup.getName()), - InputError ); + GEOS_THROW_IF( namesInRepository.empty(), + GEOS_FMT( "{0} doesn't have any children.", parentGroup.getName()), + InputError ); for( string const & inputEntry : stringutilities::tokenize( pathToken, " " ) ) { diff --git a/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp b/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp index 5841d46334f..20761ea22c7 100644 --- a/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp +++ b/src/coreComponents/unitTests/dataRepositoryTests/testGroupPath.cpp @@ -24,7 +24,7 @@ // Tests the Group::getGroup() and getPath() methods TEST( testGroupPath, testGlobalPaths ) { - using namespace geosx; + using namespace geos; using namespace dataRepository; char const * xmlInput = @@ -119,11 +119,11 @@ int main( int argc, char * * argv ) { ::testing::InitGoogleTest( &argc, argv ); - geosx::GeosxState state( geosx::basicSetup( argc, argv, false ) ); + geos::GeosxState state( geos::basicSetup( argc, argv, false ) ); int const result = RUN_ALL_TESTS(); - geosx::basicCleanup(); + geos::basicCleanup(); return result; }