Add cell volume and areas by dschwoerer · Pull Request #3326 · boutproject/BOUT-dev

dschwoerer · 2026-03-13T12:41:11Z

They should make code more readable, and also potentially faster, as now cell_areas at the cell faces can be requested from the coordinates.

This should make the hermes-3 FA operator much more readable, and at the same time also FCI compatible, as the cell areas can be done correctly once for FCI, and then used transparently in user codes. @mikekryjak

Todo:

add some tests
~~use in FA operators~~ (needs FA operators from hermes-3)

Information like perpendicular J, Bxyz and g_22 are only known to the grid generator, but are needed for more correct parallel derivatives

_[A-Z].* is preserved for compilers, thus use _[a-z].*

At this time the parallel transform is not yet set, so isFci() return false.

github-actions

clang-tidy made some suggestions

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

github-actions · 2026-03-13T12:46:00Z

include/bout/coordinates.hxx

+    if (!_jxz_ylow.has_value()) {
+      _compute_Jxz_cell_faces();
+    }
+    return *_jxz_ylow;


warning: unchecked access to optional value [bugprone-unchecked-optional-access]

return *_jxz_ylow; ^

github-actions · 2026-03-13T12:46:00Z

include/bout/coordinates.hxx

+    if (!_jxz_yhigh.has_value()) {
+      _compute_Jxz_cell_faces();
+    }
+    return *_jxz_yhigh;


warning: unchecked access to optional value [bugprone-unchecked-optional-access]

return *_jxz_yhigh; ^

github-actions · 2026-03-13T12:46:00Z

include/bout/coordinates.hxx

+    if (!_jxz_centre.has_value()) {
+      _compute_Jxz_cell_faces();
+    }
+    return *_jxz_centre;


warning: unchecked access to optional value [bugprone-unchecked-optional-access]

return *_jxz_centre; ^

github-actions · 2026-03-13T12:46:00Z

include/bout/coordinates.hxx

+    if (!_jxz_ylow.has_value()) {
+      _compute_Jxz_cell_faces();
+    }
+    return *_jxz_ylow;


warning: unchecked access to optional value [bugprone-unchecked-optional-access]

return *_jxz_ylow; ^

github-actions · 2026-03-13T12:46:00Z

include/bout/coordinates.hxx

+    if (!_jxz_yhigh.has_value()) {
+      _compute_Jxz_cell_faces();
+    }
+    return *_jxz_yhigh;


warning: unchecked access to optional value [bugprone-unchecked-optional-access]

return *_jxz_yhigh; ^

github-actions · 2026-03-13T12:46:01Z

include/bout/coordinates.hxx

+  }
+
+private:
+  mutable std::optional<FieldMetric> _g_22_ylow, _g_22_yhigh;


warning: no header providing "std::optional" is directly included [misc-include-cleaner]

include/bout/coordinates.hxx:40:

+ #include <optional>

github-actions · 2026-03-13T12:46:01Z

