Hold converged steps to the minimum time step size too - #7265
Open
hnil wants to merge 1 commit into
Open
Conversation
Both dt floors - the minimum step size and the restart count - are only
consulted when a substep fails. A run whose substeps all converge, but
whose time step controller keeps proposing ever smaller steps, consults
neither: dt shrinks without bound while the elapsed time stands still,
and the run never finishes and never fails.
network/NETWORK_MODEL5_STDW_AUTOCHK reaches this state. Every substep
converges with 9 Newton iterations, the network never balances ("maximum
of 3 network iterations has been used... continue with unconverged
network results"), and dt falls past 1e-57 days with the clock frozen at
day 143.107 of 152 - at which point t + dt == t and no number of steps
can finish the report step.
Apply the existing minimum to the accepted path as well, with a message
that says what actually happened. The deck now stops after 357 steps
instead of running forever.
No test in the compareECLFiles suite reaches the limit; the set of
failing tests is unchanged.
hnil
marked this pull request as ready for review
July 31, 2026 17:52
akva2
approved these changes
Aug 5, 2026
akva2
left a comment
Member
There was a problem hiding this comment.
LGTM. I assume there is a reason why this wasn't rolled into setTimeStep?
Member
|
jenkins build this please |
Member
Author
|
Checked rather than assumed — instrumented Two reasons I still left it at the call site:
Happy to move it if you would rather have one gate. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Both
dtfloors —--solver-min-time-stepand the restart count — are only consulted when a substep fails. A run whose substeps all converge but whose controller keeps proposing smaller steps consults neither:dtshrinks without bound, the clock stands still, and the run never finishes and never fails.network/NETWORK_MODEL5_STDW_AUTOCHKdoes exactly this. Every substep converges with 9 Newton iterations, the network never balances, anddtfalls past 1e-57 days with the clock frozen at day 143.107 of 152 — past the point wheret + dt == t, so no number of steps can finish the report step.Applying the existing minimum on the accepted path stops it after 357 steps with:
The limit is never reached anywhere in the compareECLFiles suite (128 failures before and after, same set — all pre-existing platform diffs on this machine).
Note: reaching this state at all needs #7263's deck to get past its abort; on master the deck stops earlier for the unrelated reason described there. This guard is independent and worth having regardless.