Fix lots of clang-tidy issues in fv_ops #3007
Conversation
Mostly:
- include required headers
- rename some short identifiers
- make some local variables `const`
- in a few cases, this involved renaming them to avoid reuse of the
same variable for different cases (e.g. left/right fluxes)
|
clang-tidy review says "All clean, LGTM! 👍" |
| #include "bout/utils.hxx" | ||
| #include <bout/mesh.hxx> | ||
| #include <cmath> | ||
| #include <cstdlib> | ||
|
|
||
| namespace FV { | ||
| /*! | ||
| * Div ( a Grad_perp(f) ) -- ∇⊥ ( a ⋅ ∇⊥ f) -- Vorticity |
There was a problem hiding this comment.
| * Div ( a Grad_perp(f) ) -- ∇⊥ ( a ⋅ ∇⊥ f) -- Vorticity | |
| * Div ( a Grad_perp(f) ) -- ∇⊥ ( a_coef ⋅ ∇⊥ f) -- Vorticity |
| BoutReal c = BoutNaN, m = BoutNaN, p = BoutNaN, mm = BoutNaN, pp = BoutNaN; | ||
|
|
||
| // Left and right cell face values | ||
| BoutReal L, R; | ||
| BoutReal L = BoutNaN, R = BoutNaN; |
There was a problem hiding this comment.
Are we not worried about performance implications?
|
|
||
| //////////////////////////////////////////// | ||
| // Right boundary | ||
|
|
||
| // Calculate velocity at right boundary (y+1/2) | ||
| BoutReal vpar = 0.5 * (v(i, j, k) + v(i, j + 1, k)); | ||
| BoutReal flux; |
There was a problem hiding this comment.
performance implications?
|
|
||
| if ((i.x() == mesh->xend) && (mesh->lastX())) { | ||
| // At right boundary in X | ||
| if (bndry_flux) { | ||
| BoutReal flux; | ||
| if (vR > 0.0) { | ||
| BoutReal flux = BoutNaN; |
There was a problem hiding this comment.
performance? Apparently the compiler cannot deduct that flux is always written, and thus cannot optimize the not-needed write ...
| @@ -451,21 +463,21 @@ const Field3D Div_f_v(const Field3D& n_in, const Vector3D& v, bool bndry_flux) { | |||
| // At left boundary in X | |||
|
|
|||
| if (bndry_flux) { | |||
| BoutReal flux; | |||
| if (vL < 0.0) { | |||
| BoutReal flux = BoutNaN; | |||
|
Not too concerned about performance really, initialisation doesn't tend to have much a real world impact, especially given all the conditionals we've got in this loops. Initialising to zero (instead of NaN) is one option, this basically has no impact as it can be done for free using allocation via zero-pages. Some of these at least could be rewritten as ternaries, allowing us to make the variable |
Mostly:
constsame variable for different cases (e.g. left/right fluxes)
This is mostly to avoid getting the clang-tidy warnings in #2873