Skip to content

Conversation

@MelReyCG
Copy link
Contributor

@MelReyCG MelReyCG commented Oct 30, 2024

This PR aim to standardise the behaviour & log message when user attempt to create in a given node:

  1. Non-node content, like raw text,
  2. Unexpected XML sub-node (thus, unexpected Group type name request in Group::createChild())

  1. Non-node content

For instance, before this PR, entering raw text in a <ElementRegions> resulted in a misleading message:

***** Rank 0: KeyName () not found in ObjectManager::Catalog

We would now have:

***** Rank 0: Error in node named "ElementRegions" (staircase_co2_wells_3d.xml, l.124): GEOS XML nodes cannot contain text data nor anything but XML nodes.
Erroneous content: "lorem ipsum"

If a Group type would need this kind of sub-content later, we would have to manage this data, and silence this error for the case it would be allowed.


  1. Unexpected sub-node

For instance, before this PR, adding a wrongly spelled <Perforation> , or any unexpected name, in a <InternalWell> a resulted in:

***** LOCATION: /data/pau901/SIM_CS/users/MelvinRey/WorkEnv/GEOSX_repos/GEOS/src/coreComponents/mesh/generators/WellGeneratorBase.cpp:90
***** Controlling expression (should be false): true
***** Rank 0: Unrecognized node: Prerforatio

We would now have:

***** ERROR
***** LOCATION: /data/pau901/SIM_CS/users/MelvinRey/WorkEnv/GEOSX_repos/GEOS/src/coreComponents/mesh/generators/WellGeneratorBase.cpp:81
***** Controlling expression (should be false): childKey != viewKeyStruct::perforationString()
***** Rank 0: The tag "Prerforatio" is invalid within well_producer1 (staircase_co2_wells_3d.xml, l.74). Please verify the keywords spelling and that input file parameters have not changed.
All available tags are: Perforation

Previously, error of this kind were not always managed properly. Also, various Group types messages were not standardized nor indicating available sub-type names.


This PR also improve how wrong character are underlined:

***** Rank 0: Input string validation failed at:
  "well  Region1"
       ^^
  Expected format: Input value must be a string that cannot be empty and contains only upper/lower letters, digits, and the characters  . - _

It was broken for some cases by the PR #3403, so I fixed that and also improved it by underlining the (first) whole sequence of wrong characters.

@MelReyCG MelReyCG self-assigned this Oct 30, 2024
@MelReyCG MelReyCG changed the title Standardisation of incorrect subgroup creation errors Standardisation of incorrect sub-group creation errors Oct 30, 2024
@MelReyCG MelReyCG added flag: ready for review ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI labels Nov 7, 2024
@MelReyCG MelReyCG marked this pull request as ready for review November 7, 2024 11:21
@wrtobin
Copy link
Collaborator

wrtobin commented Dec 19, 2024

looks like there are some changes from geos internal string to specifically std::string on some interfaces, is there a reason this is needed?

@MelReyCG
Copy link
Contributor Author

MelReyCG commented Jan 6, 2025

@wrtobin half mistake, half in place pratices in ObjectCatalog.hpp (this last one has a lot of old std::string).
We have a lot of std::string in coreComponents, but that is a subject for another PR.

@paveltomin paveltomin added flag: no rebaseline Does not require rebaseline ci: run code coverage enables running of the code coverage CI jobs and removed flag: ready for review labels Jan 13, 2025
@codecov
Copy link

codecov bot commented Jan 13, 2025

Codecov Report

Attention: Patch coverage is 81.30081% with 23 lines in your changes missing coverage. Please review.

Project coverage is 56.82%. Comparing base (fd36d2a) to head (58ca20c).
Report is 71 commits behind head on develop.

Files with missing lines Patch % Lines
src/coreComponents/dataRepository/Group.cpp 73.91% 6 Missing ⚠️
...rc/coreComponents/mainInterface/ProblemManager.cpp 81.48% 5 Missing ⚠️
...rc/coreComponents/dataRepository/ObjectCatalog.hpp 60.00% 2 Missing ⚠️
src/coreComponents/mesh/ElementRegionManager.cpp 75.00% 2 Missing ⚠️
src/coreComponents/mesh/ExternalDataSourceBase.cpp 0.00% 2 Missing ⚠️
src/coreComponents/mesh/ParticleManager.cpp 0.00% 2 Missing ⚠️
...c/coreComponents/constitutive/ConstitutiveBase.cpp 50.00% 1 Missing ⚠️
src/coreComponents/dataRepository/DataContext.cpp 90.90% 1 Missing ⚠️
src/coreComponents/events/EventBase.cpp 0.00% 1 Missing ⚠️
.../coreComponents/mesh/ExternalDataSourceManager.cpp 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3418      +/-   ##
===========================================
+ Coverage    56.79%   56.82%   +0.02%     
===========================================
  Files         1154     1154              
  Lines       100074   100079       +5     
===========================================
+ Hits         56836    56867      +31     
+ Misses       43238    43212      -26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@CusiniM CusiniM merged commit 212eb6b into develop Jan 14, 2025
25 checks passed
@CusiniM CusiniM deleted the bugfix/rey/bad-child-error branch January 14, 2025 04:13
danielemoretto44 pushed a commit that referenced this pull request Sep 22, 2025
* 🐛 bugfix: Proper error when detecting child that is not a xml elements

* 💄 small error edit for uniformisation

* 💡 Added note on all empty createChild()

* Re rewrite of ObjectCatalog errors for bad children :
Added context to know the erroneous parent & exposed a unique error reference for other classes.

* added ability to get context from a cpp source file (typically for tests)

* Updated ElementRegionManager error message + refactored how ElementRegionManager define possible child keys

* Updated WellSolverBase error message

* ⚰️ wrong & dead code removal

* 📝 object catalog factory docs update

* Prefering more contextualized catalog error over custom ones

* Updated WellGeneratorBase error message

* standardize group name processing

* regex validation fail message improvement

* 📝 Group::processInputName docs

* 📝 doc add & updates

* 📝 doc update

* std::string -> string

---------

Co-authored-by: Pavel Tomin <paveltomin@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci: run code coverage enables running of the code coverage CI jobs ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: no rebaseline Does not require rebaseline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants