feat: well time step selector based on rates/bhp tables and clarify well rates logic #3427
Conversation
paveltomin
requested review from
CusiniM,
castelletto1,
rrsettgast,
klevzoff,
francoishamon,
matteofrigo5,
cssherman and
wrtobin
as code owners
| @@ -377,4 +377,73 @@ WellControls const & WellSolverBase::getWellControls( WellElementSubRegion const | |||
| return this->getGroup< WellControls >( subRegion.getWellControlsName()); | |||
| } | |||
|
|
|||
| real64 WellSolverBase::setNextDt( real64 const & currentTime, const real64 & lastDt, geos::DomainPartition & domain ) | |||
There was a problem hiding this comment.
main thing is in this function
paveltomin
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
| @@ -2356,13 +2356,6 @@ bool SolidMechanicsLagrangeContact::isFractureAllInStickCondition( DomainPartiti | |||
| return ( ( numNewSlip + numSlip + numOpen ) == 0 ); | |||
| } | |||
|
|
|||
| real64 SolidMechanicsLagrangeContact::setNextDt( real64 const & currentDt, | |||
There was a problem hiding this comment.
Is this intentional? Do we really want this solver to inherit the behaviour of the base class?
There was a problem hiding this comment.
intentional - yes but I forgot about that change and didn't mention it in the description, sorry
it is very confusing what SolidMechanicsLagrangeContact::setNextDt does right now by returning currentDt and next time step, this basically blocks time step increase? (CC @CusiniM @matteofrigo5 @castelletto1)
could go in a small simple PR with just that fix
There was a problem hiding this comment.
@paveltomin Put this in its own PR.
| @@ -315,4 +315,44 @@ WellControls const & WellSolverBase::getWellControls( WellElementSubRegion const | |||
| return this->getGroup< WellControls >( subRegion.getWellControlsName()); | |||
| } | |||
|
|
|||
| real64 WellSolverBase::setNextDt( real64 const & time, const real64 & lastDt, geos::DomainPartition & domain ) | |||
There was a problem hiding this comment.
I think we should include an option to disable this. I might very finely discretise my rate function without wanting geos to take such small time steps.
There was a problem hiding this comment.
Added timeStepFromTables flag to enable new behavior, disabled by default
src/coreComponents/physicsSolvers/fluidFlow/wells/WellSolverBase.cpp
Outdated
Show resolved
Hide resolved
|
Waiting on code owner review from @CusiniM, @Guotong-Ren, @VidarStiernstrom, @castelletto1, @cssherman, @frankfeifan, @jhuang2601, @matteofrigo5, @rrsettgast, @ryar9534, and/or 1 others. |
This comment was marked as resolved.
This comment was marked as resolved.
|
@CusiniM, @VidarStiernstrom, @castelletto1, @cssherman, @frankfeifan, @jhuang2601, @matteofrigo5, @rrsettgast, @ryar9534, and/or @wrtobin please review |
paveltomin
added
the
ci: run integrated tests
paveltomin
requested review from
untereiner,
corbett5 and
MelReyCG
as code owners
paveltomin
added
the
flag: requires rebaseline
paveltomin
removed
the
ci: run integrated tests
|
@CusiniM, @VidarStiernstrom, @castelletto1, @cssherman, @frankfeifan, @jhuang2601, @matteofrigo5, @rrsettgast, @ryar9534, @wrtobin please review |
Two things in this PR:
setNextDtin well solver which looks at all rates/bhp/status tables and makes sure time step is selected so that we honor table intervals. Can be enabled bytimeStepFromTablesflag in well solver.t_n+dt(end of time step) - switch tot_n(beginning of time step) - this should be more consistent withlowerinterpolation method that is used and prevent grabbing "future" rate from tables and avoid weird behavior when time step cut can lead to shutting of wells.