Improving backward Euler solver #2631
Conversation
If scale_rhs = true then the approximate Jacobian calculated by PETSc coloring is used to scale the time derivatives (RHS) by row scaling the Jacobian. This results in pseudo-timestepping, since every equation is effectively using a different timestep.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| if ((err != 0) or (ctx == nullptr)) { | ||
| return err; | ||
| } | ||
|
|
There was a problem hiding this comment.
warning: variable 'fctx' is not initialized [cppcoreguidelines-init-variables]
| SNESSolver* fctx = nullptr; |
| } | ||
|
|
||
| // Get the the SNESSolver pointer from the function call context | ||
| SNESSolver* fctx; |
There was a problem hiding this comment.
warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast]
reinterpret_cast<void**>(&fctx));
^|
|
||
| // Call the SNESSolver function | ||
| return fctx->scaleJacobian(B); | ||
| } |
There was a problem hiding this comment.
warning: constructor does not initialize these fields: dt, nlocal, neq, snes_f, snes_x, x0, delta_x, x1, snes, Jmf, fdcoloring, rhs_scaling_factors, jac_row_inv_norms [cppcoreguidelines-pro-type-member-init]
src/solver/impls/snes/snes.hxx:94:
- BoutReal dt; ///< Current timestep used in snes_function
+ BoutReal dt{}; ///< Current timestep used in snes_functionsrc/solver/impls/snes/snes.hxx:109:
- int nlocal; ///< Number of variables on local processor
- int neq; ///< Number of variables in total
+ int nlocal{}; ///< Number of variables on local processor
+ int neq{}; ///< Number of variables in totalsrc/solver/impls/snes/snes.hxx:116:
- Vec snes_f; ///< Used by SNES to store function
- Vec snes_x; ///< Result of SNES
- Vec x0; ///< Solution at start of current timestep
- Vec delta_x; ///< Change in solution
+ Vec snes_f{}; ///< Used by SNES to store function
+ Vec snes_x{}; ///< Result of SNES
+ Vec x0{}; ///< Solution at start of current timestep
+ Vec delta_x{}; ///< Change in solutionsrc/solver/impls/snes/snes.hxx:122:
- Vec x1; ///< Previous solution
+ Vec x1{}; ///< Previous solutionsrc/solver/impls/snes/snes.hxx:125:
- SNES snes; ///< SNES context
- Mat Jmf; ///< Matrix-free Jacobian
- MatFDColoring fdcoloring; ///< Matrix coloring context, used for finite difference
+ SNES snes{}; ///< SNES context
+ Mat Jmf{}; ///< Matrix-free Jacobian
+ MatFDColoring fdcoloring{}; ///< Matrix coloring context, used for finite differencesrc/solver/impls/snes/snes.hxx:141:
- Vec rhs_scaling_factors; ///< Factors to multiply RHS function
- Vec jac_row_inv_norms; ///< 1 / Norm of the rows of the Jacobian
+ Vec rhs_scaling_factors{}; ///< Factors to multiply RHS function
+ Vec jac_row_inv_norms{}; ///< 1 / Norm of the rows of the Jacobian| if (!scale_rhs) { | ||
| return 0; // Not scaling the RHS values | ||
| } | ||
|
|
There was a problem hiding this comment.
warning: variable 'ierr' is not initialized [cppcoreguidelines-init-variables]
src/solver/impls/snes/snes.cxx:1025:
- r;
+ r = 0;|
|
||
| // Calculate the norm of each row of the Jacobian | ||
| PetscScalar* row_inv_norm_data; | ||
| ierr = VecGetArray(jac_row_inv_norms, &row_inv_norm_data); |
There was a problem hiding this comment.
warning: variable 'ncols' is not initialized [cppcoreguidelines-init-variables]
src/solver/impls/snes/snes.cxx:1042:
- cols;
+ cols = 0;| PetscScalar* row_inv_norm_data; | ||
| ierr = VecGetArray(jac_row_inv_norms, &row_inv_norm_data); | ||
| CHKERRQ(ierr); | ||
|
|
There was a problem hiding this comment.
warning: variable 'vals' is not initialized [cppcoreguidelines-init-variables]
| vals = nullptr; |
|
|
||
| if (diagnose) { |
There was a problem hiding this comment.
warning: multiple declarations in a single statement reduces readability [readability-isolate-declaration]
| if (diagnose) { | |
| ctors | |
| calePetscReal max_scale; | |
| PetscReal min_scale; |
| // Modify the RHS scaling: factor = factor / norm | ||
| ierr = VecPointwiseMult(rhs_scaling_factors, rhs_scaling_factors, jac_row_inv_norms); | ||
| CHKERRQ(ierr); | ||
|
|
There was a problem hiding this comment.
warning: variable 'max_scale' is not initialized [cppcoreguidelines-init-variables]
ctors
^this fix will not be applied because it overlaps with another fix
| // Modify the RHS scaling: factor = factor / norm | ||
| ierr = VecPointwiseMult(rhs_scaling_factors, rhs_scaling_factors, jac_row_inv_norms); | ||
| CHKERRQ(ierr); | ||
|
|
There was a problem hiding this comment.
warning: variable 'min_scale' is not initialized [cppcoreguidelines-init-variables]
ctors
^this fix will not be applied because it overlaps with another fix
Removes function PCGAMGSetSymGraph, so symmetry should be set though matrix options.
If nproc > 1 then the local index offset should only be added to points inside the domain.
| } | ||
|
|
||
| // Get the the SNESSolver pointer from the function call context | ||
| SNESSolver* fctx; |
There was a problem hiding this comment.
warning: variable 'fctx' is not initialized [cppcoreguidelines-init-variables]
| SNESSolver* fctx; | |
| SNESSolver* fctx = nullptr; |
| int ierr; | ||
|
|
||
| // Get index of rows owned by this processor | ||
| int rstart, rend; |
There was a problem hiding this comment.
warning: variable 'ierr' is not initialized [cppcoreguidelines-init-variables]
src/solver/impls/snes/snes.cxx:1030:
- err;
+ err = 0;|
|
||
| // Check that the vector has the same ownership range |
There was a problem hiding this comment.
warning: multiple declarations in a single statement reduces readability [readability-isolate-declaration]
| // Check that the vector has the same ownership range | |
| essor | |
| reint rstart; | |
| int rend; |
|
|
||
| PetscInt ncols; | ||
| const PetscScalar* vals; | ||
| for (int row = rstart; row < rend; row++) { |
There was a problem hiding this comment.
warning: variable 'ncols' is not initialized [cppcoreguidelines-init-variables]
src/solver/impls/snes/snes.cxx:1048:
- cols;
+ cols = 0;| const PetscScalar* vals; | ||
| for (int row = rstart; row < rend; row++) { | ||
| MatGetRow(B, row, &ncols, nullptr, &vals); | ||
|
|
There was a problem hiding this comment.
warning: variable 'vals' is not initialized [cppcoreguidelines-init-variables]
| vals = nullptr; |
| VecMin(rhs_scaling_factors, nullptr, &min_scale); | ||
| output.write("RHS scaling: {} -> {}\n", min_scale, max_scale); |
There was a problem hiding this comment.
warning: multiple declarations in a single statement reduces readability [readability-isolate-declaration]
| VecMin(rhs_scaling_factors, nullptr, &min_scale); | |
| output.write("RHS scaling: {} -> {}\n", min_scale, max_scale); | |
| ctors | |
| calePetscReal max_scale; | |
| PetscReal min_scale; |
| // Print maximum and minimum scaling factors | ||
| PetscReal max_scale, min_scale; | ||
| VecMax(rhs_scaling_factors, nullptr, &max_scale); | ||
| VecMin(rhs_scaling_factors, nullptr, &min_scale); |
There was a problem hiding this comment.
warning: variable 'max_scale' is not initialized [cppcoreguidelines-init-variables]
ctors
^this fix will not be applied because it overlaps with another fix
| // Print maximum and minimum scaling factors | ||
| PetscReal max_scale, min_scale; | ||
| VecMax(rhs_scaling_factors, nullptr, &max_scale); | ||
| VecMin(rhs_scaling_factors, nullptr, &min_scale); |
There was a problem hiding this comment.
warning: variable 'min_scale' is not initialized [cppcoreguidelines-init-variables]
ctors
^this fix will not be applied because it overlaps with another fix
| } | ||
|
|
||
| int ierr; | ||
|
|
There was a problem hiding this comment.
warning: variable 'ierr' is not initialized [cppcoreguidelines-init-variables]
src/solver/impls/snes/snes.cxx:1028:
- err;
+ err = 0;| int rstart, rend; | ||
| MatGetOwnershipRange(B, &rstart, &rend); |
There was a problem hiding this comment.
warning: multiple declarations in a single statement reduces readability [readability-isolate-declaration]
| int rstart, rend; | |
| MatGetOwnershipRange(B, &rstart, &rend); | |
| essor | |
| reint rstart; | |
| int rend; |
| int ierr; | ||
|
|
||
| // Get index of rows owned by this processor | ||
| int rstart, rend; |
There was a problem hiding this comment.
warning: variable 'rstart' is not initialized [cppcoreguidelines-init-variables]
essor
^this fix will not be applied because it overlaps with another fix
|
|
||
| // Get index of rows owned by this processor | ||
| int rstart, rend; | ||
| MatGetOwnershipRange(B, &rstart, &rend); |
There was a problem hiding this comment.
warning: variable 'rend' is not initialized [cppcoreguidelines-init-variables]
rend;
^this fix will not be applied because it overlaps with another fix
| ierr = VecGetArray(jac_row_inv_norms, &row_inv_norm_data); | ||
| CHKERRQ(ierr); | ||
|
|
||
| PetscInt ncols; |
There was a problem hiding this comment.
warning: variable 'ncols' is not initialized [cppcoreguidelines-init-variables]
src/solver/impls/snes/snes.cxx:1046:
- cols;
+ cols = 0;|
|
||
| PetscInt ncols; | ||
| const PetscScalar* vals; | ||
| for (int row = rstart; row < rend; row++) { |
There was a problem hiding this comment.
warning: variable 'vals' is not initialized [cppcoreguidelines-init-variables]
| for (int row = rstart; row < rend; row++) { | |
| vals = nullptr; |
| PetscReal max_scale, min_scale; | ||
| VecMax(rhs_scaling_factors, nullptr, &max_scale); |
There was a problem hiding this comment.
warning: multiple declarations in a single statement reduces readability [readability-isolate-declaration]
| PetscReal max_scale, min_scale; | |
| VecMax(rhs_scaling_factors, nullptr, &max_scale); | |
| ctors | |
| calePetscReal max_scale; | |
| PetscReal min_scale; |
|
|
||
| if (diagnose) { | ||
| // Print maximum and minimum scaling factors | ||
| PetscReal max_scale, min_scale; |
There was a problem hiding this comment.
warning: variable 'max_scale' is not initialized [cppcoreguidelines-init-variables]
ctors
^this fix will not be applied because it overlaps with another fix
|
|
||
| if (diagnose) { | ||
| // Print maximum and minimum scaling factors | ||
| PetscReal max_scale, min_scale; |
There was a problem hiding this comment.
warning: variable 'min_scale' is not initialized [cppcoreguidelines-init-variables]
ctors
^this fix will not be applied because it overlaps with another fix
* Previously the second cmake invocation failed, e.g. after some source changes or after changing a configure time option * It also generated many files, some of which overwrite git-tracked files from autotools, and others where not git-ignored.
Squash optional
Fix some warnings and deprecated headers
The parallel transform already checks whether it is needed. For the identity transform this is always needed, so the second check is harmful in that case, and otherwise not needed.
Remove unnecessary check for aligned fields when applying twist-shift
Physics models often modify metric tensor components during initialisation (e.g. normalisation), so get their values for saving to output only after init has been called.
Get mesh outputs after physics init
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.
If PETSc is not configured with Hypre then PCHYPRESetType is not defined.
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.
They are not used anyway
Reverts some of the deling commits and changes _runs to _compiles
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>
Solver improvements
Do not try to run SLEPc or PETSc in configure
Update `make dist` invocation [skip ci]
mesh:paralleltransform is a section [skip ci]
Update to dnf5 (master)
|
clang-tidy review says "All clean, LGTM! 👍" |
Attempting to improve the steady state (beuler / snes) solver and address #2624
So far:
scale_rhs = truethen the approximate Jacobian calculated by PETSc coloring is used to scale the time derivatives (RHS) by row scaling the Jacobian. This results in pseudo-timestepping, since every equation is effectively using a different timestep. Default is off (false)