Skip to content

TUNINGDP for DT#6756

Open
totto82 wants to merge 1 commit intoOPM:masterfrom
totto82:tuningdT
Open

TUNINGDP for DT#6756
totto82 wants to merge 1 commit intoOPM:masterfrom
totto82:tuningdT

Conversation

@totto82
Copy link
Member

@totto82 totto82 commented Jan 23, 2026

Depends on OPM/opm-common#4924

@totto82 totto82 added the manual:enhancement This is an enhancement/improvent that needs to be documented in the manual label Jan 23, 2026
Copy link
Contributor

@svenn-t svenn-t left a comment

Choose a reason for hiding this comment

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

The implementation in the sequential temperature model looks good. Some questions for the black oil model convergence check.

const bool use_drs_tol = this->param_.tolerance_max_drs_ > 0.0;
const bool use_drv_tol = this->param_.tolerance_max_drv_ > 0.0;
const bool use_dsol_tol = use_dp_tol || use_ds_tol || use_drs_tol || use_drv_tol;
const bool use_dtemp_tol = this->param_.tolerance_max_dtemp_ > 0.0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Some boolean indicating if the temperature model is enable is needed here?

(!use_drs_tol || (maxSolUpd.dRsMax > 0.0 && maxSolUpd.dRsMax < this->param_.tolerance_max_drs_)) &&
(!use_drv_tol || (maxSolUpd.dRvMax > 0.0 && maxSolUpd.dRvMax < this->param_.tolerance_max_drv_));
(!use_drv_tol || (maxSolUpd.dRvMax > 0.0 && maxSolUpd.dRvMax < this->param_.tolerance_max_drv_)) &&
(!use_dtemp_tol || (maxSolUpd.dTempMax > 0.0 && maxSolUpd.dTempMax < this->param_.tolerance_max_dtemp_));
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this check lead to relaxed CNV for energy as well? I guess there is a difference between fully implicit or sequential temperature model on how this should be handled?

Copy link
Member Author

Choose a reason for hiding this comment

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

Aren't we doing that now? use_relaxed_cnv ? is also checked for cnv_energy

Copy link
Contributor

Choose a reason for hiding this comment

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

I was a bit unclear. I meant setting tol_cnv_energy to 1e20 when dTempMax check is fulfilled, similar to what we do for tol_cnv with the line Scalar tolerance_cnv_relaxed = relax_dsol_cnv ? 1e20 : param_.tolerance_cnv_relaxed_;

Copy link
Member Author

Choose a reason for hiding this comment

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

I see. Fixed

msg += ") ";
}

if (use_dsol_tol) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add DT output (if fully implicit?)

print_dsol(use_dp_tol, maxSolUpd.dPMax);
print_dsol(use_ds_tol, maxSolUpd.dSMax);
print_dsol(use_drs_tol, maxSolUpd.dRsMax);
print_dsol(use_drv_tol, maxSolUpd.dRvMax);
Copy link
Contributor

Choose a reason for hiding this comment

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

Add DT output (if fully implicit?)

Copy link
Contributor

Choose a reason for hiding this comment

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

Add tolerance_max_dtemp_?

Copy link
Contributor

Choose a reason for hiding this comment

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

Add tolerance_max_dtemp_?

Copy link
Contributor

Choose a reason for hiding this comment

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

Add tolerance_max_dtemp_?

if (!isNumericalAquiferCell(elem)) {
if (scaled_norm > tolerance_cnv_energy_strict) {
sum_pv_not_converged += pvValue;
dTMax = max(dTMax, std::abs(dT_[globI]));
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't no if it matters, but could you use std::max?

struct ToleranceMaxDrv { static constexpr Scalar value = 0.0; };

template<class Scalar>
struct ToleranceMaxDTemp { static constexpr Scalar value = 0.0; };
Copy link
Contributor

Choose a reason for hiding this comment

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

This leads to command line option name "--tolerance-max-d-temp"

@totto82 totto82 force-pushed the tuningdT branch 2 times, most recently from db0e7c9 to 5ce97b8 Compare January 27, 2026 12:12
@totto82 totto82 marked this pull request as ready for review January 27, 2026 12:12
@svenn-t
Copy link
Contributor

svenn-t commented Jan 27, 2026

The updated changes look good to me!


const bool use_dsol_tol = use_dp_tol || use_ds_tol || use_drs_tol || use_drv_tol || use_dtemp_tol;
bool relax_dsol_cnv = false;
bool relax_energy_cnv = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

As jenkins pointed out, relax_energy_cnv is unused

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual:enhancement This is an enhancement/improvent that needs to be documented in the manual

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants