-
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
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()?
| 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)
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.
Yes, but checking at compile implies that we need to not build some of these for certain types, which might complicate the various macros. At any rate, it's something for a different PR!
[skip ci] Co-authored-by: David Bold <[email protected]>
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