Skip to content

Conversation

@arng40
Copy link
Contributor

@arng40 arng40 commented Sep 5, 2025

When a box (usually a user-defined volume) is not selected and the deck does not involve a SurfaceGenerator, the simulation is stopped and output an error.

***** Controlling expression (should be false): true
***** Rank 0: bcPressure (thermalLeakyWell_base.xml, l.168): this FieldSpecification targets (an) empty set(s).
Set 'east':
 - Does not capture: ElementRegions/aquiferBottom
 - Instead, captures: faceManager, edgeManager, nodeManager
Set 'north':
 - Does not capture: ElementRegions/aquiferBottom
 - Instead, captures: faceManager, edgeManager, nodeManager
Set 'south':
 - Does not capture: ElementRegions/aquiferBottom
 - Instead, captures: faceManager, edgeManager, nodeManager
Set 'west':
 - Does not capture: ElementRegions/aquiferBottom
 - Instead, captures: faceManager, edgeManager, nodeManager

The user can precise this error state by adding a errorSetMode in the targeted FieldSpecification wrapper with "silent|warning|error".

Also update the example on 5 units tests where the box defined didn't include the targeted mesh.

Edited the Thermal Leaky Well example. The smoke version of this was not implementing the boundary conditions correctly. These have been changed from element based to face based boundary conditions so that they are applicable to both the smoke and benchmark cases. This case has also changed similar to issue #3660. The images have therefore been updated to the current values.

@arng40 arng40 changed the title set an error when a box has no selection feat: Input error when a box does not select anything Sep 5, 2025
@arng40 arng40 self-assigned this Sep 5, 2025
@arng40 arng40 added type: feature New feature or request flag: ready for review flag: requires rebaseline Requires rebaseline branch in integratedTests changes XML input labels Sep 5, 2025
paveltomin
paveltomin previously approved these changes Sep 5, 2025
Copy link
Collaborator

@paveltomin paveltomin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am ok with the change but you will need to update all the hydrofrac inputs...

@arng40 arng40 added the ci: run CUDA builds Allows to triggers (costly) CUDA jobs label Sep 8, 2025
@herve-gross
Copy link
Contributor

@castelletto1 @corbett5 @bd713 : this one needs your help because the CI does not pass, and @arng40 does not understand why. Thank you!

@bd713
Copy link
Contributor

bd713 commented Oct 8, 2025

@castelletto1 @corbett5 @bd713 : this one needs your help because the CI does not pass, and @arng40 does not understand why. Thank you!

When comparing with the reference result, the CI detects that we have introduced a new errorSetMode.
As a result, it sees the outputs as different and considers the test failed... even though the numerical results themselves are likely unchanged.

@rrsettgast
Copy link
Member

@arng40
There seem to be some non-trivial changes to the xml files, which may be leading to some of your integratedTest fails. Please revert those changes back to get a valid test result. Then you can change them and rebaseline once you verify that you haven't introduced a diff through code changes.

@paveltomin paveltomin self-requested a review October 9, 2025 17:29
@paveltomin paveltomin dismissed their stale review October 9, 2025 17:29

many new changes

@castelletto1
Copy link
Contributor

castelletto1 commented Oct 9, 2025

As @bd713 pointed out, all integrated tests have restart hdf5 files that show a structural change due to addition of setErrorMode, precisely:

"Group has a child 'errorSetMode' in the file to compare but not the baseline file."

The only two tests that show numerical diffs are:

  • /compositionalMultiphaseFlow/benchmarks/thermalLeakyWell/thermalLeakyWell_smoke_3d_04/0162.python3_thermalLeakyWell_smoke_3d_04_2_restartcheck_.log
  • /compositionalMultiphaseFlow/benchmarks/thermalLeakyWell/thermalLeakyWell_smoke_3d_01/0158.python3_thermalLeakyWell_smoke_3d_01_2_restartcheck_.log

Given the PR description:

Edited the Thermal Leaky Well example. The smoke version of this was not implementing the boundary conditions correctly. These have been changed from element based to face based boundary conditions so that they are applicable to both the smoke and benchmark cases. This case has also changed similar to issue #3660. The images have therefore been updated to the current values.

... it’s not surprising to see numerical diffs in these two tests.

Unless I am missing something, I would say we are good to go.

@arng40
Copy link
Contributor Author

arng40 commented Oct 10, 2025

With the revert of thermalLeakyWell and the script set to "errorSetMode" I've got

INFO: Total number of log files processed: 1041

INFO: No unfiltered differences were found.

INFO: The following file(s) had at least one error block that was filtered:

@paveltomin
Copy link
Collaborator

{
GEOS_LOG( string( indent, '\t' ) << "-> " << view.second->getName() << " : "
<< LvArray::system::demangleType( *view.second ) );
<< LvArray::system::demangleType( *view.second ) << " size : "<<view.second->size());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add spaces to keep formatting consistent

registerWrapper( viewKeyStruct::errorSetModeString(), &m_emptySetErrorMode ).
setInputFlag( InputFlags::OPTIONAL ).
setApplyDefaultValue( SetErrorMode::error ).
setDescription( "Set the log state when we a “set” does not target any region\n"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove "we"

setDescription( "Set the log state when we a “set” does not target any region\n"
"When set to \"silent\", no output\n"
"When set to \"warning\", output an error\n"
"When set to \"error\", output a throw\n" );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

????
and "error" above

/// The name of a function used to turn on and off the boundary condition.
string m_bcApplicationFunctionName;

/// Value indicating whether we converts an error into a warning
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

grammar

void FieldSpecificationManager::validateBoundaryConditions( MeshLevel & mesh ) const
{
DomainPartition const & domain = this->getGroupByPath< DomainPartition >( "/Problem/domain" );
Group const & meshBodies = domain.getMeshBodies();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed the duplication and let this one

targetGroup.getName() == MeshLevel::groupStructKeys::faceManagerString() ) // the field names of the face BCs are not always
// registered on
// the faceManager...
if( targetGroup.hasWrapper( fieldName ) ||flag == InputFlags::FALSE ||
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formatting

@paveltomin
Copy link
Collaborator

@MelReyCG MelReyCG merged commit 2d40255 into develop Oct 13, 2025
25 of 26 checks passed
@MelReyCG MelReyCG deleted the feat/dudes/error-when-box-select-nothing branch October 13, 2025 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changes XML input ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: ready for review flag: requires rebaseline Requires rebaseline branch in integratedTests type: feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.