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

[EPIC] Improve feedback to user on XML errors #2320

Open
8 of 9 tasks
MelReyCG opened this issue Mar 3, 2023 · 4 comments · Fixed by #2341, #2357, #2404, #2447 or #2450
Open
8 of 9 tasks

[EPIC] Improve feedback to user on XML errors #2320

MelReyCG opened this issue Mar 3, 2023 · 4 comments · Fixed by #2341, #2357, #2404, #2447 or #2450
Assignees
Labels
EPIC The agile epic concept type: feature New feature or request

Comments

@MelReyCG
Copy link
Contributor

MelReyCG commented Mar 3, 2023

Typical XML user syntax, path and naming errors  (additional commas or spaces, misplaced slash, missing fields) produce messages in the standard output which consistency and readibility by a user can be improved.

error improvement exemples.pdf
general presentation.pdf



  • Case 1 - When adding a comma at start, end or between the entries of a {materialList, cellBlocks, cellBlockNames, phaseName, targetRegions}.
  • Typical current output: Controlling expression (should be false): inputStream.peek() != '}'
  • Suggested output: "wellPressureCollection": Empty entry in line "{ water, ,rock }". Please remove a comma or add an entry.
  • GEOS PR XML string array errors feedback #2357
  • LvArray PR #280

  • Case 2.1 - Enter a wrong objectPath in a PackCollection.
  • Typical current output: std::bad_alloc / Segmentation fault
  • Suggested output:
wellPressureCollection: Specified name (wellRegion1) did not find a match with a object in group (ElementRegions). 
Objects that are present in (ElementRegions) are:
reservoir, wellRegion

  • Case 2.2 - Enter a wrong objectPath in general
  • Typical current output: Group m/ElementRegion does not have a child wellRegion
  • Suggested output: Error in initialPressure objectPath : Group ElementRegions doesn't have a child named wellRegion123. ElementRegions have the following children: wellRegion1,wellRegion2,meshRegion1
  • GEOS PR ObjectPath XML errors feedback #2341

  • Case 3 - Enter a wrong fieldName
  • Typical current output: Group m/domain/MeshBodies/cartesianMesh/meshLevels/Level0/ElementRegions/elementRegionsGroup/reservoir/elementSubRegions/cellBlock doesn't have a child pressures
  • Suggested output:
"initialPressure": couldn'd find a field "pressures" in "ElementRegions/reservoir1"
Could be one of : pressure, globalCompFraction
Error happened while checking the children of 
m/domain/MeshBodies/cartesianMesh/meshLevels/Level0/ElementRegions/elementRegionsGroup/reservoir/elementSubRegions/cellBlock.

  • Case 4 - Enter an empty CompositionalMultiphaseFVM::targetRegions, materialList, cellBlockNames
  • Typical current output: Group m/domain/MeshBodies/cartesianMesh/meshLevels/Level0/ElementRegions/elementRegionsGroup doesn't have a child -1
  • Suggested output: "reservoir" must have cellBlocks.

  • Case 5.1 - Enter a forbiden characters (like , or or /) in a name field
  • Typical current output:  Group m/Solvers/compositionalMultiphaseWell doesn't have a child wellControls (the error happens when we reference the object which name ends with a space)
  • Suggested output: "cartesianMesh " name field cannot contain forbidden characters.
  • GEOS PR More precise XML input values validation #2764

  • Case 5.2 - Enter a forbiden characters (like , or or /) in a name field reference
  • Typical current output: Error detected while parsing string: "wellControls "
  • Suggested output: "wellInjector1" wellControlsName field must not contain forbidden characters.
  • GEOS PR More precise XML input values validation #2764

  • Case 5.3 - Enter an objectPath with forbiden special characters in it (like , or or /
  • Typical current outputs: 
    -> Group m/domain/MeshBodies/cartesianMesh/meshLevels/Level0/ElementRegions/elementRegionsGroup/wellRegion/elementSubRegions doesn't have a child wellRegionUniqueSubRegion_
    -> Error detected while parsing string: "ElementRegions/wellRegion/wellRegionUniqueSubRegion "
  • Suggested output: Group "wellPressureCollection" objectPath "wellRegionUniqueSubRegion_" cannot contain forbidden characters.
  • GEOS PR More precise XML input values validation #2764

Side note

These suggested messages could change, they must be followed by the stacktrace, and their goal is to contain all needed information for a developper to debug and for a user to understand his mistake in his xml.

@MelReyCG MelReyCG added type: bug Something isn't working type: new A new issue has been created and requires attention labels Mar 3, 2023
@MelReyCG MelReyCG self-assigned this Mar 3, 2023
@TotoGaz TotoGaz added type: feature New feature or request and removed type: bug Something isn't working labels Mar 5, 2023
@TotoGaz
Copy link
Contributor

TotoGaz commented Mar 5, 2023

Can you put the actual error message close to the desired error message?
It's complicated to follow...

@MelReyCG MelReyCG linked a pull request Mar 16, 2023 that will close this issue
@MelReyCG MelReyCG linked a pull request Mar 27, 2023 that will close this issue
@TotoGaz TotoGaz removed the type: new A new issue has been created and requires attention label Apr 4, 2023
TotoGaz pushed a commit that referenced this issue Apr 25, 2023
1. Prevents crashing when PathCollection::objectPath is wrong (thus, solving the case 2.1 of the issue #2320)

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

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

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

A `testGroupPath` was added to validate:
- the getPath() output,
- the getGroupByPath() input,
- if the Group hierarchy keeps being as we expect.
@MelReyCG MelReyCG linked a pull request May 3, 2023 that will close this issue
@TotoGaz TotoGaz added the EPIC The agile epic concept label May 16, 2023
@MelReyCG MelReyCG reopened this Sep 22, 2023
@MelReyCG MelReyCG changed the title Improve feedback to user on XML errors [EPIC] Improve feedback to user on XML errors Sep 25, 2023
@MelReyCG MelReyCG linked a pull request Oct 17, 2023 that will close this issue
@TotoGaz
Copy link
Contributor

TotoGaz commented Nov 30, 2023

Hurray @MelReyCG , this is done.
I let you the pleasure to tick the boxes!

EDIT: what about case 4?

@MelReyCG
Copy link
Contributor Author

MelReyCG commented Dec 4, 2023

I re-run the tests I made for this EPIC, and this point is not solved yet (the error is still the same).
I'll submit a last PR to solve it, we need to be able to specify if an X_array is allowed to be empty.

@MelReyCG MelReyCG reopened this Dec 4, 2023
@paveltomin
Copy link
Contributor

@MelReyCG is it done now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment