Skip to content

feat: well_manager + well estimator - #3972

Merged
MelReyCG merged 154 commits into
developfrom
feature/byer3/wm_we
Jul 31, 2026
Merged

feat: well_manager + well estimator#3972
MelReyCG merged 154 commits into
developfrom
feature/byer3/wm_we

Conversation

@tjb-ltk

@tjb-ltk tjb-ltk commented Feb 13, 2026

Copy link
Copy Markdown
Contributor

Replaces PR #3735 and extends PR #3971

Overview

This PR introduces

  1. An improved well schema layout and code refactor better suited for well modeling. The previous implementation primarily targeted Jacobian generation requirements for the coupled reservoir and well system.

  2. The well estimator is used to select the active well constraint by solving the well system assuming fixed reservoir conditions and selecting the constraint with the highest or lowest well flowing pressure.

    • the estimator is applied at beginning of Jacobian assembly of the coupled system
    • frequency determined via input setting
    • estimator is a data member of WellControl and a tailed version of PhysicsSolver for wells
    • logic underpins future capability to replace the segmented well model with other formulations

This is a breaking change, detailed migration instructions and migration script are posted at #4081

tjb-ltk and others added 30 commits September 29, 2025 16:50
…aint class hierachy , 3) Remove all old constraint handling methods/variables, 4) it compiles ...
…t compositionalMultiphaseFlow/soreideWhitson/1D_100cells/1D_benchmark.xml
…d convert compositionalMultiphaseFlow/soreideWhitson/1D_100cells/1D_benchmark.xml"

