-
Notifications
You must be signed in to change notification settings - Fork 104
Add some Mesh methods for handling Z parallelisation
#3245
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
base: next
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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); |
There was a problem hiding this comment.
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>
src/mesh/impls/bout/boutmesh.cxx
Outdated
| BoutReal BoutMesh::GlobalZ(BoutReal jz) const { | ||
|
|
||
| // Get global Z index as a BoutReal | ||
| BoutReal zglo; |
There was a problem hiding this comment.
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]
| 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)) |
There was a problem hiding this comment.
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))
^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"); |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // 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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // avoid overflow - no stencil need more than 5 points | |
| // avoid overflow - no stencil needs more than 5 points |
Supports #3242
Note these don't actually add Z parallelisation, just let us handle it correctly in various places
Mesh::GlobalZfor global Z index in[0, 1]Mesh::getNZPE/getZProcIndexfor Z processor infobout::fft::checkZSerialto guard routines using FFTThese also make a lot of code more consistent between the directions