Nonuniform BoundaryConditions by dschwoerer · Pull Request #1179 · boutproject/BOUT-dev

dschwoerer · 2018-07-18T22:55:54Z

Add boundaries which are aware of non-uniform grid spacing:

Dirichlet
Neumann
Free
for 2nd, 3rd and 4th each

~~Tests are not included, as they require realy~~

The code uses sympy to generate the C++ code, so extend should be easy.

* Dirichlet * Neumann * Free * for 2nd, 3rd and 4th each

Indexing starts at 0, but zero offset means that we are starting inside the boundary. Thus we need the +1 to only take points inside the domain.

johnomotani · 2018-10-22T18:19:41Z

Definitely agree there is an issue here.

I think from a skim of your code here that the solution you implement is taking into account explicitly the positions of each point in x-y space. That, if I follow these lecture notes correctly --- https://www.nada.kth.se/kurser/kth/2D1263/l6.pdf [see section 4.3, in particular eqs (4.2), (4.4) and figs 3.3 and 3.5-6] --- is the most accurate thing to do, but our derivative operators don't do that, they just use the local grid spacing at the location of the derivative (updating the non-uniform grid support in the derivatives could be a useful thing to do...?).

Using the local spacing at the boundary is a bit of a pain, especially with staggered fields (they end up needing the grid spacing at the x-y corner of cells to put the x-boundary condition on a y-staggered field or vice versa). I have put together some changes, building on other stuff I was doing to get difops to converge accurately near boundaries, that get the grid spacings at the boundary locations. See the derivatives3 test in e8b28bb, which is updated to have a non-constant grid spacing and to apply boundary conditions. With the updates, it converges correctly for dirichlet, neumann and free boundary conditions, on staggered and unstaggered grids. The cost compared to your approach is having to change a lot more stuff in Coordinates, essentially to get a calcGridSpacing in src/mesh/boundary_op.cxx to be possible. (Although many of the changes were to enable the 'x-boundary of y-staggered field' case which should work but I haven't actually managed to test yet.) Some of the changes I think could be good anyway, from a neatness and/or performance viewpoint, but that's a question for a different PR (the commit history and tests definitely need tidying up).

dschwoerer · 2018-11-07T10:22:27Z

Some timings: - based on 41382ba

dirichlet_o2:   10000 iterations took 0.776276 s
neumann_o2:     10000 iterations took 2.3407 s
free_o2:        10000 iterations took 1.56801 s
dirichlet_nu_o2:        10000 iterations took 0.613323 s
neumann_nu_o2:  10000 iterations took 0.614806 s
free_nu_o2:     10000 iterations took 0.649469 s
dirichlet_o3:   10000 iterations took 5.79157 s
free_o3:        10000 iterations took 1.76174 s
dirichlet_nu_o3:        10000 iterations took 0.792008 s
neumann_nu_o3:  10000 iterations took 0.78357 s
free_nu_o3:     10000 iterations took 0.846108 s
dirichlet_o4:   10000 iterations took 6.96777 s
dirichlet_nu_o4:        10000 iterations took 1.02474 s
neumann_nu_o4:  10000 iterations took 1.54527 s
free_nu_o4:     10000 iterations took 1.06533 s

Only dirichlet_o4 is slightly faster then the more general versions. Thus ether the general versions could be improved (Is #1351 making them faster then the non-uniform versions?) or the more general version could be used as a default. If there is no performance penalty - I think we should try to do "the right thing"

I thought of implementing some adaptive stencils - but haven't came around to do it. I thought of doing it as an aiolos-only feature, as the standard stencil code does not know the position of the derivative.

This reverts commit ef008b2.

required for recent removal of using

dschwoerer · 2019-02-11T19:30:06Z

This is ready to be merged - can someone have a look over this?

As this is to my understanding the correct implementation, thus we could discuss to replace the old, non-uniform mesh aware implementation, but I thought as a first step we could have it in parallel, to allow testing ...

ZedThree

Looking good. I'd like to get this in so we can base the boundary rewrite on it.

ZedThree · 2019-02-12T15:16:04Z

    return data[d.ind];
  }

+  BoutReal& operator[](const Indices &d) {


I'd like to avoid bringing these operators back if possible. If we added something like

// const Mesh& would be better, but I think Mesh const-ness might need fixing Ind3D indexFromXYZ(int x, int y, int z, Mesh* mesh)

would that avoid the need for Indices? If not, then I would suggest just moving Indices entirely into boundary_nonuniform.cxx and just making it POD:

struct Indices { int x, y, z; };

and explicitly indexing the fields with f(i.x, i.y, i.z)

I like Indices because they allow better readability, and are also not worse then integer indices performance wise. Further they allow to write code for Fields. Finally reintroducing them removes a breaking change from the 4.2(?) minor release.

I did not understand why they were removed in the first place, could you explain?

We removed them because we don't want to have to maintain many different ways of indexing fields. Also Indices was really an implementation detail of DataIterator. Now that we have improved on DataIterator it doesn't make sense to keep Indices about as well.

ZedThree · 2019-02-12T15:35:23Z

+    } else {
+      stag = 1;
+    }
+  }


