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

Add backports and shims for SUNDIALS 6 #2521

Merged
merged 4 commits into from
Mar 4, 2022
Merged

Add backports and shims for SUNDIALS 6 #2521

merged 4 commits into from
Mar 4, 2022

Conversation

ZedThree
Copy link
Member

@ZedThree ZedThree commented Mar 3, 2022

Adds a header for common backports shared between the different SUNDIALS solvers such that the main code only needs to differentiate between SUNDIALS pre/post 3.0.0

Tested locally on SUNDIALS 2.7.0, 3.2.1, 4.1.0, 5.8.0, and 6.1.1

Adds a header for common backports shared between the different
SUNDIALS solvers such that the main code only needs to differentiate
between SUNDIALS pre/post 3.0.0
@ZedThree ZedThree requested a review from dschwoerer March 3, 2022 18:11
Copy link
Contributor

@dschwoerer dschwoerer left a comment

Choose a reason for hiding this comment

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

Nice, looks much cleaner 👍

@ZedThree
Copy link
Member Author

ZedThree commented Mar 4, 2022

clang-tidy-review broke but did actually catch a bug: I had #include "unused.hxx" in the wrong place, it just coincidentally worked because other headers were bringing it in first. Also applied some clang-tidy fixes for braces around single lines

@ZedThree ZedThree requested a review from dschwoerer March 4, 2022 09:31
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

#endif

ArkodeSolver::ArkodeSolver(Options* opts) : Solver(opts) {
ArkodeSolver::ArkodeSolver(Options* opts) : Solver(opts), suncontext(MPI_COMM_WORLD) {
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: NOUT, TIMESTEP, hcur [cppcoreguidelines-pro-type-member-init]

ArkodeSolver::ArkodeSolver(Options* opts) : Solver(opts), suncontext(MPI_COMM_WORLD) {
^

}
#endif

CvodeSolver::CvodeSolver(Options* opts) : Solver(opts) {
CvodeSolver::CvodeSolver(Options* opts) : Solver(opts), suncontext(MPI_COMM_WORLD) {
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: NOUT, TIMESTEP, hcur [cppcoreguidelines-pro-type-member-init]

CvodeSolver::CvodeSolver(Options* opts) : Solver(opts), suncontext(MPI_COMM_WORLD) {
^

#endif

IdaSolver::IdaSolver(Options* opts) : Solver(opts) {
IdaSolver::IdaSolver(Options* opts) : Solver(opts), suncontext(MPI_COMM_WORLD) {
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: NOUT, TIMESTEP [cppcoreguidelines-pro-type-member-init]

IdaSolver::IdaSolver(Options* opts) : Solver(opts), suncontext(MPI_COMM_WORLD) {
^

@@ -160,11 +137,11 @@ int IdaSolver::init(int nout, BoutReal tstep) {
local_N);

// Allocate memory
if ((uvec = N_VNew_Parallel(BoutComm::get(), local_N, neq SUNCTX_PLACEHOLDER)) == nullptr)
if ((uvec = N_VNew_Parallel(BoutComm::get(), local_N, neq, suncontext)) == nullptr)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: statement should be inside braces [readability-braces-around-statements]

if ((uvec = N_VNew_Parallel(BoutComm::get(), local_N, neq, suncontext)) == nullptr)
                                                                                   ^
                                                                                    {

throw BoutException("SUNDIALS memory allocation failed\n");
if ((duvec = N_VNew_Parallel(BoutComm::get(), local_N, neq SUNCTX_PLACEHOLDER)) == nullptr)
if ((duvec = N_VNew_Parallel(BoutComm::get(), local_N, neq, suncontext)) == nullptr)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: statement should be inside braces [readability-braces-around-statements]

if ((duvec = N_VNew_Parallel(BoutComm::get(), local_N, neq, suncontext)) == nullptr)
                                                                                    ^
                                                                                     {

throw BoutException("SUNDIALS memory allocation failed\n");
if ((id = N_VNew_Parallel(BoutComm::get(), local_N, neq SUNCTX_PLACEHOLDER)) == nullptr)
if ((id = N_VNew_Parallel(BoutComm::get(), local_N, neq, suncontext)) == nullptr)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: statement should be inside braces [readability-braces-around-statements]

if ((id = N_VNew_Parallel(BoutComm::get(), local_N, neq, suncontext)) == nullptr)
                                                                                 ^
                                                                                  {

@@ -180,7 +157,7 @@ int IdaSolver::init(int nout, BoutReal tstep) {
set_id(NV_DATA_P(id));

// Call IDACreate to initialise
if ((idamem = IDACreate(SUNCTX_PLACEHOLDER_)) == nullptr)
if ((idamem = IDACreate(suncontext)) == nullptr)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: statement should be inside braces [readability-braces-around-statements]

if ((idamem = IDACreate(suncontext)) == nullptr)
                                                ^
                                                 {

@@ -205,7 +182,7 @@ int IdaSolver::init(int nout, BoutReal tstep) {
// Call IDASpgmr to specify the IDA linear solver IDASPGMR
const auto maxl = (*options)["maxl"].withDefault(6 * n3d);
#if SUNDIALS_VERSION_MAJOR >= 3
if ((sun_solver = SUNLinSol_SPGMR(uvec, SUN_PREC_NONE, maxl SUNCTX_PLACEHOLDER)) == nullptr)
if ((sun_solver = SUNLinSol_SPGMR(uvec, SUN_PREC_NONE, maxl, suncontext)) == nullptr)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: statement should be inside braces [readability-braces-around-statements]

if ((sun_solver = SUNLinSol_SPGMR(uvec, SUN_PREC_NONE, maxl, suncontext)) == nullptr)
                                                                                     ^
                                                                                      {

@ZedThree
Copy link
Member Author

ZedThree commented Mar 4, 2022

Failing test is because Fedora failed to update. This PR is into another PR so I'll merge and let the tests run again there

@ZedThree ZedThree merged commit 7da7e43 into sundials6 Mar 4, 2022
@ZedThree ZedThree deleted the sundials6-shims branch March 4, 2022 10:08
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

2 participants