Improvements for hermitespline interpolation by dschwoerer · Pull Request #3298 · boutproject/BOUT-dev

dschwoerer · 2026-03-03T13:02:07Z

Includes X-splitting also for monotonichermitespline and more.

For FCI we need to be able to access "random" data from the adjacent slices. If they are split in x-direction, this requires some tricky communication pattern. It can be used like this: ``` // Create object GlobalField3DAccess fci_comm(thismesh); // let it know what data points will be required: // where IndG3D is an index in the global field, which would be the // normal Ind3D if there would be only one proc. fci_comm.get(IndG3D(i, ny, nz)); // If all index have been added, the communication pattern will be // established. This has to be called by all processors in parallel fci_comm.setup() // Once the data for a given field is needed, it needs to be // communicated: GlobalField3DAccessInstance global_data = fci_comm.communicate(f3d); // and can be accessed like this BoutReal data = global_data[IndG3D(i, ny, nz)]; // ny and nz in the IndG3D are always optional. ```

If they are two instances of the same template, this allows to have an if in the inner loop that can be optimised out.

lower_bound takes into account the data is sorted

Ensures we all ways check for monotonicity

Tags were different for sender and receiver

Otherwise mpi might wait for the wrong request.

to avoid confusion whether the offsets are for sending or receiving

Using a local set for each thread ensures we do not need a mutex for adding data, at the cost of having to merge the different sets later.

We want to skip sending if there is no data for this process ...

The result needs to be well understood, as the indices are a mix of local and global indices, as we need to access non-local data.

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 · 2026-03-03T13:12:09Z

include/bout/interpolation_xz.hxx

