Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -523,10 +523,8 @@ if (BOUT_ENABLE_WARNINGS)
)

include(EnableCXXWarningIfSupport)
# Note we explicitly turn off -Wcast-function-type as PETSc *requires*
# we cast a function to the wrong type in MatFDColoringSetFunction
target_enable_cxx_warning_if_supported(bout++
FLAGS -Wnull-dereference -Wno-cast-function-type
FLAGS -Wnull-dereference
)
endif()

Expand Down
25 changes: 21 additions & 4 deletions src/solver/impls/imex-bdf2/imex-bdf2.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,25 @@ static PetscErrorCode FormFunctionForColoring(SNES UNUSED(snes), Vec x, Vec f,
return static_cast<IMEXBDF2*>(ctx)->snes_function(x, f, true);
}

static PetscErrorCode imexbdf2PCapply(PC pc, Vec x, Vec y) {
// Global context to store IMEXBDF2 instance
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: variable 'imexbdf2_ctx' declared 'static', move to anonymous namespace instead [misc-use-anonymous-namespace]

static void* imexbdf2_ctx = nullptr;
             ^

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: variable 'imexbdf2_ctx' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

static void* imexbdf2_ctx = nullptr;
             ^

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: variable 'imexbdf2_ctx' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]

static void* imexbdf2_ctx = nullptr;
             ^

static void* imexbdf2_ctx = nullptr;

Comment thread
ZedThree marked this conversation as resolved.
Comment thread
ZedThree marked this conversation as resolved.
#if PETSC_VERSION_GE(3, 24, 0) || PETSC_VERSION_RELEASE == 0
// Wrapper for PETSc 3.24 and later (signature: PetscErrorCode (*)(void*, Vec, Vec, void*))
static PetscErrorCode FormFunctionForColoringWrapper(void*, Vec x, Vec y, void* ctx) {
SNES dummy_snes = nullptr;
return FormFunctionForColoring(dummy_snes, x, y, ctx);
}
#else
// Wrapper for PETSc < 3.20 (signature: PetscErrorCode (*)(void))
Comment thread
ZedThree marked this conversation as resolved.
static PetscErrorCode FormFunctionForColoringWrapper() {
SNES dummy_snes = nullptr;
Vec dummy_vec = nullptr;
return FormFunctionForColoring(dummy_snes, dummy_vec, dummy_vec, imexbdf2_ctx);
}
#endif

Comment thread
ZedThree marked this conversation as resolved.
PetscErrorCode imexbdf2PCapply(PC pc, Vec x, Vec y) {
Copy link
Copy Markdown
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]

Suggested change
PetscErrorCode imexbdf2PCapply(PC pc, Vec x, Vec y) {
int ierr = 0;

int ierr;

// Get the context
Expand Down Expand Up @@ -651,9 +669,8 @@ void IMEXBDF2::constructSNES(SNES* snesIn) {
// Create data structure for SNESComputeJacobianDefaultColor
MatFDColoringCreate(Jmf, iscoloring, &fdcoloring);
// Set the function to difference
MatFDColoringSetFunction(
fdcoloring, reinterpret_cast<PetscErrorCode (*)()>(FormFunctionForColoring),
this);
imexbdf2_ctx = this;
MatFDColoringSetFunction(fdcoloring, FormFunctionForColoringWrapper, this);
MatFDColoringSetFromOptions(fdcoloring);
MatFDColoringSetUp(Jmf, iscoloring, fdcoloring);
ISColoringDestroy(&iscoloring);
Expand Down
24 changes: 22 additions & 2 deletions src/solver/impls/petsc/petsc.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,26 @@ extern PetscErrorCode PhysicsPCApply(PC, Vec x, Vec y);
extern PetscErrorCode PhysicsJacobianApply(Mat J, Vec x, Vec y);
extern PetscErrorCode PhysicsSNESApply(SNES, Vec);

// Global context to store PetscSolver instance
static void* petsc_ctx = nullptr;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: variable 'petsc_ctx' declared 'static', move to anonymous namespace instead [misc-use-anonymous-namespace]

static void* petsc_ctx = nullptr;
             ^

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: variable 'petsc_ctx' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

static void* petsc_ctx = nullptr;
             ^

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: variable 'petsc_ctx' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]

static void* petsc_ctx = nullptr;
             ^


#if PETSC_VERSION_GE(3, 24, 0) || PETSC_VERSION_RELEASE == 0
Comment thread
ZedThree marked this conversation as resolved.
// Wrapper for PETSc 3.24 and later (signature: PetscErrorCode (*)(void*, Vec, Vec, void*))
static PetscErrorCode FormFunctionForColoringWrapper(void*, Vec x, Vec y, void* ctx) {
TS dummy_ts = nullptr;
BoutReal dummy_time = 0.0;
return solver_f(dummy_ts, dummy_time, x, y, ctx);
}
#else
// Wrapper for PETSc < 3.20 (signature: PetscErrorCode (*)(void))
static PetscErrorCode FormFunctionForColoringWrapper() {
Comment thread
ZedThree marked this conversation as resolved.
TS dummy_ts = nullptr;
BoutReal dummy_time = 0.0;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: variable 'dummy_time' of type 'BoutReal' (aka 'double') can be declared 'const' [misc-const-correctness]

Suggested change
BoutReal dummy_time = 0.0;
BoutReal const dummy_time = 0.0;

Vec dummy_vec = nullptr;
return solver_f(dummy_ts, dummy_time, dummy_vec, dummy_vec, petsc_ctx);
}
#endif

PetscSolver::PetscSolver(Options* opts)
: Solver(opts),
diagnose(
Expand Down Expand Up @@ -514,8 +534,8 @@ int PetscSolver::init() {
CHKERRQ(ierr);
ierr = ISColoringDestroy(&iscoloring);
CHKERRQ(ierr);
ierr = MatFDColoringSetFunction(matfdcoloring,
reinterpret_cast<PetscErrorCode (*)()>(solver_f), this);
petsc_ctx = this;
ierr = MatFDColoringSetFunction(matfdcoloring, FormFunctionForColoringWrapper, this);
Comment thread
ZedThree marked this conversation as resolved.
CHKERRQ(ierr);
ierr = SNESSetJacobian(snes, J, J, SNESComputeJacobianDefaultColor, matfdcoloring);
CHKERRQ(ierr);
Expand Down
22 changes: 20 additions & 2 deletions src/solver/impls/snes/snes.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,24 @@ static PetscErrorCode FormFunctionForColoring(SNES UNUSED(snes), Vec x, Vec f,
return static_cast<SNESSolver*>(ctx)->snes_function(x, f, true);
}

// Global context to store SNESSolver instance
static void* snes_ctx = nullptr;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: variable 'snes_ctx' declared 'static', move to anonymous namespace instead [misc-use-anonymous-namespace]

static void* snes_ctx = nullptr;
             ^

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: variable 'snes_ctx' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

static void* snes_ctx = nullptr;
             ^

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: variable 'snes_ctx' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]

static void* snes_ctx = nullptr;
             ^


#if PETSC_VERSION_GE(3, 24, 0) || PETSC_VERSION_RELEASE == 0
Comment thread
ZedThree marked this conversation as resolved.
Comment thread
ZedThree marked this conversation as resolved.
// Wrapper for PETSc 3.24 and later (signature: PetscErrorCode (*)(void*, Vec, Vec, void*))
static PetscErrorCode FormFunctionForColoringWrapper(void*, Vec x, Vec y, void* ctx) {
SNES dummy_snes = nullptr;
return FormFunctionForColoring(dummy_snes, x, y, ctx);
}
#else
// Wrapper for PETSc < 3.20 (signature: PetscErrorCode (*)(void))
static PetscErrorCode FormFunctionForColoringWrapper() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: function 'FormFunctionForColoringWrapper' declared 'static', move to anonymous namespace instead [misc-use-anonymous-namespace]

static PetscErrorCode FormFunctionForColoringWrapper() {
                      ^

SNES dummy_snes = nullptr;
Vec dummy_vec = nullptr;
return FormFunctionForColoring(dummy_snes, dummy_vec, dummy_vec, snes_ctx);
}
#endif

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

Expand Down Expand Up @@ -1291,8 +1309,8 @@ void SNESSolver::updateColoring() {
// Replace the old coloring with the new one
MatFDColoringDestroy(&fdcoloring);
MatFDColoringCreate(Jfd, iscoloring, &fdcoloring);
MatFDColoringSetFunction(
fdcoloring, reinterpret_cast<PetscErrorCode (*)()>(FormFunctionForColoring), this);
snes_ctx = this;
MatFDColoringSetFunction(fdcoloring, FormFunctionForColoringWrapper, this);
Comment thread
ZedThree marked this conversation as resolved.
MatFDColoringSetFromOptions(fdcoloring);
MatFDColoringSetUp(Jfd, iscoloring, fdcoloring);
ISColoringDestroy(&iscoloring);
Expand Down