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

Bug fixes for imex-bdf2 solver #2372

Merged
merged 2 commits into from
Jul 7, 2021
Merged

Bug fixes for imex-bdf2 solver #2372

merged 2 commits into from
Jul 7, 2021

Conversation

bendudson
Copy link
Contributor

Tested with latest petsc release (3.15). In the preconditioner:

  1. Wrong vector was being used for solution
  2. VecGetArray was used instead of VecGetArrayRead
    Both of these issues were caught by PETSc's increasing strictness with locking of vectors.

Copy-paste error, extracting data from wrong Vec
Shouldn't have affected any results, since only affects
preconditioner, but might have made preconditioners less effective
than they could have been.
@bendudson bendudson added bugfix small-change Changes less than 100 lines - should be quick to review labels Jul 7, 2021
Vec is locked by PETSc (at least in 3.20), so need read-only access.
Since `load_vars` takes a `BoutReal*`, a `const_cast` is needed for
now.
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

ierr = VecGetArray(x,&xdata);CHKERRQ(ierr);
load_derivs(xdata);
ierr = VecRestoreArray(x,&xdata);CHKERRQ(ierr);
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

ierr = VecRestoreArray(x,&xdata);CHKERRQ(ierr);
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
            ^

@bendudson bendudson merged commit 3a2c890 into next Jul 7, 2021
@bendudson bendudson deleted the next-fix-imexbdf2 branch July 7, 2021 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix small-change Changes less than 100 lines - should be quick to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants