Skip to content

feat: feature/byer3/well manager#3971

Open
tjb-ltk wants to merge 97 commits intodevelopfrom
feature/byer3/well_manager
Open

feat: feature/byer3/well manager#3971
tjb-ltk wants to merge 97 commits intodevelopfrom
feature/byer3/well_manager

Conversation

@tjb-ltk
Copy link
Copy Markdown
Contributor

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

feat: Formalize definition of well constraints (this replaces #3862 feat: feature/byer3/well constraints)

allow more than 2 constraints per well
minimize restrictions on rate type
allow production/injection constraints for one well (future capability)
improve code for constraint processing in general and also needed for well estimator

Schema changes / refactoring
WellManager replaces CompositionalMultiphaseWell and SinglePhaseWell, well manager implements physic solver interfaces and iterates over wells to fill coupled Jacobin.

WellControl is now either CompositionalMultiphaseWell or SinglePhaseWell, which implements WellControl interfaces . WellControl should be renamed to "Well"

WellControl provides methods independent of fluid model. CompositionalMulitphasewell /singlephasewell are only kept as owners of Jacobian generation kernels. These could be placed in wellcontrol with simple iscompo check, maybe later.

Subsequent PR's based on this PR

wmwe. - well manager PR with well estimator

wmwe + wellhead pressure constraint

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
@dkachuma dkachuma requested review from bd713 and jafranc as code owners April 9, 2026 14:39
Copy link
Copy Markdown
Contributor

@dkachuma dkachuma left a comment

Choose a reason for hiding this comment

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

It's rather a large PR. Is there anyway we could split it up? For instance do the constraints only for now and leave the element rearrangement for the next step.

* @class BHPConstraint
* @brief This class describes a minimum pressure constraint used to control a injection well.
*/
class BHPConstraint : public WellConstraintBase
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.

Instead of declaring almost the same class twice I would just use a template parameter e.g.

template <WellTypes WELL_TYPE>
class BHPConstraint : public WellConstraintBase
{
  // ...
  static string catalogName()
  {
    if constexpr (WELL_TYPE == WellControls::Type::PRODUCER)
	{
      return "MinimumBHPConstraint";
	}
	else
	{
	  return "MaximumBHPConstraint";
	}
  }
  // ...
};

using MinimumBHPConstraint = BHPConstraint<WellTypes::PRODUCER>;
using MaximumBHPConstraint = BHPConstraint<WellTypes::INJECTOR>;

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 and committed

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.

In terms of splitting the PR... I think it would probably take at least a month and not sure why go this route.

* Either producer or injector
*/
enum class Type : integer
{
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.

We now have WellTypes defined in WellConstants.cpp. Maybe now we can remove this.

{
this->setInputFlags( InputFlags::OPTIONAL_NONUNIQUE );

this->registerWrapper( viewKeyStruct::massRateString(), &this->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.

We need to deregister the "value" wrapper before we can register this. Otherwise the object will have two wrappers that point to the same 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 the this->. But not sure what is meant by deregister. The above code follows pattern in many classes...

setInputFlag( InputFlags::OPTIONAL ).
setDescription( "Name of the well constraint schedule table when the constraint value is a time dependent function. \n" );

registerWrapper( viewKeyStruct::constraintValueString(), &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.

Most of the sub-classes reregister this value under a different name. Before that we need to ensure that this is deregistered to avoid multiple wrapper names referring to the same data.

Comment on lines +265 to +283
// Quantities computed from well constraint solve with this boundary condition

// botton hole pressure
real64 m_BHP;

// phase rates
array1d< real64 > m_phaseVolumeRates;

// liquid rate
real64 m_liquidRate;

// total volume rate
real64 m_totalVolumeRate;

// mass rate
real64 m_massRate;

/// Rate sign. +1 for injector, -1 for producer
real64 m_rateSign;
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.

These variables seem specific to some of the sub classes. Do they need to appear in the base class?

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.

When a constraint is solved using the estimator these quantities define the state of the solve, which is then used to compare constraints to determine limiting constraint. I struggled with how to best associate a state with each constraint, so decided to keep it simple and just store what is needed for compo and single phase in base class. Open to suggestions


// set the reference well element where the BHP control is applied
wellControls.setReferenceGravityCoef( refElev * gravVector[2] );
wellControls.setReferenceGravityCoef( refElev * gravVector[2] ); // tjb remove
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 this not be size 3

Suggested change
wellControls.setReferenceGravityCoef( refElev * gravVector[2] ); // tjb remove
wellControls.setReferenceGravityCoef( refElev * gravVector[3] ); // tjb remove

//struct ConstraintHelper< NC, IS_THERMAL > {};

template< integer NC, integer IS_THERMAL, typename CONSTRAINT = BHPConstraint >
struct ConstraintHelper
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 file looks like it might be difficult to compile. Can we use the kernelSpecs.json to generate instantiations?

control="totalVolRate"
maxRelativePressureChange="0.5"
maxCompFractionChange="0.5"
useMass="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.

Are we going to be specifying this per well?

Comment on lines +52 to +54
<InjectionPhaseVolumeRateConstraint
name="inj"
phaseName="water"/>
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
<InjectionPhaseVolumeRateConstraint
name="inj"
phaseName="water"/>

Not sure we need these empty constraints.


/// Number of bits in mask storage
static constexpr int NUM_BITS = internal::roundToNextPowerOfTwo( MAX_COMP );
static constexpr int NUM_BITS = geos::internal::roundToNextPowerOfTwo( MAX_COMP );
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.

I don't think we need changes to this file.

@dkachuma dkachuma self-requested a review April 20, 2026 21:12
@rrsettgast rrsettgast removed ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI labels May 4, 2026
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 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.

4 participants