Split out `XZPetscHermiteSpline` #3223

ZedThree · 2025-12-03T17:54:04Z

Includes #3200 for testing

Split out XZPetscHermiteSpline as a separate implementation, this allows us to remove all the #ifdef mess
Move all the XZInterpolations into the more usual impls/ directory layout

- Include all relevant headers, remove unused ones, add some forward declarations - Make data `private` instead of `protected` - Add getter instead of `const` member - Change member reference to pointer - Move ctor to .cxx file - Use `std::array` instead of C array - Bunch of other small clang-tidy fixes

The Div_par operators use parallel slices of B -- with 2D metrics, these are just the field itself, in 3D we need the actual slices. While `Coordinates::geometry` does communicate the fields, it puts off calculating the parallel slices because that needs the fully constructed `Coordinates`. Upcoming changes should fix this and remove the need to explicitly communicate `Coordinates` members.

Co-authored-by: David Bold <dschwoerer@users.noreply.github.com>

This allows the user to control the clipping, allowing some overshoot Also restore the FCI MMS test with this interpolator

Includes bug fix: ```diff - BoutReal gL = n.c - n.L; - BoutReal gR = n.R - n.c; + BoutReal gL = n.c - n.m; + BoutReal gR = n.p - n.c; ```

Damp gradients of velocity rather than gradients of momentum

github-actions

clang-tidy made some suggestions

include/bout/interpolation_xz.hxx

src/mesh/difops.cxx

src/mesh/interpolation/xz/impls/hermite_spline_xz.hxx

github-actions

clang-tidy made some suggestions

github-actions · 2025-12-04T12:09:27Z

include/bout/interpolation_xz.hxx

 /// Interpolate a field onto a perturbed set of points
-const Field3D interpolate(const Field3D& f, const Field3D& delta_x,
-                          const Field3D& delta_z);
+Field3D interpolate(const Field3D& f, const Field3D& delta_x, const Field3D& delta_z);


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

include/bout/interpolation_xz.hxx:26:

- #include <bout/bout_types.hxx> + #include "bout/field3d.hxx" + #include <bout/bout_types.hxx>

github-actions · 2025-12-04T12:09:27Z

include/bout/interpolation_xz.hxx

-const Field3D interpolate(const Field2D& f, const Field3D& delta_x,
-                          const Field3D& delta_z);
-const Field3D interpolate(const Field2D& f, const Field3D& delta_x);
+Field3D interpolate(const Field2D& f, const Field3D& delta_x, const Field3D& delta_z);


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

include/bout/interpolation_xz.hxx:26:

- #include <bout/bout_types.hxx> + #include "bout/field2d.hxx" + #include <bout/bout_types.hxx>

github-actions

clang-tidy made some suggestions

include/bout/interpolation_xz.hxx

Also removes protected data

ZedThree · 2025-12-04T18:18:18Z

src/mesh/interpolation/xz/impls/petsc_hermite_spline_xz.cxx

+Field3D XZPetscHermiteSpline::interpolate(const Field3D& f,
+                                          const std::string& region) const {


@dschwoerer region isn't actually used here and I don't really understand why not. This technically makes this version behave differently to the other implementations. Should they actually all use RGN_NOY/YPAR_<N>?

It doesn't look like we ever actually call interpolate with a region anywhere, so perhaps it would be better to remove the argument and fix all the implementations?

@dschwoerer region isn't actually used here and I don't really understand why not.

Because I could not understand why any other region should ever be used.

This technically makes this version behave differently to the other implementations. Should they actually all use RGN_NOY/YPAR_<N>?

It doesn't look like we ever actually call interpolate with a region anywhere, so perhaps it would be better to remove the argument and fix all the implementations?

Yes, removing the argument is probably best.

I think the intention was we might be able to use the regions for things like embedded boundaries, but at the minute that isn't possible out of the box without changes to parallel transforms and communicate, so probably best to just remove the argument for now.

dschwoerer · 2025-12-05T07:57:56Z

I unified all the HermiteSpline interpolations, because they have a fair bit in common, it is much easier to keep the different versions aligned.

Maybe we should move some of the #ifdef to if constexpr + templates?

ZedThree · 2025-12-05T09:46:56Z

Ok, I can make XZHermiteSpline a base class of XZPetscHermiteSpline too to avoid the duplication of the basis calculations. We could also add XZPetscMonotonicHermiteSpline by simply doing the interpolation and clipping afterwards

ZedThree and others added 30 commits December 3, 2025 14:25

tests: Ruff fixes for FCI runtest

74b736d

tests: Expand FCI MMS test to Grad2_par2

eb589e1

tests: Generalise FCI MMS test to allow for more cases

c0cc84b

tests: Add test for FCI Div_par

2eda48c

tests: Make MMS script update input in-place

636bf28

tests: Add test for FCI Div_par_K_Grad_par

16318ec

tests: Add test for FCI Laplace_par

2f83692

Reduce duplication in FCI mms script

0cde140

Remove circular include

37dc273

tests: Add cases for FCI interpolation methods

9ac46e8

tests: Increase nx for hermitespline interpolation boundary problem

5974a6e

tests: Add monotonichermitespline, decrease resolution scan

6ab945e

tests: Small refactor of FCI tests

16c3718

Apply black changes

3d3ff65

Apply clang-format changes

79f9d0f

Apply clang-tidy fixes to FCI tests

cee3e44

Fix comment formatting

88d8db8

Apply suggestion from @dschwoerer

9d9922c

Co-authored-by: David Bold <dschwoerer@users.noreply.github.com>

tests: Fix typo that made tests always pass

e961e75

tests: Remove monotonichermitespline FCI case

2674a58

Add rtol, atol to MonotonicHermiteSpline

f00db39

This allows the user to control the clipping, allowing some overshoot Also restore the FCI MMS test with this interpolator

Add missing header

0efe4e3

Port FV::Div_par_mod from Hermes-3

e44fbb5

Port FV::Div_par_fvv from Hermes-3

db50278

Port Div_par_K_Grad_par_mod from Hermes-3

9a603da

Add MMS tests for finite volume operators

a5b8d5d

Add tests for all FV slope limiters

bae7618

Port Superbee finite volume limiter from Hermes-3

39ae2bc

Includes bug fix: ```diff - BoutReal gL = n.c - n.L; - BoutReal gR = n.R - n.c; + BoutReal gL = n.c - n.m; + BoutReal gR = n.p - n.c; ```

ZedThree added 6 commits December 3, 2025 14:35

Fix clang-tidy warnings for fv_ops

4cd7909

Apply ruff format

9749991

tests: Communicate some coordinates fields for 3D metrics

07ba3b8

Include all headers

7857a6b

Div_par_fvv: FCI change numerical dissipation

706f428

Damp gradients of velocity rather than gradients of momentum

Set expected order of FV::Div_par_fvv to 1

21cb28a

github-actions bot reviewed Dec 3, 2025

View reviewed changes

ZedThree added 4 commits December 4, 2025 09:43

Communicate some more coordinate quantities for FCI MMS test

972a19d

Guard against non-thread safe PETSc call

b6c1705

Remove some unused headers

e095708

Split XZ interpolation into usual impls layout

22b2589

ZedThree force-pushed the split-petsc-hermite-spline branch from ecedb55 to d6375f0 Compare December 4, 2025 12:00

github-actions bot reviewed Dec 4, 2025

View reviewed changes

ZedThree force-pushed the split-petsc-hermite-spline branch from 24b7c6a to 66b5f4b Compare December 4, 2025 13:34

github-actions bot reviewed Dec 4, 2025

View reviewed changes

include/bout/interpolation_xz.hxx Show resolved Hide resolved

ZedThree and others added 8 commits December 4, 2025 14:36

Split out XZPetscHermiteSpline

430a46f

Make member variable a local array

77c82bc

Clang-tidy fixes

94aae64

Remove duplication between hermite spline implementations

910f407

Also removes protected data

Fix clang-tidy issues with lagrange_4pt

09a1474

Fix interpolation/FCI tests for separate petschermitespline

3f80b86

Make petschermitespline the default if possible

24bc52b

Apply clang-format changes

aff19f0

ZedThree force-pushed the split-petsc-hermite-spline branch from 66b5f4b to aff19f0 Compare December 4, 2025 14:40

ZedThree commented Dec 4, 2025

View reviewed changes

		Field3D XZPetscHermiteSpline::interpolate(const Field3D& f,
		const std::string& region) const {

Split out XZPetscHermiteSpline #3223

Are you sure you want to change the base?

Split out XZPetscHermiteSpline #3223

Uh oh!

Conversation

ZedThree commented Dec 3, 2025

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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 Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ZedThree Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

dschwoerer Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

ZedThree Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

dschwoerer commented Dec 5, 2025

Uh oh!

ZedThree commented Dec 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Split out `XZPetscHermiteSpline` #3223

Split out `XZPetscHermiteSpline` #3223