Report the operative CNV tolerance, the convergence gate and the oscillation source - #7298
Report the operative CNV tolerance, the convergence gate and the oscillation source#7298hnil wants to merge 5 commits into
Conversation
hnil
added
the
manual:enhancement
|
jenkins build this please |
|
CI was red on |
|
jenkins build this please |
bska
left a comment
bska
left a comment
There was a problem hiding this comment.
Looks good for the most part. I just have a couple of remarks concerning the tabulated output format in the INFOITER file, and especially the column sizes.
| { | ||
| // Which condition granted the relaxed CNV tolerance, and the tolerance actually | ||
| // applied. Without this the accepted state's quality is invisible: the same run | ||
| // may accept some steps at tolerance-cnv and others at its relaxed twin. | ||
| os << std::right << std::setw(colSize) << to_string(report.cnvRelaxSource()); | ||
|
|
||
| const auto tol = report.cnvToleranceApplied(); | ||
| if (tol > 0.0) { | ||
| os << std::right << std::setw(colSize) << tol; | ||
| } | ||
| else { | ||
| os << std::right << std::setw(colSize) << '-'; | ||
| } | ||
| } | ||
|
|
||
| void writeOscillationSource(std::ostream& os, | ||
| const std::string::size_type colSize, | ||
| const Opm::ConvergenceReport& report) | ||
| { | ||
| os << std::right << std::setw(colSize) << to_string(report.oscillationSource()); | ||
| } | ||
|
|
||
| void writeConvergenceGate(std::ostream& os, | ||
| const std::string::size_type colSize, | ||
| const Opm::ConvergenceReport& report) | ||
| { | ||
| // Which secondary gate, if any, kept this iteration from counting as | ||
| // converged. These gates fire when the residual metrics themselves | ||
| // are already satisfied, so they identify iterations spent purely on | ||
| // well/group control changes or network balancing. | ||
| const auto gate = report.wellGroupTargetsViolated() | ||
| ? (report.networkNeedsMoreBalancing() ? "GROUP+NETWORK" : "GROUP") | ||
| : (report.networkNeedsMoreBalancing() ? "NETWORK" : "NONE"); | ||
|
|
||
| os << std::right << std::setw(colSize) << gate; | ||
| } |
There was a problem hiding this comment.
As PR #7241 points out, std::setw() takes an int, not a string::size_type. While I would have liked to have this component defined in terms of string::size_type, I've (reluctantly) accepted that that would lead to too many explicit type conversions (i.e., static_cast<>s) in the calls to setw() so that it's better to use int instead of string::size_type here.
It's not a fault of this PR which was just following the established pattern, but I do think we should use int for column sizes instead of string::size_type going forward.
| // are already satisfied, so they identify iterations spent purely on | ||
| // well/group control changes or network balancing. | ||
| const auto gate = report.wellGroupTargetsViolated() | ||
| ? (report.networkNeedsMoreBalancing() ? "GROUP+NETWORK" : "GROUP") |
There was a problem hiding this comment.
Does the string GROUP+NETWORK fit in the colSize? If not, we might consider including that string in the determination of the appropriate column size.
An iteration can have all residual metrics converged and still not count as converged, because a well/group control target changed or the network needs further balancing. These secondary gates were invisible in the convergence output, so iterations spent purely on control changes could not be distinguished from ones spent on the physics. Add const accessors for the two gate flags on ConvergenceReport and a Gate column (NONE/GROUP/NETWORK/GROUP+NETWORK) to INFOITER, placed before WellStatus since well failures append variable-length text that must stay last on the line.
The CNV criterion has two tolerances, tolerance-cnv and tolerance-cnv-relaxed (100x apart by default), and which one applies is decided per iteration by four conditions. Nothing in the output said which tolerance accepted a step, so the local volumetric error of an accepted solution could vary by 100x within a run with no way to tell. ConvergenceReport now carries the granting condition and the tolerance actually used; INFOITER gains CnvRelax (NONE|PVFRAC|DSOL|FINALIT|ITER) and CnvTolUsed columns, and the PRT trailer counts substeps accepted only under a relaxed tolerance. On SPE1CASE2_RADIAL this immediately shows 1357 of 1402 iterations running at 1.0 via the pore-volume branch and only 45 at the nominal 1e-2 -- previously that took a multi-deck study to establish. INFOSTEP unchanged on SPE1CASE2_RADIAL, STONE2_MOD1, 3B_WSEGVALV_MODEL3, GASLIFT-06 and 2_GCONINJE_NETV; convergencereport, gatherconvergencereport and broadcast tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Newton oscillation detector reads reservoir residual history and its only response is damping the reservoir update, but the oscillation may originate in the well/group control layer, where damping dx only helps indirectly. Nothing in the output distinguished the two. The convergence report now records, for iterations where damping triggered, whether the reservoir residuals, the control layer (control equations or a group/network gate), or both were unsatisfied; INFOITER gains an OscSource column and the log line names the source. Measured over the eight decks where the detector fires hardest (774 firings): 85% reservoir-only, 15% mixed, <1% control-only. Read with the base rate in mind -- a non-final iteration nearly always has unsatisfied reservoir residuals -- so the discriminating signal is control participation: 0% on DISPERC_FINGERS, STONE2_MOD1 and ACTIONX_MULT, versus 45% on WVFPEXP-01, 27% on 01-WGRUPCON and ~24% on GASLIFT-09/-10. That separates the pure solution-oscillation decks from those where the control layer takes part. INFOSTEP unchanged on SPE1CASE2_RADIAL, STONE2_MOD1, 3B_WSEGVALV_MODEL3 and GASLIFT-06. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ColumnData stored a Column* per header name while emplacing into the vector those pointers point into. Past the reserve of 20 the vector reallocates and every stored pointer dangles; the INFOITER header was exactly 20 wide, so this only surfaced once a column was added. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
0f0d946 to
81af14c
Compare
"GROUP+NETWORK" is wider than the "Gate" heading it appears under, so the column width has to consider the longest token each diagnostic column can emit. Tabulate the headings with those tokens and take the width from both. The width is unchanged in practice -- the fixed headers already force 17 -- so the INFOITER output is the same; the invariant is now explicit rather than incidental. Also take the column size as int in the three new writers, since that is what std::setw() wants (cf. OPM#7241). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Both addressed: |
|
jenkins build this please |
3 participants
Three INFOITER columns, no numerics touched: which condition granted the relaxed CNV tolerance and the tolerance actually applied (CnvRelax, CnvTolUsed), whether a physically converged iteration was blocked by the well-group/network gate (Gate), and what was unsatisfied when the Newton oscillation damping triggered (OscSource). The PRT trailer counts substeps accepted under a relaxed tolerance.
Motivation: on SPE1CASE2_RADIAL a single run now shows 1357 of 1402 iterations accepted at tolerance 1.0 via the pore-volume branch and only 45 at the nominal 1e-2 — previously invisible. INFOSTEP byte-identical on the decks tested.
🤖 Generated with Claude Code