This reverts commit fa61ab1.
…well initialization vagaries assoiated with useSurfaceConditions=0
Comment on lines +372 to +377
string const bhp_type = isProducerWell ? MinimumBHPConstraint::catalogName() : MaximumBHPConstraint::catalogName();
bool const no_match_found = std::none_of( constraints.begin(), constraints.end(), [&bhp_type]( const auto & constraint_tuple )
{
return std::get< 1 >( constraint_tuple ) == bhp_type;
} );
GEOS_THROW_IF( no_match_found,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When the sole BHP or rate constraint has constraintActive=0, these existence checks still accept it, while getBHPConstraint and the rate-list helpers filter it out. Subsequent selection either dereferences null or erases an end() iterator because the current constraint is absent from the active list.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure if this address your comment... for all current models constraintActive = 1. The addition of this flag was needed for 2 workflows, wellheadpressure, where the simulator can impose an "internal constraint" and the underpinning of future functionality where a well can be assigned producer and injection constraints, which then come into consideration when only the well type changes, versus the need to define 2 wells with same perforation information. For either the code will need to ensure that an active pressure and rate constraint are present.

Comment thread src/coreComponents/physicsSolvers/fluidFlow/wells/WellControls.cpp
Comment thread src/coreComponents/physicsSolvers/fluidFlow/wells/WellManager.cpp
Comment thread src/coreComponents/physicsSolvers/fluidFlow/wells/WellManager.hpp Outdated
m_useMass( false ),
m_useTotalMassEquation( 1 ),
m_isThermal( 0 ),
m_isCompositional( true ),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should'nt it be an integer and registered for restart ? what happen if we want to restard a non-compositional well ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The variable is an internal flag set for initial and restart runs. I'm guessing the integratedTests would fail if it was set for restarts.

array1d< real64 > localResidualNorm, wellResidalNorm;
array1d< real64 > localResidualNormalizer;

if( isThermal() )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sticking to conventionm shouldn't it be set by NDOF or so ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, it would add the volume balance.. I will create a separate PR so any changes are associated with a specific change.

Comment on lines +609 to +612
if( wellResidalNorm[i] > localResidualNorm[i] )
{
localResidualNorm[i] = wellResidalNorm[i];
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
if( wellResidalNorm[i] > localResidualNorm[i] )
{
localResidualNorm[i] = wellResidalNorm[i];
}
localResidualNorm[i] = LvArray::math::max( localResidualNorm[i], wellResidualNorm[i] );

Comment thread src/coreComponents/physicsSolvers/fluidFlow/wells/WellManager.cpp Outdated
Comment thread src/coreComponents/physicsSolvers/fluidFlow/wells/WellManager.cpp

@jhuang2601 jhuang2601 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@tjb-ltk Great job!

Please update the branch with latest develop repo, which will fix the following failures for readthedoc build:

WARNING: Error, no command xref target from index:QuickStart
WARNING: Error, no command xref target from index:Tutorials
WARNING: Error, no command xref target from index:BasicExamples
WARNING: Error, no command xref target from index:AdvancedExamples
WARNING: Error, no command xref target from index:UserGuide

Comment thread src/coreComponents/physicsSolvers/fluidFlow/wells/CompositionalMultiphaseWell.cpp Outdated
Comment on lines +766 to +769
std::cout << " Well: " << subRegion.getName() << " Est Attempt: " << dtAttempt
<< ", ConfigurationIter: " << configurationLoopIter
<< ", NewtonIter: " << newtonIter
<< ", Residual Norm: " << residualNorm << std::endl;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is inside a Newton iteration loop and will spam output every iteration.
Remove it? or guard by a log level?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed

#include "fileIO/Outputs/OutputBase.hpp"
#include "physicsSolvers/fluidFlow/wells/WellFields.hpp"

#include "functions/FunctionManager.hpp"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
#include "functions/FunctionManager.hpp"

remove this duplicate, as it has already been added in line 37

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

deleted

setApplyDefaultValue( 0 ).
setInputFlag( InputFlags::OPTIONAL ).
setDescription( "Name of the BHP table when the rate is a time dependent function" );
setDescription( "Flag to esitmate well solution prior to coupled reservoir and well solve." );

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
setDescription( "Flag to esitmate well solution prior to coupled reservoir and well solve." );
setDescription( "Flag to estimate well solution prior to coupled reservoir and well solve." );

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Comment thread src/coreComponents/physicsSolvers/fluidFlow/wells/WellControls.hpp Outdated
@@ -345,8 +401,6 @@
scale="0.1"/>
</FieldSpecifications>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
</FieldSpecifications>
</FieldSpecifications>
<!-- SPHINX_TUT_DEAD_OIL_EGG_FIELD_SPECS_END -->

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done


<!-- SPHINX_TUT_DEAD_OIL_EGG_FIELD_SPECS_END -->
<!-- SPHINX_TUT_DEAD_OIL_EGG_OUTPUTS -->
<Outputs>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
<Outputs>
<!-- SPHINX_TUT_DEAD_OIL_EGG_OUTPUTS -->
<Outputs>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

<Restart
name="restartOutput"/>

</Outputs>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
</Outputs>
</Outputs>
<!-- SPHINX_TUT_DEAD_OIL_EGG_OUTPUTS_END -->

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done


<!-- SPHINX_TUT_DEAD_OIL_EGG_OUTPUTS_END -->
<!-- SPHINX_TUT_DEAD_OIL_EGG_TASKS -->
<Tasks>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
<Tasks>
<!-- SPHINX_TUT_DEAD_OIL_EGG_TASKS -->
<Tasks>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

objectPath="ElementRegions/wellRegion4/wellRegion4UniqueSubRegion"
fieldName="wellElementMixtureConnectionRate"/>
fieldName="wellElementConnectionRate"/>
</Tasks>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
</Tasks>
</Tasks>
<!-- SPHINX_TUT_DEAD_OIL_EGG_TASKS_END -->

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good catch

@jhuang2601 jhuang2601 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@tjb-ltk Great job!

Please update the branch with latest develop repo, which will fix the following failures for readthedoc build:

WARNING: Error, no command xref target from index:QuickStart
WARNING: Error, no command xref target from index:Tutorials
WARNING: Error, no command xref target from index:BasicExamples
WARNING: Error, no command xref target from index:AdvancedExamples
WARNING: Error, no command xref target from index:UserGuide

{
setInputFlags( InputFlags::OPTIONAL_NONUNIQUE );

registerWrapper( viewKeyStruct::massRateString(), &m_constraintValue ).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To clarify the comment, the base class WellConstraintBase registers the data member m_constraintValue using the string "value" (WellConstraintBase::viewKeyStruct::constraintValueString()). This means the user will have two field options "value" and "massRate" to set the same value. So we need to deregister the first string before we can attach this one. This applies to the other derived types from WellConstraintBase.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed registration from base class

control="totalVolRate"
maxRelativePressureChange="0.5"
maxCompFractionChange="0.5">
<InjectionPhaseVolumeRateConstraint

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove undefined constraint?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed

Comment thread src/coreComponents/schema/schema.xsd Outdated
Comment on lines +6289 to +6293
<!--massRate => Maximum mass rate (kg/s)-->
<xsd:attribute name="massRate" type="real64" default="0" />
<!--value => Constraint value.
-->
<xsd:attribute name="value" type="groupNameRef" default="0" />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For instance, here "value" and "massRate" refer to the same piece of data.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I removed registration in the base constraint class


<!-- SPHINX_TUT_DEAD_OIL_EGG_EVENTS_END -->
<!-- SPHINX_TUT_DEAD_OIL_EGG_NUMERICAL_METHODS -->
<NumericalMethods>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The SPHINX anchors may be used in the documentation build.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

these were added back

@victorapm victorapm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great, thanks for all the work, Tom!

@MelReyCG MelReyCG left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The PR is a big improvement in architecture and in feature scalability (classes per constraints), thanks for your work Tom!
I have a some concerns on code quality, but i will submit a PR or an issue later, thanks to not delete the branch.

@MelReyCG

Copy link
Copy Markdown
Contributor

We are lacking an approval from @OmarDuran, @bd713, @jafranc, @rrsettgast, and/or @wrtobin for the src/coreComponents/mesh folder.

@MelReyCG

Copy link
Copy Markdown
Contributor

I reverted the changes, we will apply them back later in another (smaller) PR. I left some TODO.

@MelReyCG
MelReyCG merged commit 0f9c572 into develop Jul 31, 2026
22 checks passed
@MelReyCG
MelReyCG deleted the feature/byer3/wm_we branch July 31, 2026 11:57
@MelReyCG
MelReyCG restored the feature/byer3/wm_we branch July 31, 2026 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changes XML input 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: ready for review flag: requires rebaseline Requires rebaseline branch in integratedTests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants