Skip to content

Conversation

@joshuahansel
Copy link
Contributor

@joshuahansel joshuahansel self-assigned this Oct 10, 2025
@moosebuild
Copy link
Contributor

moosebuild commented Oct 10, 2025

Job Documentation, step Docs: sync website on f8ed4a2 wanted to post the following:

View the site here

This comment will be updated on new commits.

Copy link
Contributor

@GiudGiud GiudGiud left a comment

Choose a reason for hiding this comment

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

early review as I m looking for PRs we could process soon

The parameter [!param](/Convergence/PostprocessorConvergence/max_diverging_iterations) may be used to diverge after the specified number of consecutive iterations for which the post-processor/error value is "diverging". By default, "diverging" means the error value is getting larger, but the parameter [!param](/Convergence/PostprocessorConvergence/diverging_iteration_rel_reduction) can be used to specify some minimum reduction value $\tau_\text{reduction,min}$ such that the "diverging" condition is the following:

!equation
\frac{|y_{\ell-1}| - |y_\ell|}{|y_{\ell-1}|} < \tau_\text{reduction,min}
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe > ?
seems like reduction is good for convergence

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was intentional, but on second look, I need to think more about it. Basically I'm trying to capture when convergence is stagnating (converging too slowly), but the same might be used for a convergence criteria... I guess this divergence criteria only makes sense if you are NOT using any sort of step convergence criteria, only an absolute criteria (like a residual).

Do you think it would be better if this feature were its own Convergence object (maybe it's more clear that way? SlowProgressDivergence).

Copy link
Contributor

Choose a reason for hiding this comment

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

Basically I'm trying to capture when convergence is stagnating (converging too slowly)

the text is not clear then. You say . By default, "diverging" means the error value is getting larger, and then talk about it getting reduced by a factor and I did not catch it on first read
Maybe make bullet points

Do you think it would be better if this feature were its own Convergence object (maybe it's more clear that way? SlowProgressDivergence).

SlowProgress could be anything though. it d need to be a 'PostprocessorConvergence' still in the name.

At the end of the day you are hard coding that you want at least linear convergence. If you were to generalize to make a more self-standing object you'd want to make it about the order of convergence

{
oss << "Converged due to iterations (" << iter << ") >= max iterations ("
<< _max_iterations << ") and 'converge_at_max_iterations' = 'true'.";
oss << COLOR_GREEN << "Converged due to iterations (" << iter << ") >= max iterations ("
Copy link
Contributor

Choose a reason for hiding this comment

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

There was some argument about setting colors directly in ActionWarehouse.C line 268

  /**
   * Note: This routine uses the XTerm colors directly which is not advised for general purpose
   * output coloring.
   * Most users should prefer using Problem::colorText() which respects the "color_output" option
   * for terminals
   * that do not support coloring.  Since this routine is intended for debugging only and runs
   * before several
   * objects exist i

@permcody you wrote that do we still care?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only see "colorText" for python, not C.

Copy link
Contributor

Choose a reason for hiding this comment

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

It might have been an old routine

params.addRequiredParam<Real>("tolerance", "Absolute tolerance to use for convergence criteria");
params.addParam<unsigned int>("max_diverging_iterations",
std::numeric_limits<unsigned int>::max(),
"Number of consecutive iterations of the error getting worse at "
Copy link
Contributor

Choose a reason for hiding this comment

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

"error" is not really well defined here. We refer to the PP and a tolerance but not necessarily an error

}
else
{
std::ostringstream oss;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you want to reset _diverging_iterations = 0 in that case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I'll put it in the "else" condition for if (rel_reduction < _div_rel_reduction)

@moosebuild
Copy link
Contributor

Job Test, step Results summary on f8ed4a2 wanted to post the following:

Framework test summary

Compared against bb0a08b in job civet.inl.gov/job/3353620.

Removed tests

Added tests

Test Time (s)
convergence/postprocessor_convergence.diverging_iterations 1.29

Run time changes

Modules test summary

Compared against bb0a08b in job civet.inl.gov/job/3353620.

Removed tests

Added tests

Run time changes

@moosebuild
Copy link
Contributor

Job Coverage, step Generate coverage on f8ed4a2 wanted to post the following:

Framework coverage

bb0a08 #31706 f8ed4a
Total Total +/- New
Rate 86.02% 86.02% +0.00% 100.00%
Hits 124559 124594 +35 45
Misses 20244 20244 - 0

Diff coverage report

Full coverage report

Modules coverage

Coverage did not change

Full coverage reports

Reports

This comment will be updated on new commits.

@joshuahansel joshuahansel marked this pull request as ready for review November 4, 2025 15:01
@joshuahansel joshuahansel requested a review from GiudGiud November 4, 2025 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants