Skip to content

Conversation

@ZedThree
Copy link
Member

The Laplacian inversion solvers LaplaceCyclic, LaplacePCR, LaplacePCR_THOMAS all use identical code for transforming using FFTs/DSTs. This commit pulls out that common code to make it easier to maintain.

Also, make the transforms parallel-Z-ready by using the Z guard cells as offsets

The Laplacian inversion solvers `LaplaceCyclic`, `LaplacePCR`,
`LaplacePCR_THOMAS` all use identical code for transforming using
FFTs/DSTs. This commit pulls out that common code to make it easier to maintain.

Also, make the transforms parallel-Z-ready by using the Z guard cells as offsets
@ZedThree ZedThree force-pushed the laplace-common-transforms branch from 89d4071 to cb6260d Compare January 30, 2026 13:38
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 28. Check the log or trigger a new build to see more.

void tridagCoefs(int jx, int jy, int jz, dcomplex& a, dcomplex& b, dcomplex& c,
const Field2D* ccoef = nullptr, const Field2D* d = nullptr,
CELL_LOC loc = CELL_DEFAULT);
CELL_LOC loc = CELL_DEFAULT) const;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "CELL_DEFAULT" is directly included [misc-include-cleaner]

                   CELL_LOC loc = CELL_DEFAULT) const;
                                  ^


Matrices result(nsys, nx);

BOUT_OMP_PERF(parallel)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "BOUT_OMP_PERF" is directly included [misc-include-cleaner]

src/invert/laplace/common_transform.cxx:10:

- #include "bout/utils.hxx"
+ #include "bout/openmpwrap.hxx"
+ #include "bout/utils.hxx"

{
/// Create a local thread-scope working array
// ZFFT routine expects input of this length
auto k1d = Array<dcomplex>((nz / 2) + 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "Array" is directly included [misc-include-cleaner]

src/invert/laplace/common_transform.cxx:2:

- #include "bout/constants.hxx"
+ #include "bout/array.hxx"
+ #include "bout/constants.hxx"

// (unless periodic in x)
BOUT_OMP_PERF(for)
for (int ind = 0; ind < nxny; ++ind) {
int ix = xs + (ind / ny);
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 'ix' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change
int ix = xs + (ind / ny);
int const ix = xs + (ind / ny);

BOUT_OMP_PERF(for)
for (int ind = 0; ind < nxny; ++ind) {
int ix = xs + (ind / ny);
int iy = ys + (ind % ny);
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 'iy' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change
int iy = ys + (ind % ny);
int const iy = ys + (ind % ny);

// including boundary conditions
BOUT_OMP_PERF(for nowait)
for (int kz = 0; kz < nmode; kz++) {
BoutReal kwave = kz * 2.0 * PI / zlength; // wave number is 1/[rad]
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 'kwave' of type 'BoutReal' (aka 'double') can be declared 'const' [misc-const-correctness]

Suggested change
BoutReal kwave = kz * 2.0 * PI / zlength; // wave number is 1/[rad]
BoutReal const kwave = kz * 2.0 * PI / zlength; // wave number is 1/[rad]

/// Helper class to do FFTs for `LaplaceCyclic`, `LaplacePCR`, `LaplacePCR_THOMAS`
class FFTTransform {
/// Physical length of the Z domain
BoutReal zlength;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "BoutReal" is directly included [misc-include-cleaner]

src/invert/laplace/common_transform.hxx:2:

- #include "bout/dcomplex.hxx"
+ #include "bout/bout_types.hxx"
+ #include "bout/dcomplex.hxx"


if (localmesh->periodicX) {
// Subtract X average of kz=0 mode
BoutReal local[2] = {
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 declare C-style arrays, use 'std::array' instead [cppcoreguidelines-avoid-c-arrays]

    BoutReal local[2] = {
    ^


irfft(std::begin(k1d), localmesh->LocalNz, x[ix]);
}
BoutReal global[2];
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 declare C-style arrays, use 'std::array' instead [cppcoreguidelines-avoid-c-arrays]

    BoutReal global[2];
    ^

irfft(std::begin(k1d), localmesh->LocalNz, x[ix]);
}
BoutReal global[2];
MPI_Allreduce(local, global, 2, MPI_DOUBLE, MPI_SUM, localmesh->getXcomm());
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 implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

    MPI_Allreduce(local, global, 2, MPI_DOUBLE, MPI_SUM, localmesh->getXcomm());
                         ^

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.

2 participants