+/// Perhaps should only impose near boundaries, since that is where
+/// problems most obviously occur.
+template <bool monotonic>
+class XZHermiteSplineBase : public XZInterpolation {


warning: class 'XZHermiteSplineBase' defines a non-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 XZHermiteSplineBase : public XZInterpolation { ^

github-actions · 2026-03-03T13:12:09Z

include/bout/interpolation_xz.hxx

  Tensor<SpecificInd<IND_TYPE::IND_3D>> i_corner; // index of bottom-left grid point
  Tensor<int> k_corner;                           // z-index of bottom-left grid point

+  std::unique_ptr<GlobalField3DAccess> gf3daccess;


warning: member variable 'gf3daccess' has protected visibility [cppcoreguidelines-non-private-member-variables-in-classes]

std::unique_ptr<GlobalField3DAccess> gf3daccess; ^

github-actions · 2026-03-03T13:12:09Z

include/bout/interpolation_xz.hxx

  Tensor<int> k_corner;                           // z-index of bottom-left grid point

+  std::unique_ptr<GlobalField3DAccess> gf3daccess;
+  Tensor<std::array<int, 4>> g3dinds;


warning: member variable 'g3dinds' has protected visibility [cppcoreguidelines-non-private-member-variables-in-classes]

Tensor<std::array<int, 4>> g3dinds; ^

include/bout/interpolation_xz.hxx

github-actions · 2026-03-03T13:12:09Z

include/bout/interpolation_xz.hxx

 #endif

+  /// Factors to allow for some wiggleroom
+  BoutReal abs_fac_monotonic{1e-2};


warning: member variable 'abs_fac_monotonic' has protected visibility [cppcoreguidelines-non-private-member-variables-in-classes]

BoutReal abs_fac_monotonic{1e-2}; ^

src/mesh/parallel/fci_comm.cxx

github-actions · 2026-03-03T13:12:10Z

src/mesh/parallel/fci_comm.hxx

+  int ind;
+};
+struct globalToLocal1D {
+  const int mg;


warning: member 'mg' of type 'const int' is const qualified [cppcoreguidelines-avoid-const-or-ref-data-members]

const int mg; ^

src/mesh/parallel/fci_comm.hxx

github-actions

clang-tidy made some suggestions

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

github-actions · 2026-03-04T09:25:27Z

src/mesh/interpolation/hermite_spline_xz.cxx


-#if USE_NEW_WEIGHTS
-#ifdef HS_USE_PETSC
+  std::unique_ptr<GlobalField3DAccessInstance> gf;


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

src/mesh/interpolation/hermite_spline_xz.cxx:30:

- #include <string> + #include <memory> + #include <string>

github-actions · 2026-03-04T09:25:27Z

src/mesh/interpolation/hermite_spline_xz.cxx


-#if USE_NEW_WEIGHTS
-#ifdef HS_USE_PETSC
+  std::unique_ptr<GlobalField3DAccessInstance> gf;


warning: variable 'gf' of type 'std::unique_ptr' can be declared 'const' [misc-const-correctness]

Suggested change

std::unique_ptr<GlobalField3DAccessInstance> gf;

std::unique_ptr<GlobalField3DAccessInstance> const gf;

github-actions · 2026-03-04T09:25:28Z

src/mesh/interpolation/hermite_spline_xz.cxx


+    if constexpr (monotonic) {
+#endif
+      const auto corners = {(*gf)[IndG3D(g3dinds[i][0])], (*gf)[IndG3D(g3dinds[i][1])],


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

const auto corners = {(*gf)[IndG3D(g3dinds[i][0])], (*gf)[IndG3D(g3dinds[i][1])], ^

src/mesh/interpolation/hermite_spline_xz.cxx

github-actions · 2026-03-04T09:25:28Z

src/mesh/interpolation/hermite_spline_xz.cxx

+      f_interp[iyp] = std::min(f_interp[iyp], minmax.second + diff);
+    }
+#if USE_NEW_WEIGHTS and defined(HS_USE_PETSC)
+    ASSERT2(std::isfinite(cptr[int(i)]));


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

ASSERT2(std::isfinite(cptr[int(i)])); ^

src/mesh/parallel/fci_comm.hxx

github-actions

clang-tidy made some suggestions

github-actions · 2026-03-04T10:45:46Z

include/bout/region.hxx

 // NOLINTEND(cppcoreguidelines-macro-usage,bugprone-macro-parentheses)

-enum class IND_TYPE { IND_3D = 0, IND_2D = 1, IND_PERP = 2 };
+enum class IND_TYPE { IND_3D = 0, IND_2D = 1, IND_PERP = 2, IND_GLOBAL_3D = 3 };


warning: enum 'IND_TYPE' uses a larger base type ('int', size: 4 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size]

enum class IND_TYPE { IND_3D = 0, IND_2D = 1, IND_PERP = 2, IND_GLOBAL_3D = 3 }; ^

github-actions · 2026-03-04T10:45:46Z

src/mesh/interpolation/hermite_spline_xz.cxx

 };

-XZHermiteSpline::XZHermiteSpline(int y_offset, Mesh* meshin)
+template <bool monotonic>


warning: constructor does not initialize these fields: petscWeights, rhs, result [cppcoreguidelines-pro-type-member-init]

include/bout/interpolation_xz.hxx:179:

- Mat petscWeights; - Vec rhs, result; + Mat petscWeights{}; + Vec rhs{}, result{};

github-actions · 2026-03-04T10:45:46Z

src/mesh/parallel/fci_comm.hxx

+    }
+    o_ids.clear();
+#endif
+    toGet.resize(static_cast<size_t>(g2lx.npe * g2ly.npe * g2lz.npe));


warning: either cast from 'int' to 'size_t' (aka 'unsigned long') is ineffective, or there is loss of precision before the conversion [bugprone-misplaced-widening-cast]

toGet.resize(static_cast<size_t>(g2lx.npe * g2ly.npe * g2lz.npe)); ^

github-actions · 2026-03-04T10:45:46Z

src/mesh/parallel/fci_comm.hxx

+      int offset = 0;
+      for (const auto& get : toGet) {
+        getOffsets.push_back(offset);
+        offset += get.size();


warning: narrowing conversion from 'size_type' (aka 'unsigned long') to signed type 'int' is implementation-defined [bugprone-narrowing-conversions]

offset += get.size(); ^

github-actions · 2026-03-04T10:45:46Z

src/mesh/parallel/fci_comm.hxx

+      auto it = std::lower_bound(vec.begin(), vec.end(), tofind);
+      ASSERT3(it != vec.end());
+      ASSERT3(*it == tofind);
+      mapping[id] = std::distance(vec.begin(), it) + getOffsets[proc];


warning: narrowing conversion from 'typename iterator_traits<__gnu_cxx::__normal_iterator<const int *, std::vector<int, std::allocator>>>::difference_type' (aka 'long') to signed type 'mapped_type' (aka 'int') is implementation-defined [bugprone-narrowing-conversions]

mapping[id] = std::distance(vec.begin(), it) + getOffsets[proc]; ^

github-actions · 2026-03-04T10:45:47Z

src/mesh/parallel/fci_comm.hxx

+
+public:
+  const fci_comm::XYZ2Ind<Ind3D> xyzl;
+  const fci_comm::XYZ2Ind<IndG3D> xyzg;


warning: member variable 'xyzg' has public visibility [cppcoreguidelines-non-private-member-variables-in-classes]

const fci_comm::XYZ2Ind<IndG3D> xyzg; ^

github-actions · 2026-03-04T10:45:47Z

src/mesh/parallel/fci_comm.hxx

+      }
+      auto ret =
+          MPI_Irecv(static_cast<void*>(data.data() + getOffsets[proc]),
+                    toGet[proc].size(), MPI_DOUBLE, proc, 666, comm, reqs.data() + cnt1);


warning: narrowing conversion from 'size_t' (aka 'unsigned long') to signed type 'int' is implementation-defined [bugprone-narrowing-conversions]

toGet[proc].size(), MPI_DOUBLE, proc, 666, comm, reqs.data() + cnt1); ^

github-actions · 2026-03-04T10:45:47Z

src/mesh/parallel/fci_comm.hxx

+      }
+      auto ret =
+          MPI_Irecv(static_cast<void*>(data.data() + getOffsets[proc]),
+                    toGet[proc].size(), MPI_DOUBLE, proc, 666, comm, reqs.data() + cnt1);


warning: narrowing conversion from 'size_type' (aka 'unsigned long') to signed type 'int' is implementation-defined [bugprone-narrowing-conversions]

toGet[proc].size(), MPI_DOUBLE, proc, 666, comm, reqs.data() + cnt1); ^

github-actions · 2026-03-04T10:45:47Z

src/mesh/parallel/fci_comm.hxx

+      for (auto i : toSend[proc]) {
+        sendBuffer[cnt++] = f[Ind3D(i)];
+      }
+      auto ret = MPI_Send(start, toSend[proc].size(), MPI_DOUBLE, proc, 666, comm);


warning: narrowing conversion from 'size_t' (aka 'unsigned long') to signed type 'int' is implementation-defined [bugprone-narrowing-conversions]

auto ret = MPI_Send(start, toSend[proc].size(), MPI_DOUBLE, proc, 666, comm); ^

github-actions · 2026-03-04T10:45:47Z

src/mesh/parallel/fci_comm.hxx

+      for (auto i : toSend[proc]) {
+        sendBuffer[cnt++] = f[Ind3D(i)];
+      }
+      auto ret = MPI_Send(start, toSend[proc].size(), MPI_DOUBLE, proc, 666, comm);


warning: narrowing conversion from 'size_type' (aka 'unsigned long') to signed type 'int' is implementation-defined [bugprone-narrowing-conversions]

auto ret = MPI_Send(start, toSend[proc].size(), MPI_DOUBLE, proc, 666, comm); ^

github-actions

clang-tidy made some suggestions

src/mesh/parallel/fci_comm.hxx

dschwoerer added 30 commits March 3, 2026 12:45

Allow XZHermiteSpline also without y-offset

5502ce3

Unify XZMonotonicHermiteSpline and XZMonotonicHermiteSpline

afc68d9

If they are two instances of the same template, this allows to have an if in the inner loop that can be optimised out.

Use x-splitting for monotonichermitespline test

84237c8

Set region for lagrange4pt

0662449

Add monotonic check also to other code branches

f77849e

use lower_bound instead of find

820f0a5

lower_bound takes into account the data is sorted

Do not shadow mapping

1178243

Ensure setup has been called

3d6f587

Use pointer to data

265c8cd

Call setup before communicator is used

16a162e

Deduplicate code

e5878c1

Ensures we all ways check for monotonicity

Fix tags for comm

aee2713

Tags were different for sender and receiver

Use pointer instead of std::vector

b4f7be5

Do not reuse requests if the array is still in use

578ee8d

Otherwise mpi might wait for the wrong request.

rename offset to getOffsets

bf9c9eb

to avoid confusion whether the offsets are for sending or receiving

Fix: mixup of sending / receiving size

2155f06

Fix receive data offset

0662247

Add check to ensure the proc layout is as expected

28d4e28

clang-format

e4507d9

Use BOUT++ assert

d7ba297

Fix check

f55f8e4

Take periodicity into account

0e9f7d6

Sort a reference, not a copy

944adfb

Only communicate non-empty vectors

6daec20

Make check stricter

3444dd0

Fix: include global offset in monotonic spline

91bee68

make fci_comm openmp thread safe

d7aeae4

Using a local set for each thread ensures we do not need a mutex for adding data, at the cost of having to merge the different sets later.

Fix communication for fci

ac5cc70

We want to skip sending if there is no data for this process ...

Add check to reduce risk of bugs

885b465

The result needs to be well understood, as the indices are a mix of local and global indices, as we need to access non-local data.

ZedThree and others added 6 commits March 3, 2026 12:58

tests: Increase nx for hermitespline interpolation boundary problem

a1c4033

region may be not used

1c25b0e

Fix [[maybe_unused]] location

c7f8504

Add some wiggle room for monotonichermitespline

96aa2bd

[bot] Apply format changes

65880e4

[bot] Add last format changes commit to ignore file

a91d3e7

github-actions bot reviewed Mar 3, 2026

View reviewed changes

Apply fixes from clang-format

f473a6b

github-actions bot reviewed Mar 4, 2026

View reviewed changes

dschwoerer added 2 commits March 4, 2026 11:20

Apply clang-tidy changes

f341037

Apply clang-tidy improvements

999d4ca

github-actions bot reviewed Mar 4, 2026

View reviewed changes

dschwoerer and others added 3 commits March 4, 2026 11:52

MPI_Request may not be a pointer

614a727

[bot] Apply format changes

d1cfb8a

[bot] Add last format changes commit to ignore file

5c0d41d

github-actions bot reviewed Mar 4, 2026

View reviewed changes

src/mesh/parallel/fci_comm.hxx Show resolved Hide resolved

	std::unique_ptr<GlobalField3DAccessInstance> gf;
	std::unique_ptr<GlobalField3DAccessInstance> const gf;

Conversation

dschwoerer commented Mar 3, 2026

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!