These two conditionals are repeated in each apply. Can they be pulled out into free function(s)?

ZedThree · 2019-02-12T17:11:16Z

It would be really good if this included the tests as well. Can they be written without using realy?

johnomotani · 2019-02-24T18:31:31Z

@dschwoerer could you add a docstring explaining how the coefficients are calculated? I'd find that very helpful.

dschwoerer · 2019-02-25T08:39:54Z

Do you mean the derivation, starting with the taylor expansion, or rather explain the implementation?

johnomotani · 2019-02-25T09:30:35Z

If you could sketch the derivation, with notation corresponding to the variables in the code, that would be great 🥇

The test seems to be broken, as the convergence is not what it is supposed to be. This is most likely a bug in getting the correct dy and realy, which seems to limit the accuracy. with the realy() implementation, the test converged to more reasonable results, i.e. an o4 was never limited to first order convergence.

Do not generate code by default. The inversions take around 3 minutes to compute.

The differences are not that drastic that the code cannot be unified again.

github-actions

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 25 out of 101. Check the log or trigger a new build to see more.

github-actions · 2022-10-25T23:00:29Z

+  Field3D f{0.};
+  f.setBoundary("f");
+  Options* opt = Options::getRoot();
+  int ntests;


warning: variable 'ntests' is not initialized [cppcoreguidelines-init-variables]

Suggested change

int ntests;

int ntests = 0;

github-actions · 2022-10-25T23:00:29Z

  add(new BoundaryFree_O2(), "free_o2");
  add(new BoundaryFree_O3(), "free_o3");
-
+  add(new BoundaryDirichletNonUniform_O4(), "dirichlet_nu_o4");


warning: initializing non-owner argument of type 'BoundaryOp *' with a newly created 'gsl::owner<>' [cppcoreguidelines-owning-memory]

add(new BoundaryDirichletNonUniform_O4(), "dirichlet_nu_o4"); ^

github-actions · 2022-10-25T23:00:32Z

+
+#if !BOUT_USE_METRIC_3D
+
+void BoundaryDirichletNonUniform_O2::apply(Field3D& f, MAYBE_UNUSED(BoutReal t)) {


warning: function 'apply' has cognitive complexity of 39 (threshold 25) [readability-function-cognitive-complexity]

void BoundaryDirichletNonUniform_O2::apply(Field3D& f, MAYBE_UNUSED(BoutReal t)) { ^

src/mesh/boundary_nonuniform.cxx:51: +1, including nesting penalty of 0, nesting level increased to 1

if (!fg) ^

src/mesh/boundary_nonuniform.cxx:62: +1, including nesting penalty of 0, nesting level increased to 1

for (; !bndry->isDone(); bndry->next1d()) { ^

src/mesh/boundary_nonuniform.cxx:63: +2, including nesting penalty of 1, nesting level increased to 2

if (fg) { ^

src/mesh/boundary_nonuniform.cxx:75: +3, including nesting penalty of 2, nesting level increased to 3

for (int zk = 0; zk < mesh->LocalNz; zk++) { ^

src/mesh/boundary_nonuniform.cxx:84: +2, including nesting penalty of 1, nesting level increased to 2

bndry->by != 0 ? mesh->getCoordinates()->dy : mesh->getCoordinates()->dx; ^

src/mesh/boundary_nonuniform.cxx:86: +2, including nesting penalty of 1, nesting level increased to 2

if (stagger == 0) { ^

src/mesh/boundary_nonuniform.cxx:93: +1, nesting level increased to 2

} else { ^

src/mesh/boundary_nonuniform.cxx:97: +2, including nesting penalty of 1, nesting level increased to 2

if (stagger == -1) { ^

src/mesh/boundary_nonuniform.cxx:102: +2, including nesting penalty of 1, nesting level increased to 2

for (int i = ((stagger == -1) ? -1 : 0); i < bndry->width; i++) { ^

src/mesh/boundary_nonuniform.cxx:102: +3, including nesting penalty of 2, nesting level increased to 3

for (int i = ((stagger == -1) ? -1 : 0); i < bndry->width; i++) { ^

src/mesh/boundary_nonuniform.cxx:105: +3, including nesting penalty of 2, nesting level increased to 3

if (stagger == 0) { ^

src/mesh/boundary_nonuniform.cxx:110: +1, nesting level increased to 3

} else { ^

src/mesh/boundary_nonuniform.cxx:111: +4, including nesting penalty of 3, nesting level increased to 4

if (stagger == -1 && i != -1) { ^

src/mesh/boundary_nonuniform.cxx:111: +1

if (stagger == -1 && i != -1) { ^

src/mesh/boundary_nonuniform.cxx:115: +4, including nesting penalty of 3, nesting level increased to 4

if (stagger == 1) { ^

src/mesh/boundary_nonuniform.cxx:119: +3, including nesting penalty of 2, nesting level increased to 3

for (int iz = 0; iz < mesh->LocalNz; iz++) { ^

src/mesh/boundary_nonuniform.cxx:120: +4, including nesting penalty of 3, nesting level increased to 4

const BoutReal val = (fg) ? vals[iz] : 0.0; ^

github-actions · 2022-10-25T23:00:32Z

+  if (!fg)
+    fg = f.getBndryGenerator(bndry->location);


warning: statement should be inside braces [readability-braces-around-statements]

Suggested change

if (!fg)

fg = f.getBndryGenerator(bndry->location);

if (!fg) {

fg = f.getBndryGenerator(bndry->location);

}

github-actions · 2022-10-25T23:00:32Z

+      }
+    }
+
+    vec2 spacing;


warning: uninitialized record type: 'spacing' [cppcoreguidelines-pro-type-member-init]

Suggested change

vec2 spacing;

vec2 spacing{};

github-actions · 2022-10-25T23:00:32Z

+        bndry->by != 0 ? mesh->getCoordinates()->dy : mesh->getCoordinates()->dx;
+    Indices i1{bndry->x - 1 * bndry->bx, bndry->y - 1 * bndry->by, 0};
+    if (stagger == 0) {
+      BoutReal offset;


warning: variable 'offset' is not initialized [cppcoreguidelines-init-variables]

src/mesh/boundary_nonuniform.cxx:0:

- #include <bout/constants.hxx> + #include <math.h> + + #include <bout/constants.hxx>

Suggested change

BoutReal offset;

BoutReal offset = NAN;

github-actions · 2022-10-25T23:00:32Z

+      BoutReal total_offset = 0;
+      offset = coords_field(i1.x, i1.y);
+      spacing.f1 = total_offset + offset / 2;
+      total_offset += offset;


warning: Value stored to 'total_offset' is never read [clang-analyzer-deadcode.DeadStores]

total_offset += offset; ^

src/mesh/boundary_nonuniform.cxx:92: Value stored to 'total_offset' is never read

total_offset += offset; ^

Also use SpecificInd for data access

This avoids race conditions, as the rename in the end is atomic. Out-of-source generation doesn't help, as the copy in the end is not atomic and rename cannot be used in general.

github-actions

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 25 out of 72. Check the log or trigger a new build to see more.

github-actions · 2022-10-26T14:41:29Z

+
+#if !BOUT_USE_METRIC_3D
+using IndMetric = Ind2D;
+#define IND(var, x, y, z) IndMetric var(x* localNy + y, localNy, 1)


warning: function-like macro 'IND' used; consider a 'constexpr' template function [cppcoreguidelines-macro-usage]

#define IND(var, x, y, z) IndMetric var(x* localNy + y, localNy, 1) ^

I could use a jinja2 macros instead of a C macros. Would that be preferred? Certainly clang-tidy would be happy ...

github-actions · 2022-10-26T14:41:30Z

+
+#if !BOUT_USE_METRIC_3D
+using IndMetric = Ind2D;
+#define IND(var, x, y, z) IndMetric var(x* localNy + y, localNy, 1)


warning: macro argument should be enclosed in parentheses [bugprone-macro-parentheses]

Suggested change

#define IND(var, x, y, z) IndMetric var(x* localNy + y, localNy, 1)

#define IND(var, x, y, z) IndMetric var((x)* localNy + y, localNy, 1)

github-actions · 2022-10-26T14:41:30Z

+
+#if !BOUT_USE_METRIC_3D
+using IndMetric = Ind2D;
+#define IND(var, x, y, z) IndMetric var(x* localNy + y, localNy, 1)


warning: macro argument should be enclosed in parentheses [bugprone-macro-parentheses]

Suggested change

#define IND(var, x, y, z) IndMetric var(x* localNy + y, localNy, 1)

#define IND(var, x, y, z) IndMetric var(x* localNy + (y), localNy, 1)

github-actions · 2022-10-26T14:41:30Z

+#if !BOUT_USE_METRIC_3D
+using IndMetric = Ind2D;
+#define IND(var, x, y, z) IndMetric var(x* localNy + y, localNy, 1)
+#define IND3D(var, x, y, z) Ind3D var((x * localNy + y) * localNz + z, localNy, localNz)


warning: function-like macro 'IND3D' used; consider a 'constexpr' template function [cppcoreguidelines-macro-usage]

#define IND3D(var, x, y, z) Ind3D var((x * localNy + y) * localNz + z, localNy, localNz) ^

github-actions · 2022-10-26T14:41:30Z

+#if !BOUT_USE_METRIC_3D
+using IndMetric = Ind2D;
+#define IND(var, x, y, z) IndMetric var(x* localNy + y, localNy, 1)
+#define IND3D(var, x, y, z) Ind3D var((x * localNy + y) * localNz + z, localNy, localNz)


warning: macro argument should be enclosed in parentheses [bugprone-macro-parentheses]

Suggested change

#define IND3D(var, x, y, z) Ind3D var((x * localNy + y) * localNz + z, localNy, localNz)

#define IND3D(var, x, y, z) Ind3D var(((x) * localNy + y) * localNz + z, localNy, localNz)

github-actions · 2022-10-26T14:41:33Z

+
+      for (; !bndry->isDone(); bndry->next1d()) {
+
+        vec2 spacing;


warning: uninitialized record type: 'spacing' [cppcoreguidelines-pro-type-member-init]

Suggested change

vec2 spacing;

vec2 spacing{};

github-actions · 2022-10-26T14:41:33Z

+#else
+      Ind3D ic{temp3d + Ind3D(i * offset * localNz)};
+#endif
+            vec2 facs;


warning: uninitialized record type: 'facs' [cppcoreguidelines-pro-type-member-init]

Suggested change

vec2 facs;

vec2 facs{};

github-actions · 2022-10-26T14:41:34Z

+#if BOUT_USE_METRIC_3D
+#define MAKE3D(x) x
+#else
+#define MAKE3D(x) x##3d


warning: function-like macro 'MAKE3D' used; consider a 'constexpr' template function [cppcoreguidelines-macro-usage]

#define MAKE3D(x) x##3d ^

github-actions · 2022-10-26T14:41:34Z

+        return new BoundaryFreeNonUniform_O2(region, newgen);
+      }
+
+      vec2 BoundaryFreeNonUniform_O2::calc_interp_to_stencil(const vec2& spacing) const {


warning: method 'calc_interp_to_stencil' can be made static [readability-convert-member-functions-to-static]

Suggested change

vec2 BoundaryFreeNonUniform_O2::calc_interp_to_stencil(const vec2& spacing) const {

vec2 BoundaryFreeNonUniform_O2::calc_interp_to_stencil(const vec2& spacing) {

src/mesh/boundary_nonuniform.hxx:92:

- vec2 calc_interp_to_stencil(const vec2& spacing) const; + static vec2 calc_interp_to_stencil(const vec2& spacing) ;

github-actions · 2022-10-26T14:41:34Z

+      }
+
+      vec2 BoundaryFreeNonUniform_O2::calc_interp_to_stencil(const vec2& spacing) const {
+        vec2 facs;


warning: uninitialized record type: 'facs' [cppcoreguidelines-pro-type-member-init]

Suggested change

vec2 facs;

vec2 facs{};

github-actions

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 25 out of 47. Check the log or trigger a new build to see more.

github-actions · 2022-11-02T10:43:56Z

+        return facs;
+      }
+
+      void BoundaryDirichletNonUniform_O3::apply(Field3D & f, MAYBE_UNUSED(BoutReal t)) {


warning: function 'apply' has cognitive complexity of 39 (threshold 25) [readability-function-cognitive-complexity]

void BoundaryDirichletNonUniform_O3::apply(Field3D & f, MAYBE_UNUSED(BoutReal t)) { ^

src/mesh/boundary_nonuniform.cxx:446: +1, including nesting penalty of 0, nesting level increased to 1

if (!fg) { ^

src/mesh/boundary_nonuniform.cxx:458: +1, including nesting penalty of 0, nesting level increased to 1

for (; !bndry->isDone(); bndry->next1d()) { ^

src/mesh/boundary_nonuniform.cxx:459: +2, including nesting penalty of 1, nesting level increased to 2

if (fg) { ^

src/mesh/boundary_nonuniform.cxx:471: +3, including nesting penalty of 2, nesting level increased to 3

for (int zk = 0; zk < mesh->LocalNz; zk++) { ^

src/mesh/boundary_nonuniform.cxx:480: +2, including nesting penalty of 1, nesting level increased to 2

bndry->by != 0 ? mesh->getCoordinates()->dy : mesh->getCoordinates()->dx; ^

src/mesh/boundary_nonuniform.cxx:505: +2, including nesting penalty of 1, nesting level increased to 2

if (stagger == 0) { ^

src/mesh/boundary_nonuniform.cxx:513: +1, nesting level increased to 2

} else { ^

src/mesh/boundary_nonuniform.cxx:518: +2, including nesting penalty of 1, nesting level increased to 2

if (stagger == -1) { ^

src/mesh/boundary_nonuniform.cxx:533: +2, including nesting penalty of 1, nesting level increased to 2

for (int i = ((stagger == -1) ? -1 : 0); i < bndry->width; i++) { ^

src/mesh/boundary_nonuniform.cxx:533: +3, including nesting penalty of 2, nesting level increased to 3

for (int i = ((stagger == -1) ? -1 : 0); i < bndry->width; i++) { ^

src/mesh/boundary_nonuniform.cxx:542: +3, including nesting penalty of 2, nesting level increased to 3

if (stagger == 0) { ^

src/mesh/boundary_nonuniform.cxx:547: +1, nesting level increased to 3

} else { ^

src/mesh/boundary_nonuniform.cxx:548: +4, including nesting penalty of 3, nesting level increased to 4

if (stagger == -1 && i != -1) { ^

src/mesh/boundary_nonuniform.cxx:548: +1

if (stagger == -1 && i != -1) { ^

src/mesh/boundary_nonuniform.cxx:552: +4, including nesting penalty of 3, nesting level increased to 4

if (stagger == 1) { ^

src/mesh/boundary_nonuniform.cxx:557: +3, including nesting penalty of 2, nesting level increased to 3

for (int iz{0}; iz < mesh->LocalNz; iz++) { ^

src/mesh/boundary_nonuniform.cxx:564: +4, including nesting penalty of 3, nesting level increased to 4

const BoutReal val = (fg) ? vals[iz] : 0.0; ^

github-actions · 2022-11-02T10:43:56Z

+            }
+          }
+
+          vec3 spacing;


warning: uninitialized record type: 'spacing' [cppcoreguidelines-pro-type-member-init]

Suggested change

vec3 spacing;

vec3 spacing{};

github-actions · 2022-11-02T10:43:56Z

+#else
+      Ind3D ic{temp3d + Ind3D(i * offset * localNz)};
+#endif
+              vec3 facs;


warning: uninitialized record type: 'facs' [cppcoreguidelines-pro-type-member-init]

Suggested change

vec3 facs;

vec3 facs{};

github-actions · 2022-11-02T10:43:56Z

+#if BOUT_USE_METRIC_3D
+#define MAKE3D(x) x
+#else
+#define MAKE3D(x) x##3d


warning: function-like macro 'MAKE3D' used; consider a 'constexpr' template function [cppcoreguidelines-macro-usage]

#define MAKE3D(x) x##3d ^

github-actions · 2022-11-02T10:43:57Z

+          return new BoundaryDirichletNonUniform_O3(region, newgen);
+        }
+
+        vec3 BoundaryDirichletNonUniform_O3::calc_interp_to_stencil(const vec3& spacing)


warning: method 'calc_interp_to_stencil' can be made static [readability-convert-member-functions-to-static]

src/mesh/boundary_nonuniform.cxx:584:

- const { + {

src/mesh/boundary_nonuniform.hxx:111:

- vec3 calc_interp_to_stencil(const vec3& spacing) const; + static vec3 calc_interp_to_stencil(const vec3& spacing) ;

github-actions · 2022-11-02T10:43:59Z

+                  }
+                }
+
+                vec4 spacing;


warning: uninitialized record type: 'spacing' [cppcoreguidelines-pro-type-member-init]

Suggested change

vec4 spacing;

vec4 spacing{};

github-actions · 2022-11-02T10:43:59Z

+#else
+      Ind3D ic{temp3d + Ind3D(i * offset * localNz)};
+#endif
+                    vec4 facs;


warning: uninitialized record type: 'facs' [cppcoreguidelines-pro-type-member-init]

Suggested change

vec4 facs;

vec4 facs{};

github-actions · 2022-11-02T10:43:59Z

+#if BOUT_USE_METRIC_3D
+#define MAKE3D(x) x
+#else
+#define MAKE3D(x) x##3d


warning: function-like macro 'MAKE3D' used; consider a 'constexpr' template function [cppcoreguidelines-macro-usage]

#define MAKE3D(x) x##3d ^

github-actions · 2022-11-02T10:43:59Z

+                return new BoundaryDirichletNonUniform_O4(region, newgen);
+              }
+
+              vec4 BoundaryDirichletNonUniform_O4::calc_interp_to_stencil(


warning: method 'calc_interp_to_stencil' can be made static [readability-convert-member-functions-to-static]

src/mesh/boundary_nonuniform.cxx:1038:

- const vec4& spacing) const { + const vec4& spacing) {

src/mesh/boundary_nonuniform.hxx:168:

- vec4 calc_interp_to_stencil(const vec4& spacing) const; + static vec4 calc_interp_to_stencil(const vec4& spacing) ;

github-actions · 2022-11-02T10:43:59Z

+
+              vec4 BoundaryDirichletNonUniform_O4::calc_interp_to_stencil(
+                  const vec4& spacing) const {
+                vec4 facs;


warning: uninitialized record type: 'facs' [cppcoreguidelines-pro-type-member-init]

Suggested change

vec4 facs;

vec4 facs{};

Makes the C++ code simpler. The template does not change a lot, as the conditionals just change from C++ to jinja.

dschwoerer · 2022-11-03T11:48:09Z

Boundary condition	1000 ; 2D ; 128^3	1000 ; 2D ; 128x16384x1	1000 ; 3D ; 128^3
dirichlet_o2	0.353929 s	0.476291 s	0.375638 s
neumann_o2	0.442191 s	0.5878 s	0.856184 s
free_o2	0.217463 s	0.421346 s	0.243244 s
dirichlet_nu_o2	0.187466 s	1.97744 s	2.78296 s
neumann_nu_o2	0.177839 s	1.79646 s	2.10596 s
free_nu_o2	0.187784 s	2.74089 s	3.44364 s
dirichlet_o3	0.829212 s	0.785762 s	0.907647 s
free_o3	0.309709 s	0.984481 s	0.382664 s
dirichlet_nu_o3	0.251681 s	3.85593 s	5.58085 s
neumann_nu_o3	0.238788 s	2.94091 s	3.49203 s
free_nu_o3	0.287577 s	3.22091 s	4.60549 s
dirichlet_o4	1.09995 s	0.773845 s	1.07976 s
dirichlet_nu_o4	0.339356 s	3.487 s	4.87303 s
neumann_nu_o4	0.362675 s	4.04734 s	6.26752 s
free_nu_o4	0.386731 s	3.74004 s	5.49017 s

So for 2D and large enough nz the overhead of the correct stencil is minimal. Otherwise it can be expensive. Would be nice to know why 3D is slower, probably something that can be fixed.

github-actions

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 25 out of 77. Check the log or trigger a new build to see more.

github-actions · 2022-11-03T12:23:00Z

+            }
+          }
+
+          vec2 spacing;


warning: uninitialized record type: 'spacing' [cppcoreguidelines-pro-type-member-init]

Suggested change

vec2 spacing;

vec2 spacing{};

I would like to avoid the overhead of that ... (applies to all copies of the uninitialised warnings)

github-actions · 2022-11-03T12:23:04Z

+#if BOUT_USE_METRIC_3D
+#define MAKE3D(x) x
+#else
+#define MAKE3D(x) x##3d


warning: function-like macro 'MAKE3D' used; consider a 'constexpr' template function [cppcoreguidelines-macro-usage]

#define MAKE3D(x) x##3d ^

github-actions

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 25 out of 52. Check the log or trigger a new build to see more.

github-actions · 2022-11-03T12:48:47Z

+#if BOUT_USE_METRIC_3D
+#define MAKE3D(x) x
+#else
+#define MAKE3D(x) x##3d


warning: function-like macro 'MAKE3D' used; consider a 'constexpr' template function [cppcoreguidelines-macro-usage]

#define MAKE3D(x) x##3d ^

github-actions · 2022-11-03T12:48:48Z

+#if BOUT_USE_METRIC_3D
+#define MAKE3D(x) x
+#else
+#define MAKE3D(x) x##3d


warning: function-like macro 'MAKE3D' used; consider a 'constexpr' template function [cppcoreguidelines-macro-usage]

#define MAKE3D(x) x##3d ^

github-actions · 2022-11-03T12:48:50Z

+#if BOUT_USE_METRIC_3D
+#define MAKE3D(x) x
+#else
+#define MAKE3D(x) x##3d


warning: function-like macro 'MAKE3D' used; consider a 'constexpr' template function [cppcoreguidelines-macro-usage]

#define MAKE3D(x) x##3d ^

github-actions · 2022-11-03T12:48:51Z

+#if BOUT_USE_METRIC_3D
+#define MAKE3D(x) x
+#else
+#define MAKE3D(x) x##3d


warning: function-like macro 'MAKE3D' used; consider a 'constexpr' template function [cppcoreguidelines-macro-usage]

#define MAKE3D(x) x##3d ^

I doubt that can be replaced with a template function ...
Is it possible?

I guess something like this should work:

struct Make3D { const FieldMetric& fm; #if BOUT_USE_METRIC_3D const Field3D& f3d; #else const Field3D f3d; #endif Make3D(const FieldMetric& f) : fm(f), f3d(f) {}; }

dschwoerer · 2026-06-29T06:59:32Z

This is, as written, currently not compatible with 3D metrics. Fixing it requires some time, which I currently do not have.

dschwoerer added 3 commits July 18, 2018 22:20

Add Dirichlet 4th order for non-uniform BCs

920266b

More boundaries for non-uniform BC

acdbb93

* Dirichlet * Neumann * Free * for 2nd, 3rd and 4th each

Fix for Free interpolations

b784a15

Indexing starts at 0, but zero offset means that we are starting inside the boundary. Thus we need the +1 to only take points inside the domain.

dschwoerer changed the title ~~Nonuniform bc~~ Nonuniform BoundaryConditions Jul 18, 2018

Add O1 boundaries

ef008b2

dschwoerer mentioned this pull request Oct 22, 2018

BoundaryOp refactor #1334

Open

Example to time application of boundary conditions

5c560f4

Ajust tests

73081b6

johnomotani mentioned this pull request Nov 7, 2018

BoundaryOp refactor performance regression fix? #1351

Merged

Revert "Add O1 boundaries"

3d56703

This reverts commit ef008b2.

dschwoerer mentioned this pull request Nov 8, 2018

Remove deprecated routines #1326

Merged

dschwoerer added 5 commits November 8, 2018 16:46

Merge remote-tracking branch 'boutproject/next' into nonuniform-bc

d1ae0b2

Add Indices

c074cd7

Fix nonuniform boundaries

df54238

Fix: boundary-nonuniform

be1d6e7

required for recent removal of using

Merge remote-tracking branch 'boutproject/next' into nonuniform-bc

8e1aabc

dschwoerer mentioned this pull request Jan 18, 2019

test MMS/advection/weno3 is broken #1499

Closed

d7919 requested review from bendudson and johnomotani February 11, 2019 19:45

ZedThree requested changes Feb 12, 2019

View reviewed changes

johnomotani reviewed Feb 12, 2019

View reviewed changes

dschwoerer added the work in progress Not ready for merging label Feb 18, 2019

install sympy also for coverage test

336e25f

dschwoerer requested a review from ZedThree March 27, 2020 22:53

dschwoerer mentioned this pull request May 14, 2020

3D metrics #2025

Merged

11 tasks

dschwoerer and others added 10 commits July 15, 2021 14:09

Merge remote-tracking branch 'origin/next' into nonuniform-bc

970ea32

[skip ci] Apply black changes

8f41360

Merge remote-tracking branch 'origin/next' into nonuniform-bc-new

c6fdf29

Allow to specify output as argument

28fa564

Enable generating boundary_nonuniform.?xx

376ff26

Fix some minor issues

6a1b9d2

Do not generate code by default. The inversions take around 3 minutes to compute.

enable caching for stencils

868295b

Add basic 3D metric implementation

101b42c

The differences are not that drastic that the code cannot be unified again.

Update input and call to bc.options

ea15bf3

Apply clang-format changes

ab0d3a7

github-actions Bot reviewed Oct 25, 2022

View reviewed changes

dschwoerer added 2 commits October 26, 2022 16:35

Unify 2D and 3D code

0b66e8f

Also use SpecificInd for data access

Use random suffix for cmake

b1b74d0

This avoids race conditions, as the rename in the end is atomic. Out-of-source generation doesn't help, as the copy in the end is not atomic and rename cannot be used in general.

github-actions Bot reviewed Oct 26, 2022

View reviewed changes

github-actions Bot reviewed Nov 2, 2022

View reviewed changes

dschwoerer and others added 4 commits November 3, 2022 11:33

Split functions for different cases

16a0448

Makes the C++ code simpler. The template does not change a lot, as the conditionals just change from C++ to jinja.

Apply clang-format changes

534feb5

Apply suggetions from clang-tidy

54f8165

Merge remote-tracking branch 'origin/next' into nonuniform-bc

03594d6

dschwoerer added 2 commits November 3, 2022 13:09

Update generated code

9761f51

Fix test for new boutcore

dcd4a4d

github-actions Bot reviewed Nov 3, 2022

View reviewed changes

dschwoerer added the merge-conflicts PR with significant merge conflicts that need to be resolved label Oct 16, 2023

dschwoerer closed this Jun 29, 2026


		#if !BOUT_USE_METRIC_3D

		void BoundaryDirichletNonUniform_O2::apply(Field3D& f, MAYBE_UNUSED(BoutReal t)) {

	#define IND(var, x, y, z) IndMetric var(x* localNy + y, localNy, 1)
	#define IND(var, x, y, z) IndMetric var((x)* localNy + y, localNy, 1)

	#define IND3D(var, x, y, z) Ind3D var((x * localNy + y) * localNz + z, localNy, localNz)
	#define IND3D(var, x, y, z) Ind3D var(((x) * localNy + y) * localNz + z, localNy, localNz)

	vec2 BoundaryFreeNonUniform_O2::calc_interp_to_stencil(const vec2& spacing) const {
	vec2 BoundaryFreeNonUniform_O2::calc_interp_to_stencil(const vec2& spacing) {

Uh oh!

Conversation

dschwoerer commented Jul 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johnomotani commented Oct 22, 2018

Uh oh!

dschwoerer commented Nov 7, 2018

Uh oh!

dschwoerer commented Feb 11, 2019

Uh oh!

ZedThree left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ZedThree Feb 12, 2019

Choose a reason for hiding this comment

Uh oh!

dschwoerer Feb 12, 2019

Choose a reason for hiding this comment

Uh oh!

ZedThree Feb 13, 2019

Choose a reason for hiding this comment

Uh oh!

ZedThree Feb 12, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ZedThree commented Feb 12, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

johnomotani commented Feb 24, 2019

Uh oh!

dschwoerer commented Feb 25, 2019

Uh oh!

johnomotani commented Feb 25, 2019

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 25, 2022

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 25, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot Oct 25, 2022

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 25, 2022

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 25, 2022

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 25, 2022

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 25, 2022

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 26, 2022

Choose a reason for hiding this comment

Uh oh!

dschwoerer Oct 27, 2022

dschwoerer commented Jul 18, 2018 •

edited

Loading