src/mesh/coordinates.cxx

 const Coordinates::FieldMetric& Coordinates::invSg() const {
  if (invSgCache == nullptr) {
-    auto ptr = std::make_unique<FieldMetric>();
+    auto ptr = std::make_unique<Coordinates::FieldMetric>();


warning: no header providing "std::make_unique" is directly included [misc-include-cleaner]

auto ptr = std::make_unique<Coordinates::FieldMetric>(); ^

github-actions · 2026-03-13T12:46:01Z

src/mesh/coordinates.cxx

+  if (_g_22_ylow.has_value()) {
+    return *_g_22_ylow;
+  }
+  _g_22_ylow.emplace(emptyFrom(g_22));


warning: no header providing "emptyFrom" is directly included [misc-include-cleaner]

_g_22_ylow.emplace(emptyFrom(g_22)); ^

github-actions · 2026-03-13T12:46:01Z

src/mesh/coordinates.cxx

+  }
+  _g_22_ylow.emplace(emptyFrom(g_22));
+  //_g_22_ylow->setLocation(CELL_YLOW);
+  auto mesh = Bxy.getMesh();


warning: 'auto mesh' can be declared as 'auto *mesh' [readability-qualified-auto]

Suggested change

auto mesh = Bxy.getMesh();

auto *mesh = Bxy.getMesh();

src/mesh/coordinates.cxx

mikekryjak · 2026-03-13T13:59:26Z

@dschwoerer excellent work, this has been on my wishlist for a very long time. The amount of places we calculate the areas and volumes is considerable, and so is the number of potential mistakes this generates.

I have some questions on what is actually being calculated here. I see there are areas for each side of X, Y and Z. What about parallel areas?

What is Jxz? Is that just J?

ZedThree

Looks great, thanks @dschwoerer! A couple of suggestions:

ZedThree · 2026-03-13T14:13:33Z

include/bout/coordinates.hxx

+  const FieldMetric& Jxz_ylow() const {
+    if (!_jxz_ylow.has_value()) {
+      _compute_Jxz_cell_faces();
+    }
+    return *_jxz_ylow;


I think this clang-tidy check is unfortunately not clever enough. If _compute_Jxz_cell_faces() returns the computed value, this can actually just be

Suggested change

const FieldMetric& Jxz_ylow() const {

if (!_jxz_ylow.has_value()) {

_compute_Jxz_cell_faces();

}

return *_jxz_ylow;

const FieldMetric& Jxz_ylow() const {

return _jxz_low.value_or(_compute_Jxz_cell_faces());

Unfortunately _compute_Jxz_cell_faces() computes 3 different quantities.
We could split up the functions to do that, but that would add even more boiler plate code.

ZedThree · 2026-03-13T14:18:14Z

include/bout/coordinates.hxx

+  FieldMetric& g_22_ylow();
+  FieldMetric& g_22_yhigh();


Probably don't need these overloads?

We probably want to add the cell length, both in the cell centre, as well as at the faces, i.e. the distance between the cell centres.

That would replace g_22 then, right now they are still needed.

dschwoerer · 2026-03-16T08:01:31Z

@dschwoerer excellent work, this has been on my wishlist for a very long time. The amount of places we calculate the areas and volumes is considerable, and so is the number of potential mistakes this generates.

I have some questions on what is actually being calculated here. I see there are areas for each side of X, Y and Z. What about parallel areas?

What do you mean by "parallel" areas? Parallel to what? Or for the parallel slices?

What is Jxz? Is that just J?

No, that is the 2D jacobian in the x-z plane. Probably not needed anymore, we should use the areas directly.

github-actions

clang-tidy made some suggestions

src/mesh/coordinates.cxx

github-actions · 2026-03-16T09:54:30Z

src/mesh/coordinates.cxx

+  // metrics.
+  auto* mesh = Bxy.getMesh();
+  ASSERT0(mesh->xstart > 0);
+  BOUT_FOR(i, _jxz_centre->getRegion("RGN_NOX")) {


warning: unchecked access to optional value [bugprone-unchecked-optional-access]

BOUT_FOR(i, _jxz_centre->getRegion("RGN_NOX")) { ^

github-actions · 2026-03-16T09:54:30Z

src/mesh/coordinates.cxx

+
+void Coordinates::_compute_cell_area_y() const {
+  Jxz();
+  if (_jxz_centre->isFci()) {


warning: unchecked access to optional value [bugprone-unchecked-optional-access]

if (_jxz_centre->isFci()) { ^

github-actions · 2026-03-16T09:54:30Z

src/mesh/coordinates.cxx

+    ASSERT2(isUniform(dx, false, "RGN_ALL"));
+    ASSERT3(isUniform(dz, true, "RGN_ALL"));
+    ASSERT2(isUniform(dz, false, "RGN_ALL"));
+    _cell_area_ylow.emplace(*_jxz_ylow * dx * dz);


warning: unchecked access to optional value [bugprone-unchecked-optional-access]

_cell_area_ylow.emplace(*_jxz_ylow * dx * dz); ^

github-actions · 2026-03-16T09:54:30Z

src/mesh/coordinates.cxx

+    ASSERT3(isUniform(dz, true, "RGN_ALL"));
+    ASSERT2(isUniform(dz, false, "RGN_ALL"));
+    _cell_area_ylow.emplace(*_jxz_ylow * dx * dz);
+    _cell_area_yhigh.emplace(*_jxz_yhigh * dx * dz);


warning: unchecked access to optional value [bugprone-unchecked-optional-access]

_cell_area_yhigh.emplace(*_jxz_yhigh * dx * dz); ^

github-actions · 2026-03-16T09:54:30Z

src/mesh/coordinates.cxx

+    // metrics.
+    auto* mesh = Bxy.getMesh();
+    ASSERT0(mesh->ystart > 0);
+    BOUT_FOR(i, _jxz_centre->getRegion("RGN_NOY")) {


warning: unchecked access to optional value [bugprone-unchecked-optional-access]

BOUT_FOR(i, _jxz_centre->getRegion("RGN_NOY")) { ^

github-actions · 2026-03-16T09:54:30Z

src/mesh/coordinates.cxx

+  // We cannot setLocation, as that would trigger the computation of staggered
+  // metrics.
+  //ASSERT0(mesh->zstart > 0);
+  BOUT_FOR(i, _jxz_centre->getRegion("RGN_NOZ")) {


warning: unchecked access to optional value [bugprone-unchecked-optional-access]

BOUT_FOR(i, _jxz_centre->getRegion("RGN_NOZ")) { ^

dschwoerer added 13 commits March 13, 2026 13:38

Add metric info on cell faces for FCI

3f521d0

Information like perpendicular J, Bxyz and g_22 are only known to the grid generator, but are needed for more correct parallel derivatives

Do not used preserved names

3065f88

_[A-Z].* is preserved for compilers, thus use _[a-z].*

Add non-const versions

1c0e385

Add cell areas and cell volumes

5c28542

Fix formatting

078322d

Fix return statements

7547d9e

Remove duplicate line

01d776b

Remove non-used Jg()

13fdb8a

add asField3DParallel stub to Field2D

db66e35

Compile time fixes

cca09ac

Remove no-op check

a0d4954

At this time the parallel transform is not yet set, so isFci() return false.

Fixup compilation

aae5b5d

Switch to ASSERT3

63ab8b4

dschwoerer requested a review from ZedThree March 13, 2026 12:41

github-actions bot reviewed Mar 13, 2026

View reviewed changes

ZedThree reviewed Mar 13, 2026

View reviewed changes

dschwoerer added 2 commits March 16, 2026 09:07

Cleanup code

6b66188

Add unit tests for cell areas and volume

4c0c495

github-actions bot reviewed Mar 16, 2026

View reviewed changes

Add missing headers

fd12c54

Conversation

dschwoerer commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mikekryjak commented Mar 13, 2026

Uh oh!

ZedThree left a comment

Choose a reason for hiding this comment

Uh oh!

ZedThree Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

dschwoerer Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

ZedThree Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

dschwoerer Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

dschwoerer commented Mar 16, 2026

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

dschwoerer commented Mar 13, 2026 •

edited

Loading