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

Merge 4.4.1 into next #2488

Merged
merged 41 commits into from
Feb 2, 2022
Merged

Merge 4.4.1 into next #2488

merged 41 commits into from
Feb 2, 2022

Conversation

ZedThree
Copy link
Member

@bendudson Could you double-check the SNES stuff? There were a couple of conflicts I had to fix

ZedThree and others added 30 commits September 7, 2021 17:54
Shared objects should be executable
Fix permission of shared objects
Enables much quicker calculation of steady-state solutions than
CVODE typically can.
Recent change in commit hash broke this

Fixes #2461
Ambiguous between mpark:: and std::visit when compiling with GCC
11.1 (Arch linux)
Before this change, setting an integer/BoutReal to, for example,
'true', would yield:

    Cannot find variable 'true'

With this change, now the error is:

    Couldn't get BoutReal from option conduction:chi = 'true': \
    Cannot find variable 'true'

Backported from next

Fixes #2466
…essage

Raise more descriptive error if converting Option fails
Print correct CMake build directory in configure message
This makes the output feel a bit more accurate
[skip ci]

Co-authored-by: johnomotani <john.omotani@ukaea.uk>
CMake: Fix using the build directory directly
- Add equation_form option, to change the nonlinear equation
  being solved: Pseudo-transient, backward Euler (two forms) and
  direct Newton
- Add line_search_type option, to set the SNES line search
- Add stol convergence tolerance option
- Use SNESSetForceIteration if PETSc >= 3.8
- Print more diagnostics if solve diverges or fails
- Add forward Euler step to get out of local minima
- Print more diagnostics if diagnose = true
* master: (27 commits)
  Fix contributor's name
  Update release date
  Update changelog
  Revert SONAME/SOVERSION to 4.4.0
  Fix test-bout-override-default-option for cmake
  Update DOI and release date for 4.4.1
  Update changelog for 4.4.1
  Update translation files for 4.4.1
  Bump version to 4.4.1
  Enable setting SNES solver PETSc options from input file
  Try to consolidate some loops/branches in SNESSolver
  Use `BOUT_ENUM_CLASS` for `SNESSolver::equation_form`
  Use `std::vector/array` instead of C style arrays
  Remove `__FUNCT__` macros for PETSc callbacks
  Update backport of beuler/snes solver
  Fix typo in docs
  Remove IDL section and recommend xBOUT for analysis
  Recommend using xBOUT for manipulating restart files
  Update log output in "running BOUT++" manual section
  Move "Initialising solver" message to top of `Solver::solve`
  ...
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

There were too many comments to post at once. Showing the first 25 out of 33. Check the log or trigger a new build to see more.

}

static PetscErrorCode snesPCapply(PC pc, Vec x, Vec y) {
int ierr;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable ierr is not initialized [cppcoreguidelines-init-variables]

int ierr;
    ^
         = 0

int ierr;

// Get the context
SNESSolver* s;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable s is not initialized [cppcoreguidelines-init-variables]

SNESSolver* s;
            ^
              = nullptr


// Get the context
SNESSolver* s;
ierr = PCShellGetContext(pc, reinterpret_cast<void**>(&s));
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast]

ierr = PCShellGetContext(pc, reinterpret_cast<void**>(&s));
                             ^

PetscFunctionReturn(s->precon(x, y));
}

SNESSolver::SNESSolver(Options* opt)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: constructor does not initialize these fields: timestep, dt, dt_min_reset, lower_its, upper_its, out_timestep, nsteps, diagnose, nlocal, neq, equation_form, snes_f, snes_x, x0, delta_x, predictor, x1, snes, Jmf, fdcoloring [cppcoreguidelines-pro-type-member-init]

SNESSolver::SNESSolver(Options* opt)
^

.doc("Line search type: basic, bt, l2, cp, nleqerr")
.withDefault("default");
if (line_search_type != "default") {
SNESLineSearch linesearch;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable linesearch is not initialized [cppcoreguidelines-init-variables]

SNESLineSearch linesearch;
               ^
                          = nullptr

SNESType snestype;
SNESGetType(snes, &snestype);
output.write("SNES Type : %s\n", snestype);
if (ksptype) {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: implicit conversion KSPType (aka const char *) -> bool [readability-implicit-bool-conversion]

if (ksptype) {
    ^
            != nullptr

if (ksptype) {
output.write("KSP Type : %s\n", ksptype);
}
if (pctype) {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: implicit conversion PCType (aka const char *) -> bool [readability-implicit-bool-conversion]

if (pctype) {
    ^
           != nullptr

if (predictor) {
// Save previous values: x1 <- x0
VecCopy(x0, x1);
time1 = simtime;
}

int nl_its;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable nl_its is not initialized [cppcoreguidelines-init-variables]

int nl_its;
    ^
           = 0

const BoutReal* xdata = nullptr;
int ierr = VecGetArrayRead(snes_x, &xdata);
CHKERRQ(ierr);
load_vars(const_cast<BoutReal*>(xdata));
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]

load_vars(const_cast<BoutReal*>(xdata));
          ^

its);
// Gather and print diagnostic information

int lin_its;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable lin_its is not initialized [cppcoreguidelines-init-variables]

int lin_its;
    ^
            = 0

bendudson
bendudson previously approved these changes Jan 27, 2022
- Ensure that the linear boolean flag is passed through to the rhs
  function
- Formatting strings e.g. `%e` -> `{}`
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

throw BoutException("No user preconditioner");
}

int ierr;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable ierr is not initialized [cppcoreguidelines-init-variables]

int ierr;
    ^
         = 0

int ierr;

// Get data from PETSc into BOUT++ fields
Vec solution;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable solution is not initialized [cppcoreguidelines-init-variables]

Vec solution;
    ^
             = nullptr

// Get data from PETSc into BOUT++ fields
Vec solution;
SNESGetSolution(snes, &solution);
BoutReal* soldata;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable soldata is not initialized [cppcoreguidelines-init-variables]

BoutReal* soldata;
          ^
                  = nullptr

CHKERRQ(ierr);

// Load vector to be inverted into ddt() variables
const BoutReal* xdata;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable xdata is not initialized [cppcoreguidelines-init-variables]

const BoutReal* xdata;
                ^
                      = nullptr

const BoutReal* xdata;
ierr = VecGetArrayRead(x, &xdata);
CHKERRQ(ierr);
load_derivs(const_cast<BoutReal*>(xdata)); // Note: load_derivs does not modify data
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]

load_derivs(const_cast<BoutReal*>(xdata)); // Note: load_derivs does not modify data
            ^

runPreconditioner(simtime + dt, dt, 0.0);

// Save the solution from F_vars
BoutReal* fdata;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable fdata is not initialized [cppcoreguidelines-init-variables]

BoutReal* fdata;
          ^
                = nullptr

@@ -1246,13 +1244,13 @@ int Solver::run_rhs(BoutReal t) {

save_vars(tmp.begin()); // Copy variables into tmp
pre_rhs(t);
status = model->runConvective(t);
status = model->runConvective(t, linear);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: Value stored to status is never read [clang-analyzer-deadcode.DeadStores]

status = model->runConvective(t, linear);
    ^
src/solver/solver.cxx:1247:5: note: Value stored to 'status' is never read

@@ -74,6 +74,18 @@ void PetscLib::setOptionsFromInputFile(KSP& ksp) {
}
}

void PetscLib::setOptionsFromInputFile(SNES& snes) {
auto ierr = SNESSetOptionsPrefix(snes, options_prefix.c_str());
if (ierr) {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: implicit conversion int -> bool [readability-implicit-bool-conversion]

if (ierr) {
    ^
         != 0

}

ierr = SNESSetFromOptions(snes);
if (ierr) {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: implicit conversion int -> bool [readability-implicit-bool-conversion]

if (ierr) {
    ^
         != 0

API includes a `linear` flag, which is set to `true` when
a linearised rhs() function can be used.
@ZedThree ZedThree merged commit 7f733f9 into next Feb 2, 2022
@ZedThree ZedThree deleted the merge-441 branch February 2, 2022 10:09
Copy link
Contributor

@bendudson bendudson left a comment

Choose a reason for hiding this comment

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

Thanks @ZedThree ! LGTM

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

3 participants