Skip to content

bug: Allow perf connection in top well element - #4146

Open
tjb-ltk wants to merge 8 commits into
developfrom
feature/byer3/inj_constraint
Open

tjb-ltk wants to merge 8 commits into
developfrom
feature/byer3/inj_constraint

Conversation

@tjb-ltk

@tjb-ltk tjb-ltk commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

This PR removes the restriction (thermal models) of no perforations being allowed in the top well element, incorporating the single phase fix in @npillardou PR 3973 and then extending to compositional models. The fix retains the constant temperature boundary condition at top well element but now includes terms for allowing convective heat transfer from the top element into the reservoir cell.
Integrated tests are included for single phase and compositional models. The compositional model also activates the well estimator to arrive at a good initial estimate for the well solution.

@tjb-ltk tjb-ltk self-assigned this Sep 17, 2026
@tjb-ltk tjb-ltk added type: bug Something isn't working type: feature New feature or request ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: no rebaseline Does not require rebaseline ci: run code coverage enables running of the code coverage CI jobs labels Sep 17, 2026
@tjb-ltk
tjb-ltk requested a review from npillardou September 17, 2026 14:44
@tjb-ltk tjb-ltk changed the title fix: Allow perf connection in top well element bug: Allow perf connection in top well element Sep 24, 2026
@tjb-ltk tjb-ltk added the ci: run device builds Allows to run the device (CUDA/HIP) builds label Sep 24, 2026
// populate local flux vector and derivatives
localPerf[TAG::RES ] = m_dt * m_energyPerfFlux[iperf];
localPerf[TAG::WELL ] = -m_dt * m_energyPerfFlux[iperf];
localPerf[TAG::WELL ] = isTopInjectorElement ? 0.0 : -m_dt * m_energyPerfFlux[iperf];

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'm confused as to where the injection temperature boundary condition is being applied. It is very likely being done somewhere else but I am having trouble locating it.

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 question src/coreComponents/physicsSolvers/fluidFlow/wells/kernels/ThermalSinglePhaseWellKernels.hpp line 131 , the energy equation has coefficient of 1 for the dt derivative, all other coefficients are zero, the rhs is zero, the initial segment temp never changes.

@npillardou npillardou 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.

Nice fix on the energy flux, thanks @tjb-ltk, I just have two comments below

// This is a normalizer for the balance equations. The normalizaer should be the current rate not the constraint value!!
// This is one of the reasons for restricting constraint type for a production well
// Another pr will remove fix this (so the cause for difference results is isolated to one change)
auto const * rateConstraint = wellControls.getRateConstraints().front();

@npillardou npillardou Oct 7, 2026 •

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.

front() is a possibly-empty vector :
getRateConstraints() returns a stdVector by value, built by populateConstraints, which only pushes constraints satisfying isConstraintActive() && getConstraintSource() == source. Nothing guarantees the result is non-empty, so front() is UB here. The != nullptr guard can't catch it either: populateConstraints only ever pushes &constraint, never a null pointer. It guards against a null element, not an empty vector.

Perhaps we should use :

 auto const rateConstraints = wellControls.getRateConstraints();
 if( !rateConstraints.empty() )
 {
     m_constraintValue = rateConstraints.front()->getConstraintValue( time );
 }

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.

That's a better safeguard. The source flag does add the potential for error if the developer isn't careful. Currently the default source type will always give you the user specified rate, and if the user doesn't provide a rate constraint in the xml, the model will generate an error and this code is never reached. The source flag was introduced for other workflows where the simulator may decided to introduce a constraint. The wellhead pressure branch uses this flag, but that option never removes the user constraints.

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 device builds Allows to run the device (CUDA/HIP) builds ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: no rebaseline Does not require rebaseline type: bug Something isn't working type: feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants