Field3DParallel + more FCI changes #3197
Conversation
Without this fix, boundary conditions set on yup/down fields are not applied when a boundary is copied from one field to another.
Fix Field3D::setBoundaryTo for FCI methods
Only used in `Coordinates`, so make private implementation detail
Allows throwing more specific error in coordinates
This works for 2D and 3D fields (and is also shorter code)
I sometimes have `[mesh:file]` set in the input file, and specify `grid` on the command line, only to be confused why the wrong grid was picked.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| void limit_at_least(Field3D& f, BoutReal value) const { | ||
| if (ynext(f) < value) { | ||
| ynext(f) = value; | ||
| } |
There was a problem hiding this comment.
warning: use std::max instead of < [readability-use-std-min-max]
include/bout/boundary_iterator.hxx:2:
- #include "bout/mesh.hxx"
+ #include <algorithm>
+
+ #include "bout/mesh.hxx"| } | |
| ynext(f) = std::max(ynext(f), value); |
| #include "bout/sys/range.hxx" | ||
|
|
||
| #include <algorithm> | ||
| #include <functional> |
There was a problem hiding this comment.
warning: included header algorithm is not used directly [misc-include-cleaner]
| #include <functional> | |
| #include <functional> |
| #include <algorithm> | ||
| #include <functional> | ||
|
|
||
| class BoundaryRegionIter { |
There was a problem hiding this comment.
warning: destructor of 'BoundaryRegionIter' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
class BoundaryRegionIter {
^Additional context
include/bout/boundary_iterator.hxx:13: make it public and virtual
class BoundaryRegionIter {
^| virtual void _next() = 0; | ||
| BoundaryRegionIter& operator*() { return *this; } | ||
|
|
||
| void dirichlet_o2(Field3D& f, BoutReal value) const { |
There was a problem hiding this comment.
warning: no header providing "Field3D" is directly included [misc-include-cleaner]
include/bout/boundary_iterator.hxx:4:
- #include "bout/mesh.hxx"
+ #include "bout/field3d.hxx"
+ #include "bout/mesh.hxx"| } | ||
|
|
||
| void limit_at_least(Field3D& f, BoutReal value) const { | ||
| if (ynext(f) < value) { |
There was a problem hiding this comment.
warning: use std::max instead of < [readability-use-std-min-max]
| if (ynext(f) < value) { | |
| ynext(f) = std::max(ynext(f), value); |
| } | ||
| } | ||
|
|
||
| static int abs_offset() { return 1; } |
There was a problem hiding this comment.
warning: no header providing "BOUT_USE_METRIC_3D" is directly included [misc-include-cleaner]
include/bout/boundary_iterator.hxx:4:
- #include "bout/mesh.hxx"
+ #include "bout/build_defines.hxx"
+ #include "bout/mesh.hxx"| RangeIterator r; | ||
| bool is_end; | ||
| }; | ||
|
|
There was a problem hiding this comment.
warning: the parameter 'r' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
| NewBoundaryRegionY(Mesh* mesh, bool lower, const RangeIterator& r) |
| bool is_end; | ||
| }; | ||
|
|
||
| class NewBoundaryRegionY { |
There was a problem hiding this comment.
warning: no header providing "std::move" is directly included [misc-include-cleaner]
include/bout/boundary_iterator.hxx:12:
+ #include <utility>| bool is_end; | ||
| }; | ||
|
|
||
| class NewBoundaryRegionY { |
There was a problem hiding this comment.
warning: passing result of std::move() as a const reference argument; no move will actually happen [performance-move-const-arg]
| class NewBoundaryRegionY { | |
| : mesh(mesh), lower(lower), r(r) {} |
Additional context
include/bout/sys/range.hxx:26: 'RangeIterator' is not move assignable/constructible
class RangeIterator {
^|
|
||
| #include "bout/mesh.hxx" | ||
| #include "bout/region.hxx" | ||
| #include "bout/sys/parallel_stencils.hxx" |
There was a problem hiding this comment.
warning: included header region.hxx is not used directly [misc-include-cleaner]
| #include "bout/sys/parallel_stencils.hxx" | |
| #include "bout/sys/parallel_stencils.hxx" |
| #include "bout/mesh.hxx" | ||
| #include "bout/region.hxx" | ||
| #include "bout/sys/parallel_stencils.hxx" | ||
| #include <string> |
There was a problem hiding this comment.
warning: included header parallel_stencils.hxx is not used directly [misc-include-cleaner]
| #include <string> | |
| #include <string> |
Co-authored-by: David Bold <dschwoerer@users.noreply.github.com>
| void limit_at_least(Field3D& f, BoutReal value) const { | ||
| if (ynext(f) < value) { | ||
| ynext(f) = value; | ||
| } |
There was a problem hiding this comment.
warning: use std::max instead of < [readability-use-std-min-max]
| } | |
| ynext(f) = std::max(ynext(f), value); |
|
|
||
| static int abs_offset() { return 1; } | ||
|
|
||
| #if BOUT_USE_METRIC_3D == 0 |
There was a problem hiding this comment.
warning: no header providing "BOUT_USE_METRIC_3D" is directly included [misc-include-cleaner]
include/bout/boundary_iterator.hxx:4:
- #include "bout/mesh.hxx"
+ #include "bout/build_defines.hxx"
+ #include "bout/mesh.hxx"| static int abs_offset() { return 1; } | ||
|
|
||
| #if BOUT_USE_METRIC_3D == 0 | ||
| BoutReal& ynext(Field2D& f) const { return f[ind().yp(by).xp(bx)]; } |
There was a problem hiding this comment.
warning: no header providing "Field2D" is directly included [misc-include-cleaner]
include/bout/boundary_iterator.hxx:4:
- #include "bout/mesh.hxx"
+ #include "bout/field2d.hxx"
+ #include "bout/mesh.hxx"| const BoutReal& yprev(const Field2D& f) const { return f[ind().yp(-by).xp(-bx)]; } | ||
| #endif | ||
|
|
||
| const int dir; |
There was a problem hiding this comment.
warning: member 'dir' of type 'const int' is const qualified [cppcoreguidelines-avoid-const-or-ref-data-members]
const int dir;
^| const BoutReal& yprev(const Field2D& f) const { return f[ind().yp(-by).xp(-bx)]; } | ||
| #endif | ||
|
|
||
| const int dir; |
There was a problem hiding this comment.
warning: member variable 'dir' has public visibility [cppcoreguidelines-non-private-member-variables-in-classes]
const int dir;
^|
|
||
| template <typename T> | ||
| auto& getStore() { | ||
| if constexpr (std::is_same<T, Field3DParallel>::value) { |
There was a problem hiding this comment.
warning: no header providing "Field3DParallel" is directly included [misc-include-cleaner]
include/bout/deriv_store.hxx:36:
- #include <bout/scorepwrapper.hxx>
+ #include "bout/field3d.hxx"
+ #include <bout/scorepwrapper.hxx>|
|
||
| template <typename T> | ||
| auto& getStore() { | ||
| if constexpr (std::is_same<T, Field3DParallel>::value) { |
There was a problem hiding this comment.
warning: no header providing "std::is_same" is directly included [misc-include-cleaner]
include/bout/deriv_store.hxx:34:
- #include <unordered_map>
+ #include <type_traits>
+ #include <unordered_map>| template <typename T> | ||
| auto& getStore() { | ||
| if constexpr (std::is_same<T, Field3DParallel>::value) { | ||
| return DerivativeStore<Field3D>::getInstance(); |
There was a problem hiding this comment.
warning: no header providing "Field3D" is directly included [misc-include-cleaner]
return DerivativeStore<Field3D>::getInstance();
^| swap(first.directions, second.directions); | ||
| } | ||
|
|
||
| virtual void setRegion(size_t UNUSED(regionID)) {} |
There was a problem hiding this comment.
warning: no header providing "UNUSED" is directly included [misc-include-cleaner]
include/bout/field.hxx:26:
- class Field;
+ #include "bout/unused.hxx"
+ class Field;| template <typename T> | ||
| inline T toFieldAligned(const T& f, const std::string& region = "RGN_ALL") { | ||
| static_assert(bout::utils::is_Field_v<T>, "toFieldAligned only works on Fields"); | ||
| ASSERT3(f.getCoordinates() != nullptr); |
There was a problem hiding this comment.
warning: no header providing "ASSERT3" is directly included [misc-include-cleaner]
ASSERT3(f.getCoordinates() != nullptr);
^
| ******************************************************************/ | ||
|
|
||
| #define PVODE_BAND_ELEM(A,i,j) ((A->data)[j][i-j+(A->smu)]) | ||
| #define PVODE_BAND_ELEM(A, i, j) (((A)->data)[j][(i) - (j) + ((A)->smu)]) |
There was a problem hiding this comment.
warning: function-like macro 'PVODE_BAND_ELEM' used; consider a 'constexpr' template function [cppcoreguidelines-macro-usage]
#define PVODE_BAND_ELEM(A, i, j) (((A)->data)[j][(i) - (j) + ((A)->smu)])
^| ******************************************************************/ | ||
|
|
||
| #define PVODE_BAND_COL(A,j) (((A->data)[j])+(A->smu)) | ||
| #define PVODE_BAND_COL(A, j) ((((A)->data)[j]) + ((A)->smu)) |
There was a problem hiding this comment.
warning: function-like macro 'PVODE_BAND_COL' used; consider a 'constexpr' template function [cppcoreguidelines-macro-usage]
#define PVODE_BAND_COL(A, j) ((((A)->data)[j]) + ((A)->smu))
^| ******************************************************************/ | ||
|
|
||
| #define BAND_COL(A, j) (((A->data)[j]) + (A->smu)) | ||
| #define PVODE_BAND_COL(A, j) ((((A)->data)[j]) + ((A)->smu)) |
There was a problem hiding this comment.
warning: function-like macro 'PVODE_BAND_COL' used; consider a 'constexpr' template function [cppcoreguidelines-macro-usage]
#define PVODE_BAND_COL(A, j) ((((A)->data)[j]) + ((A)->smu))
^| #include <algorithm> | ||
| #include <functional> | ||
|
|
||
| class BoundaryRegionIter { |
There was a problem hiding this comment.
warning: class 'BoundaryRegionIter' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
class BoundaryRegionIter {
^| inline T toFieldAligned(const T& f, const std::string& region = "RGN_ALL") { | ||
| static_assert(bout::utils::is_Field_v<T>, "toFieldAligned only works on Fields"); | ||
| ASSERT3(f.getCoordinates() != nullptr); | ||
| return f.getCoordinates()->getParallelTransform().toFieldAligned(f, region); |
There was a problem hiding this comment.
warning: member access into incomplete type 'Coordinates' [clang-diagnostic-error]
return f.getCoordinates()->getParallelTransform().toFieldAligned(f, region);
^Additional context
include/bout/index_derivs_interface.hxx:218: in instantiation of function template specialization 'toFieldAligned' requested here
const T f_aligned = is_unaligned ? toFieldAligned(f, "RGN_NOX") : f;
^include/bout/index_derivs_interface.hxx:227: in instantiation of function template specialization 'bout::derivatives::index::DDY' requested here
return DDY(f.asField3D(), outloc, method, region);
^include/bout/field_data.hxx:42: forward declaration of 'Coordinates'
class Coordinates;
^|
|
||
| int tracking_state{0}; | ||
| Options* tracking{nullptr}; | ||
| std::string selfname; |
There was a problem hiding this comment.
warning: member variable 'selfname' has protected visibility [cppcoreguidelines-non-private-member-variables-in-classes]
std::string selfname;
^| explicit Field3DParallel(Types... args) : Field3D(std::move(args)...) { | ||
| ensureFieldAligned(); | ||
| } | ||
| Field3DParallel(const Field3D& f) : Field3D(std::move(f)) { ensureFieldAligned(); } |
There was a problem hiding this comment.
warning: std::move of the const variable 'f' has no effect; remove std::move() or make the variable non-const [performance-move-const-arg]
| Field3DParallel(const Field3D& f) : Field3D(std::move(f)) { ensureFieldAligned(); } | |
| Field3DParallel(const Field3D& f) : Field3D(f) { ensureFieldAligned(); } |
| ensureFieldAligned(); | ||
| } | ||
| Field3DParallel(const Field3D& f) : Field3D(std::move(f)) { ensureFieldAligned(); } | ||
| Field3DParallel(const Field2D& f) : Field3D(std::move(f)) { ensureFieldAligned(); } |
There was a problem hiding this comment.
warning: std::move of the const variable 'f' has no effect; remove std::move() or make the variable non-const [performance-move-const-arg]
| Field3DParallel(const Field2D& f) : Field3D(std::move(f)) { ensureFieldAligned(); } | |
| Field3DParallel(const Field2D& f) : Field3D(f) { ensureFieldAligned(); } |
| }; | ||
|
|
||
| Field3DParallel Field3D::asField3DParallel() { return Field3DParallel(*this); } | ||
| const Field3DParallel Field3D::asField3DParallel() const { |
There was a problem hiding this comment.
warning: return type 'const Field3DParallel' is 'const'-qualified at the top level, which may reduce code readability without improving const correctness [readability-const-return-type]
include/bout/field3d.hxx:533:
- inline const Field3DParallel asField3DParallel() const;
+ inline Field3DParallel asField3DParallel() const;| const Field3DParallel Field3D::asField3DParallel() const { | |
| Field3DParallel Field3D::asField3DParallel() const { |
|
|
||
| inline Field3DParallel | ||
| filledFrom(const Field3DParallel& f, | ||
| std::function<BoutReal(int yoffset, Ind3D index)> func) { |
There was a problem hiding this comment.
warning: the parameter 'func' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
| std::function<BoutReal(int yoffset, Ind3D index)> func) { | |
| const std::function<BoutReal(int yoffset, Ind3D index)>& func) { |
| ******************************************************************/ | ||
|
|
||
| #define PVODE_BAND_ELEM(A,i,j) ((A->data)[j][i-j+(A->smu)]) | ||
| #define PVODE_BAND_ELEM(A, i, j) (((A)->data)[j][(i) - (j) + ((A)->smu)]) |
There was a problem hiding this comment.
warning: function-like macro 'PVODE_BAND_ELEM' used; consider a 'constexpr' template function [cppcoreguidelines-macro-usage]
#define PVODE_BAND_ELEM(A, i, j) (((A)->data)[j][(i) - (j) + ((A)->smu)])
^| ******************************************************************/ | ||
|
|
||
| #define PVODE_BAND_COL(A,j) (((A->data)[j])+(A->smu)) | ||
| #define PVODE_BAND_COL(A, j) ((((A)->data)[j]) + ((A)->smu)) |
There was a problem hiding this comment.
warning: function-like macro 'PVODE_BAND_COL' used; consider a 'constexpr' template function [cppcoreguidelines-macro-usage]
#define PVODE_BAND_COL(A, j) ((((A)->data)[j]) + ((A)->smu))
^| ******************************************************************/ | ||
|
|
||
| #define BAND_COL(A, j) (((A->data)[j]) + (A->smu)) | ||
| #define PVODE_BAND_COL(A, j) ((((A)->data)[j]) + ((A)->smu)) |
There was a problem hiding this comment.
warning: function-like macro 'PVODE_BAND_COL' used; consider a 'constexpr' template function [cppcoreguidelines-macro-usage]
#define PVODE_BAND_COL(A, j) ((((A)->data)[j]) + ((A)->smu))
^| #include <algorithm> | ||
| #include <functional> | ||
|
|
||
| class BoundaryRegionIter { |
There was a problem hiding this comment.
warning: class 'BoundaryRegionIter' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
class BoundaryRegionIter {
^| inline T toFieldAligned(const T& f, const std::string& region = "RGN_ALL") { | ||
| static_assert(bout::utils::is_Field_v<T>, "toFieldAligned only works on Fields"); | ||
| ASSERT3(f.getCoordinates() != nullptr); | ||
| return f.getCoordinates()->getParallelTransform().toFieldAligned(f, region); |
There was a problem hiding this comment.
warning: member access into incomplete type 'Coordinates' [clang-diagnostic-error]
return f.getCoordinates()->getParallelTransform().toFieldAligned(f, region);
^Additional context
include/bout/index_derivs_interface.hxx:218: in instantiation of function template specialization 'toFieldAligned' requested here
const T f_aligned = is_unaligned ? toFieldAligned(f, "RGN_NOX") : f;
^include/bout/index_derivs_interface.hxx:227: in instantiation of function template specialization 'bout::derivatives::index::DDY' requested here
return DDY(f.asField3D(), outloc, method, region);
^include/bout/field_data.hxx:42: forward declaration of 'Coordinates'
class Coordinates;
^|
|
||
| int tracking_state{0}; | ||
| Options* tracking{nullptr}; | ||
| std::string selfname; |
There was a problem hiding this comment.
warning: member variable 'selfname' has protected visibility [cppcoreguidelines-non-private-member-variables-in-classes]
std::string selfname;
^| explicit Field3DParallel(Types... args) : Field3D(std::move(args)...) { | ||
| ensureFieldAligned(); | ||
| } | ||
| Field3DParallel(const Field3D& f) : Field3D(std::move(f)) { ensureFieldAligned(); } |
There was a problem hiding this comment.
warning: std::move of the const variable 'f' has no effect; remove std::move() or make the variable non-const [performance-move-const-arg]
| Field3DParallel(const Field3D& f) : Field3D(std::move(f)) { ensureFieldAligned(); } | |
| Field3DParallel(const Field3D& f) : Field3D(f) { ensureFieldAligned(); } |
| ensureFieldAligned(); | ||
| } | ||
| Field3DParallel(const Field3D& f) : Field3D(std::move(f)) { ensureFieldAligned(); } | ||
| Field3DParallel(const Field2D& f) : Field3D(std::move(f)) { ensureFieldAligned(); } |
There was a problem hiding this comment.
warning: std::move of the const variable 'f' has no effect; remove std::move() or make the variable non-const [performance-move-const-arg]
| Field3DParallel(const Field2D& f) : Field3D(std::move(f)) { ensureFieldAligned(); } | |
| Field3DParallel(const Field2D& f) : Field3D(f) { ensureFieldAligned(); } |
| }; | ||
|
|
||
| Field3DParallel Field3D::asField3DParallel() { return Field3DParallel(*this); } | ||
| const Field3DParallel Field3D::asField3DParallel() const { |
There was a problem hiding this comment.
warning: return type 'const Field3DParallel' is 'const'-qualified at the top level, which may reduce code readability without improving const correctness [readability-const-return-type]
include/bout/field3d.hxx:533:
- inline const Field3DParallel asField3DParallel() const;
+ inline Field3DParallel asField3DParallel() const;| const Field3DParallel Field3D::asField3DParallel() const { | |
| Field3DParallel Field3D::asField3DParallel() const { |
|
|
||
| inline Field3DParallel | ||
| filledFrom(const Field3DParallel& f, | ||
| std::function<BoutReal(int yoffset, Ind3D index)> func) { |
There was a problem hiding this comment.
warning: the parameter 'func' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
| std::function<BoutReal(int yoffset, Ind3D index)> func) { | |
| const std::function<BoutReal(int yoffset, Ind3D index)>& func) { |
|
Please can you split this into one PR per change? It's 100 files and 5k lines of changes, it's going to be very difficult to review. GitHub will only show a single file at once. |
|
Sure! |
Here a summary of the changes:
Field3DParallel
A Field3DParallel is essentially a Field3D, that preserves the parallel slices.
This allows to avoid unnecessary computation of of parallel fields, that are not needed.
It also ensures parallel slices are present (for FCI) when later parallel derivatives are computed.
So any function that will call
DDYlater on, can change the signature toField3DParallelto declare that parallel slices need to be present for FCI.New Y-Boundary Operators
Custom sheath boundary conditions can now be implemented using an abstraction. The documentation is on RTD
That allows having one code path for FCI and FA, for upper and lower sheath BC, all in one place.
Various small fixups
loadParallelMetrics
For FCI the parallel slices of the metric components are loaded, as they are sometimes needed.
This makes FCI essentially 3D only.
There is now also the option to say a field is not allow to compute the parallel slices, to avoid overwriting them:
allowCalcParallelSlices.Tracking on failure
Only if the simulation fails to evolve (Currently euler and pvode only):
The different components of timederivate are dumped to a
BOUT.debug.*.ncwhich can be handy for figuring out which term is causing the instability. No more having to re-run the simulation with some terms disabled to debug such issues. This causes (an unkown) memory overhead - but ONLY if the solver has failed already.More names of parallel boundary region + cleanup of code
Support things like
bndry_par_xinorbndry_par_yup.Add monotonichermitespline for all cases
Merged into hermitespline and works also in parallel.
Support several parallel slices for FCI
MYG=2is now supported.