Skip to content

Conversation

@ZedThree
Copy link
Member

@ZedThree ZedThree commented Jan 9, 2026

Supports #3242

Note these don't actually add Z parallelisation, just let us handle it correctly in various places

  • Mesh::GlobalZ for global Z index in [0, 1]
  • Mesh::getNZPE/getZProcIndex for Z processor info
  • bout::fft::checkZSerial to guard routines using FFT

These also make a lot of code more consistent between the directions

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

/// Generally, FFTs must be done over the full Z domain. Currently, most
/// methods using FFTs don't handle parallelising in Z
#if BOUT_CHECK_LEVEL > 0
void checkZSerial(const Mesh& mesh, std::string_view name);
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 "Mesh" is directly included [misc-include-cleaner]

include/bout/fft.hxx:32:

- #include <bout/array.hxx>
+ #include "bout/field2d.hxx"
+ #include <bout/array.hxx>

BoutReal BoutMesh::GlobalZ(BoutReal jz) const {

// Get global Z index as a BoutReal
BoutReal zglo;
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 'zglo' is not initialized [cppcoreguidelines-init-variables]

Suggested change
BoutReal zglo;
BoutReal zglo = NAN;

parameters["z"] = (loc == CELL_ZLOW)
? TWOPI * (iz - 0.5) / static_cast<BoutReal>(msh->LocalNz)
: TWOPI * iz / static_cast<BoutReal>(msh->LocalNz);
parameters["z"] = (loc == CELL_ZLOW) ? PI * (msh->GlobalZ(iz) + msh->GlobalZ(iz - 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 "CELL_ZLOW" is directly included [misc-include-cleaner]

  parameters["z"] = (loc == CELL_ZLOW) ? PI * (msh->GlobalZ(iz) + msh->GlobalZ(iz - 1))
                            ^

d7919 and others added 10 commits January 9, 2026 14:34
Tests are updated because `FakeMesh` has `GlobalZ(z) == z`. This is completely
arbitrary, we could have a `GlobalZ` definition that matches `BoutMesh` (`z / nz`)
then we wouldn't need to change the tests, but this way is a bit clearer what
we're testing.
Also make related methods all `const`
These are just the `BoutReal` overloads of the equivalent `getGlobal?Index` methods
Solver* UNUSED(solver))
: Laplacian(opt, loc, mesh_in), A(0.0), C(1.0), D(1.0) {

bout::fft::checkZSerial(*localmesh, "`tri` inversion");
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be clearer to name this assertZSerial()?

const BoutReal zglo = getGlobalZIndex(jz);

if (symmetricGlobalZ) {
// With this definition the boundary sits dx/2 away form the first/last inner points
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// With this definition the boundary sits dx/2 away form the first/last inner points
// With this definition the boundary sits dz/2 away form the first/last inner points

ASSERT2(var.getMesh()->getNguard(direction) >= nGuards);
ASSERT2(direction == DIRECTION::Z); // Only in Z for now
ASSERT2(stagger == STAGGER::None); // Staggering not currently supported
ASSERT2(bout::utils::is_Field3D_v<T>); // Should never need to call this with Field2D
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't that known at compile time?
So there is no runtime overhead, to have this check always included?

(Same for several of the above)

interp_from_aligned->calcWeights(zt_prime_from);

int yvalid = mesh.LocalNy - 2 * mesh.ystart;
// avoid overflow - no stencil need more than 5 points
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// avoid overflow - no stencil need more than 5 points
// avoid overflow - no stencil needs more than 5 points

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.

4 participants