Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Solver improvements #2716

Merged
merged 10 commits into from
Jun 13, 2023
Merged

Solver improvements #2716

merged 10 commits into from
Jun 13, 2023

Conversation

bendudson
Copy link
Contributor

@bendudson bendudson commented May 25, 2023

cvode

Added input options to control tolerances on the inner nonlinear and linear solvers. Tightening the linear solver tolerance, for example, might improve the robustness of the nonlinear iterations. The optimal choice will be problem dependent, so the defaults are unchanged.

beuler / snes

Improved / fixed the preallocation of Jacobian rows, and detection of boundary cells, so that no allocations are needed while setting the stencil pattern. Tested in 1D and 2D (tokamak X-point).

Added an option pc_hypre_type to specify the type of hypre solver (choices are boomeramg, euclid and pilut). The default is pilut which seems to work best for a 1D test problem; euclid seems to be better in 2D. boomeramg doesn't seem to work very well for the transport problems tested.

Moved the PETSc options setting to before PETSc is initialized. This fixes the log_view option. In BOUT.inp:

[petsc]
log_view = true

or on the command line petsc:log_view

bendudson and others added 4 commits May 24, 2023 21:18
No mallocs now needed when setting matrix elements in serial or
parallel for 1D-recycling example.

Hypre preconditioner type can now be set with pc_hypre_type option.
Defaults to pilut, as this seems to give the best performance in
parallel.
cvode_nonlinear_convergence_coef
    Sets parameter through call to CVodeSetNonlinConvCoef
    https://sundials.readthedocs.io/en/latest/cvodes/Usage/SIM.html#c.CVodeSetNonlinConvCoef

cvode_linear_convergence_coef
    Sets parameter through call to CVodeSetEpsLin
    https://sundials.readthedocs.io/en/latest/cvodes/Usage/SIM.html#c.CVodeSetEpsLin
Add some documentation of new cvode and beuler solver options.
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

If PETSc is not configured with Hypre then PCHYPRESetType is
not defined.
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@@ -458,6 +467,10 @@ int CvodeSolver::init() {
#endif
}

// Set internal tolerance factors
CVodeSetNonlinConvCoef(cvode_mem, cvode_nonlinear_convergence_coef);
CVodeSetEpsLin(cvode_mem, cvode_linear_convergence_coef);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: use of undeclared identifier 'CVodeSetEpsLin' [clang-diagnostic-error]

;
    ^

Copy link
Member

Choose a reason for hiding this comment

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

Ah, this is because we use the system version of SUNDIALS for the clang-tidy action, which turns out to be 3.1.2.

I think just updating clang-tidy-review will use a more recent version of Ubuntu and give us SUNDIALS 5.1.8, but I guess the question which versions of SUNDIALS do we want to support?

@mikekryjak
Copy link

This is exciting. Can we add some flags for diagnostics? -log_view gives a detailed profiling report after the completion of the simulation. It would be nice to have that in the input file.

I remember that SNES had a separate tolerance setting for controlling when it's "steady state enough" to go fast and not worry about time accuracy. Could this be a factor that influences the tests? I don't remember whether we have it exposed in the input file or not. I seem to remember it was called ptol or something similar...

If a step fails, re-calculate the Jacobian and preconditioner.
This can be an expensive operation, so doing this on failure
allows lag_jacobian to be increased.
Enables `petsc:log_view=true` option to set the PETSc `-log_view`
switch correctly.
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

src/solver/impls/snes/snes.cxx Outdated Show resolved Hide resolved
src/solver/impls/snes/snes.cxx Outdated Show resolved Hide resolved
src/solver/impls/snes/snes.cxx Outdated Show resolved Hide resolved
@@ -234,10 +240,26 @@ int SNESSolver::init() {
std::vector<PetscInt> o_nnz(localN);

// Set values for most points
const int ncells_x = (mesh->LocalNx > 1) ? 2 : 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const int ncells_x = (mesh->LocalNx > 1) ? 2 : 0;
const int ncells_x = (mesh->xstart > 0) ? 2 : 0;

Maybe rather check whether we have guards in x and y?

@@ -234,10 +240,26 @@ int SNESSolver::init() {
std::vector<PetscInt> o_nnz(localN);

// Set values for most points
const int ncells_x = (mesh->LocalNx > 1) ? 2 : 0;
const int ncells_y = (mesh->LocalNy > 1) ? 2 : 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const int ncells_y = (mesh->LocalNy > 1) ? 2 : 0;
const int ncells_y = (mesh->ystart > 0) ? 2 : 0;

@boutproject boutproject deleted a comment from github-actions bot Jun 6, 2023
bendudson and others added 3 commits June 13, 2023 10:42
Co-authored-by: David Bold <dschwoerer@users.noreply.github.com>
Co-authored-by: David Bold <dschwoerer@users.noreply.github.com>
Co-authored-by: David Bold <dschwoerer@users.noreply.github.com>
@bendudson bendudson merged commit 3084bfb into master Jun 13, 2023
24 checks passed
@bendudson bendudson deleted the solver-improvements branch June 13, 2023 20: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.

None yet

4 participants