Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| protected: | ||
| int z{0}; | ||
| int x; | ||
| int y; |
There was a problem hiding this comment.
warning: member variable 'y' has protected visibility [cppcoreguidelines-non-private-member-variables-in-classes]
int y;
^| int z{0}; | ||
| int x; | ||
| int y; | ||
| const int bx; |
There was a problem hiding this comment.
warning: member 'bx' of type 'const int' is const qualified [cppcoreguidelines-avoid-const-or-ref-data-members]
const int bx;
^| int z{0}; | ||
| int x; | ||
| int y; | ||
| const int bx; |
There was a problem hiding this comment.
warning: member variable 'bx' has protected visibility [cppcoreguidelines-non-private-member-variables-in-classes]
const int bx;
^| int x; | ||
| int y; | ||
| const int bx; | ||
| const int by; |
There was a problem hiding this comment.
warning: member 'by' of type 'const int' is const qualified [cppcoreguidelines-avoid-const-or-ref-data-members]
const int by;
^| int x; | ||
| int y; | ||
| const int bx; | ||
| const int by; |
There was a problem hiding this comment.
warning: member variable 'by' has protected visibility [cppcoreguidelines-non-private-member-variables-in-classes]
const int by;
^
| } | ||
| }; | ||
|
|
||
| class BoundaryRegionIterY : public BoundaryRegionIter { |
There was a problem hiding this comment.
warning: destructor of 'BoundaryRegionIterY' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
class BoundaryRegionIterY : public BoundaryRegionIter {
^Additional context
include/bout/boundary_iterator.hxx:164: make it public and virtual
class BoundaryRegionIterY : public BoundaryRegionIter {
^
|
|
||
| bool contains(const int ix, const int iy, const int iz) const { | ||
| const auto i2 = xyz2ind(ix, iy, iz, localmesh); | ||
| for (auto i1 : bndry_points) { |
There was a problem hiding this comment.
warning: replace loop by 'std::any_of()' [readability-use-anyofallof]
for (auto i1 : bndry_points) {
^| return BoundaryRegionParIter(bndry_points, bndry_points.end(), dir, localmesh); | ||
| } | ||
|
|
||
| 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;
^| return BoundaryRegionParIter(bndry_points, bndry_points.end(), dir, localmesh); | ||
| } | ||
|
|
||
| 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;
^Makes it easier to reuse for other code
Mimicks the parallel case, to write FCI independent code
The boundary region does not match what is done for the parallel case, thus porting it to a iterator does not work.
hasBndryUpperY / hasBndryLowerY does not work for FCI and thus the request does not make sense / can be configured to throw. Thus it should not be checked if it is not needed.
BoundaryRegionIter has been delted
This allows to extend the boundary code to place the boundary further away from the boundary.
This is more general and takes the offset() into account, and thus works for cases where the boundary is between the first and second guard cell
Previously only the first boundary point was set
Taken from hermes-3, adopted for higher order
This allows to write code for FCI and non-FCI using templates.
Directly iterate over the points
dschwoerer
force-pushed
the
new-par-bc-unified
branch
from
e6eebff to
e9a0944
Compare
|
I rebased this, and applied clang-format to all commits, with respect to origin/next. I fail to see why clang-format in CI is not happy. I am using And this is the reason why I think having CI fail for missing clang-format is an issue. |
dschwoerer
force-pushed
the
new-par-bc-unified
branch
from
57fd55b to
c59626a
Compare
dschwoerer
force-pushed
the
new-par-bc-unified
branch
from
01fb37f to
7975518
Compare
| #include "bout/sys/range.hxx" | ||
| #include <functional> | ||
| #include <utility> | ||
|
|
There was a problem hiding this comment.
warning: included header utility is not used directly [misc-include-cleaner]
| #include <functional> | ||
| #include <utility> | ||
|
|
||
| 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 {
^| } | ||
|
|
||
| void limit_at_least(Field3D& f, BoutReal value) const { | ||
| ynext(f) = std::max(ynext(f), value); |
There was a problem hiding this comment.
warning: no header providing "std::max" is directly included [misc-include-cleaner]
include/bout/boundary_iterator.hxx:12:
- #include <functional>
+ #include <algorithm>
+ #include <functional>| } | ||
| }; | ||
|
|
||
| class BoundaryRegionIterY : public BoundaryRegionIter { |
There was a problem hiding this comment.
warning: class 'BoundaryRegionIterY' 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 BoundaryRegionIterY : public BoundaryRegionIter {
^
include/bout/boundary_iterator.hxx
Outdated
|
|
||
| class BoundaryRegionIterY : public BoundaryRegionIter { | ||
| public: | ||
| virtual ~BoundaryRegionIterY() = default; |
There was a problem hiding this comment.
warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [cppcoreguidelines-explicit-virtual-functions]
| virtual ~BoundaryRegionIterY() = default; | |
| ~BoundaryRegionIterY() override = default; |
|
|
||
| BoutReal extrapolate_sheath_free(const Field3D& f, SheathLimitMode mode) const { | ||
| const auto fac = valid() > 0 ? limitFreeScale(yprev(f), ythis(f), mode) | ||
| : (mode == SheathLimitMode::linear_free ? 0 : 1); |
There was a problem hiding this comment.
warning: conditional operator is used as sub-expression of parent conditional operator, refrain from using nested conditional operators [readability-avoid-nested-conditional-operator]
: (mode == SheathLimitMode::linear_free ? 0 : 1);
^Additional context
include/bout/parallel_boundary_region.hxx:293: parent conditional operator here
const auto fac = valid() > 0 ? limitFreeScale(yprev(f), ythis(f), mode)
^|
|
||
| void set_free(Field3D& f, SheathLimitMode mode) const { | ||
| const auto fac = valid() > 0 ? limitFreeScale(yprev(f), ythis(f), mode) | ||
| : (mode == SheathLimitMode::linear_free ? 0 : 1); |
There was a problem hiding this comment.
warning: conditional operator is used as sub-expression of parent conditional operator, refrain from using nested conditional operators [readability-avoid-nested-conditional-operator]
: (mode == SheathLimitMode::linear_free ? 0 : 1);
^Additional context
include/bout/parallel_boundary_region.hxx:301: parent conditional operator here
const auto fac = valid() > 0 ? limitFreeScale(yprev(f), ythis(f), mode)
^| boundary->add_point(x, y, z, x + dx, y + offset - (sgn(offset) * 0.5), | ||
| z + dz, // Intersection point in local index space | ||
| std::abs(offset) - 0.5, // Distance to intersection | ||
| defValid, offset); |
There was a problem hiding this comment.
warning: narrowing conversion from 'int' to signed type 'char' is implementation-defined [bugprone-narrowing-conversions]
defValid, offset);
^| boundary->add_point(x, y, z, x + dx, y + offset - (sgn(offset) * 0.5), | ||
| z + dz, // Intersection point in local index space | ||
| std::abs(offset) - 0.5, // Distance to intersection | ||
| defValid, offset); |
There was a problem hiding this comment.
warning: narrowing conversion from 'int' to signed type 'signed char' is implementation-defined [bugprone-narrowing-conversions]
defValid, offset);
^| bndry->setValid(0); | ||
| for (auto pnt : *bndry) { | ||
| for (auto pnt2 : *bndry2) { | ||
| #warning this could likely be done faster |
There was a problem hiding this comment.
warning: this could likely be done faster [clang-diagnostic-#warnings]
#warning this could likely be done faster
^
include/bout/yboundary_regions.hxx
Outdated
| }); | ||
| } | ||
|
|
||
| bool contains_ylow(Ind3D ind) const { return _contains[0][ind] } |
There was a problem hiding this comment.
warning: no header providing "Ind3D" is directly included [misc-include-cleaner]
include/bout/yboundary_regions.hxx:8:
- #include <memory>
+ #include "bout/region.hxx"
+ #include <memory>
| const bool inner_x = | ||
| options["inner_x"].doc("Boundary on inner x?").withDefault<bool>(false); | ||
|
|
||
| if (mesh->isFci()) { |
There was a problem hiding this comment.
warning: Called C++ object pointer is null [clang-analyzer-core.CallAndMessage]
if (mesh->isFci()) {
^Additional context
tests/integrated/test-fci-boundary/get_par_bndry.cxx:23: Assuming the condition is false
for (int i = 0; i < fields.size(); i++) {
^tests/integrated/test-fci-boundary/get_par_bndry.cxx:23: Loop condition is false. Execution continues on line 40
for (int i = 0; i < fields.size(); i++) {
^tests/integrated/test-fci-boundary/get_par_bndry.cxx:39: Assuming '__begin1' is equal to '__end1'
for (const auto& name : {"forward_xt_prime", "backward_xt_prime"}) {
^tests/integrated/test-fci-boundary/get_par_bndry.cxx:46: Value assigned to 'mesh'
Options dummy;
^tests/integrated/test-fci-boundary/get_par_bndry.cxx:48: Calling 'YBoundary::init'
ybndry.init(dummy, mesh);
^include/bout/yboundary_regions.hxx:47: Field 'is_init' is false
if (is_init) {
^include/bout/yboundary_regions.hxx:47: Taking false branch
if (is_init) {
^include/bout/yboundary_regions.hxx:53: Assuming pointer value is null
if (mesh == nullptr) {
^include/bout/yboundary_regions.hxx:53: Taking true branch
if (mesh == nullptr) {
^include/bout/yboundary_regions.hxx:54: Null pointer value stored to 'mesh'
mesh = bout::globals::mesh;
^include/bout/yboundary_regions.hxx:66: Called C++ object pointer is null
if (mesh->isFci()) {
^| }); | ||
| } | ||
|
|
||
| bool contains_ylow(Ind3D ind) const { return _contains[0][ind]; } |
There was a problem hiding this comment.
warning: no header providing "Ind3D" is directly included [misc-include-cleaner]
include/bout/yboundary_regions.hxx:9:
- #include <memory>
+ #include "bout/region.hxx"
+ #include <memory>| YBoundary ybndry; | ||
| ybndry.init(dummy, mesh); | ||
|
|
||
| std::vector<Field3D> fields(mesh->ystart * 2 + 1, Field3D{0.0}); |
There was a problem hiding this comment.
warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
| std::vector<Field3D> fields(mesh->ystart * 2 + 1, Field3D{0.0}); | |
| std::vector<Field3D> fields((mesh->ystart * 2) + 1, Field3D{0.0}); |
dschwoerer
force-pushed
the
new-par-bc-unified
branch
from
6bafccc to
52cb85a
Compare
| const bool inner_x = | ||
| options["inner_x"].doc("Boundary on inner x?").withDefault<bool>(false); | ||
|
|
||
| if (mesh->isFci()) { |
There was a problem hiding this comment.
warning: Called C++ object pointer is null [clang-analyzer-core.CallAndMessage]
if (mesh->isFci()) {
^Additional context
tests/integrated/test-fci-boundary/get_par_bndry.cxx:23: Assuming the condition is false
for (int i = 0; i < fields.size(); i++) {
^tests/integrated/test-fci-boundary/get_par_bndry.cxx:23: Loop condition is false. Execution continues on line 40
for (int i = 0; i < fields.size(); i++) {
^tests/integrated/test-fci-boundary/get_par_bndry.cxx:39: Assuming '__begin1' is equal to '__end1'
for (const auto& name : {"forward_xt_prime", "backward_xt_prime"}) {
^tests/integrated/test-fci-boundary/get_par_bndry.cxx:46: Value assigned to 'mesh'
Options dummy;
^tests/integrated/test-fci-boundary/get_par_bndry.cxx:48: Calling 'YBoundary::init'
ybndry.init(dummy, mesh);
^include/bout/yboundary_regions.hxx:48: Field 'is_init' is false
if (is_init) {
^include/bout/yboundary_regions.hxx:48: Taking false branch
if (is_init) {
^include/bout/yboundary_regions.hxx:54: Assuming pointer value is null
if (mesh == nullptr) {
^include/bout/yboundary_regions.hxx:54: Taking true branch
if (mesh == nullptr) {
^include/bout/yboundary_regions.hxx:55: Null pointer value stored to 'mesh'
mesh = bout::globals::mesh;
^include/bout/yboundary_regions.hxx:67: Called C++ object pointer is null
if (mesh->isFci()) {
^
|
|
||
| bool contains(const int ix, const int iy, const int iz) const { | ||
| const auto i2 = xyz2ind(ix, iy, iz, localmesh); | ||
| for (auto i1 : bndry_points) { |
There was a problem hiding this comment.
warning: replace loop by 'std::ranges::any_of()' [readability-use-anyofallof]
for (auto i1 : bndry_points) {
^1 participant
This allows to implement unified BCs for yup/ydown as well as FCI / FA.
Taking into acount things like whether the boundary is next to the domain, or between the parallel slices.