Skip to content

Support GPU lazy field arithmetic operators#3385

Merged
bendudson merged 60 commits into
nextfrom
support-gpu-field-operators-lazy
Jun 20, 2026
Merged

Support GPU lazy field arithmetic operators#3385
bendudson merged 60 commits into
nextfrom
support-gpu-field-operators-lazy

Conversation

@bendudson

@bendudson bendudson commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Expressions involving fields build trees that are merged into kernels. Aims to achieve good GPU performance without needing a lot of user code changes.

This PR implements the template expression infrastructure (BinaryExpr) that fuses arithmetic operations on fields.

Next steps will include extending this to include differential operators.

Running elm-pb example on 1 CPU core, the output is identical between next and this branch. Oddly this branch seems to be slightly slower than next.

next:

1.000e+00         76       1.66e+00    68.1   20.5    0.1    0.8   10.5
2.000e+00         48       1.05e+00    67.8   20.5    0.1    1.0   10.6
3.000e+00         51       1.11e+00    67.8   20.6    0.1    0.9   10.6
4.000e+00         41       8.89e-01    68.0   20.6    0.1    1.1   10.2
5.000e+00         45       1.02e+00    68.7   20.1    0.1    1.0   10.0
6.000e+00         39       9.55e-01    68.8   19.7    0.2    1.2   10.1
...
3.900e+01         63       1.38e+00    68.1   20.4    0.1    0.7   10.6
4.000e+01         57       1.25e+00    68.0   20.4    0.1    0.8   10.6
Run time : 38 s

This branch:

1.000e+00         76       2.07e+00    74.7   16.2    0.1    0.6    8.3
2.000e+00         48       1.32e+00    74.6   16.1    0.1    0.8    8.4
3.000e+00         51       1.39e+00    74.6   16.2    0.1    0.7    8.4
4.000e+00         41       1.12e+00    74.7   16.2    0.1    0.9    8.1
5.000e+00         45       1.22e+00    74.8   16.2    0.1    0.8    8.1
6.000e+00         39       1.06e+00    74.7   16.2    0.1    0.9    8.0
...
3.900e+01         63       1.76e+00    74.6   16.2    0.1    0.6    8.4
4.000e+01         57       1.58e+00    74.6   16.1    0.1    0.7    8.5
Run time : 48 s

ggeorgakoudis and others added 30 commits February 25, 2025 14:00
- Compiles but crashes. I suspect it's because nested operators copy
Field3D on the device (rhs)
- Uses Views to avoid copying uncopyable stuff
- operatorors +=, -=, *=, /= are working
- Use functor template parameter for operation
- More operators
- Working version
macro for dedup definition per operator
- Add field functions (sqrt, abs, etc.)
- Add ResT template parameter to BinaryExpr for future use
- Update some operators (min, max, etc.) to take as input a BinaryExpr
  and evaluate it before apply
Most difficult conflicts were due to merging RAJA and Field3DParallel
changes, since these changes both modified the field operators and
functions on fields.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

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

Comment thread include/bout/field.hxx
namespace bout::op {
struct Square {
template <typename LView, typename RView>
BOUT_HOST_DEVICE BoutReal operator()(int idx, const LView& L, const RView&) const {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: all parameters should be named in a function [readability-named-parameter]

Suggested change
BOUT_HOST_DEVICE BoutReal operator()(int idx, const LView& L, const RView&) const {
> /*unused*/

Comment thread include/bout/field3d.hxx Outdated
BOUT_HOST_DEVICE inline BoutReal operator()(int idx) const {
return data[(idx * mul) / div];
}
BOUT_HOST_DEVICE inline BoutReal& operator[](int idx) const {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'operator[]' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]

Suggested change
BOUT_HOST_DEVICE inline BoutReal& operator[](int idx) const {
BOUT_HOST_DEVICE BoutReal& operator[](int idx) const {

Comment thread include/bout/field3d.hxx
}
};
operator View() { return View{&data[0]}; }
operator View() const { return View{const_cast<BoutReal*>(&data[0])}; }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not use const_cast to remove const qualifier [cppcoreguidelines-pro-type-const-cast]

  operator View() const { return View{const_cast<BoutReal*>(&data[0])}; }
                                      ^

Comment thread include/bout/field3d.hxx
Field3D& operator=(BoutReal val);
///@}
template <typename ResT, typename L, typename R, typename Func>
std::enable_if_t<is_expr_field3d_v<L>, Field3D&>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: operator=() should return 'Field3D&' [cppcoreguidelines-c-copy-assignment-signature]

  std::enable_if_t<is_expr_field3d_v<L>, Field3D&>
  ^

Comment thread include/bout/field3d.hxx
Field3D operator-(const Field3D& lhs, const Field3D& rhs);
Field3D operator*(const Field3D& lhs, const Field3D& rhs);
Field3D operator/(const Field3D& lhs, const Field3D& rhs);
#define FIELD3D_FIELD3D_FIELD3D_OP(OP_SYM, OP_TYPE) \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

#define FIELD3D_FIELD3D_FIELD3D_OP(OP_SYM, OP_TYPE)                                    \
        ^

Comment thread include/bout/fieldops.hxx

struct Add {
template <typename LView, typename RView>
BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal operator()(int idx, const LView& L,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal operator()(int idx, const LView& L,
                   ^

Comment thread include/bout/fieldops.hxx

BOUT_HOST_DEVICE BOUT_FORCEINLINE int size() const { return num_indices; }
BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal valueAtRegionPos(int idx) const {
return expr(indices[idx]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Array access (via field 'indices') results in a null pointer dereference [clang-analyzer-core.NullDereference]

    return expr(indices[idx]);
                ^
Additional context

src/invert/laplace/impls/naulin/naulin_laplace.cxx:230: Assuming the condition is true

  ASSERT1(rhs.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:230: Taking false branch

  ASSERT1(rhs.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:231: Assuming the condition is true

  ASSERT1(x0.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:231: Taking false branch

  ASSERT1(x0.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:232: Assuming the condition is true

  ASSERT1(Dcoef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:232: Taking false branch

  ASSERT1(Dcoef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:233: Assuming the condition is true

  ASSERT1(C1coef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:233: Taking false branch

  ASSERT1(C1coef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:234: Assuming the condition is true

  ASSERT1(C2coef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:234: Taking false branch

  ASSERT1(C2coef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:235: Assuming the condition is true

  ASSERT1(Acoef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:235: Taking false branch

  ASSERT1(Acoef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Assuming the condition is true

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Left side of '&&' is true

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
          ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Assuming the condition is true

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
                                        ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Taking false branch

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:318: Loop condition is true. Entering loop body

  while (true) {
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:322: Calling 'max<Field3D, Field3D, Field3D, bout::op::abs>'

    error_abs = max(abs(error3D, "RGN_NOBNDRY"), true, "RGN_NOBNDRY");
                ^

include/bout/field.hxx:473: 'reduction_view.indices' initialized to a null pointer value

;
    ^

include/bout/field.hxx:477: Calling 'reduceExpr<bout::reduce::Max, ReductionView<BinaryExpr<Field3D, Field3D, Field3D, bout::op::abs>::View>>'

=
                                    ^

include/bout/fieldops.hxx:282: Assuming the condition is true

ate;
                ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

include/bout/fieldops.hxx:282: Taking false branch

ate;
        ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

include/bout/fieldops.hxx:301: Loop condition is true. Entering loop body

ty();
        ^

include/bout/fieldops.hxx:302: Calling 'ReductionView::valueAtRegionPos'

+i) {
                                      ^

include/bout/fieldops.hxx:171: Array access (via field 'indices') results in a null pointer dereference

    return expr(indices[idx]);
                ^

Comment thread include/bout/fieldops.hxx

BOUT_HOST_DEVICE BOUT_FORCEINLINE int size() const { return num_indices; }
BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal valueAtRegionPos(int idx) const {
return expr(indices[idx]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Array access (via field 'indices') results in a null pointer dereference [clang-analyzer-core.NullDereference]

    return expr(indices[idx]);
                ^
Additional context

src/mesh/coordinates.cxx:1300: Loop condition is false. Execution continues on line 1325

  BOUT_FOR_SERIAL(i, g_11.getRegion(region)) {
  ^

include/bout/region.hxx:120: expanded from macro 'BOUT_FOR_SERIAL'

  for (auto block = (region).getBlocks().cbegin(), end = (region).getBlocks().cend(); \
  ^

src/mesh/coordinates.cxx:1325: Calling 'max<Field2D, Field2D, Field2D, bout::op::abs>'

  maxerr = BOUTMAX(max(abs(FieldMetric{(g_11 * g11 + g_12 * g12 + g_13 * g13) - 1})),
                   ^

include/bout/field.hxx:473: 'reduction_view.indices' initialized to a null pointer value

;
    ^

include/bout/field.hxx:477: Calling 'reduceExpr<bout::reduce::Max, ReductionView<BinaryExpr<Field2D, Field2D, Field2D, bout::op::abs>::View>>'

=
                                    ^

include/bout/fieldops.hxx:282: Assuming the condition is true

ate;
                ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

include/bout/fieldops.hxx:282: Taking false branch

ate;
        ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

include/bout/fieldops.hxx:301: Loop condition is true. Entering loop body

ty();
        ^

include/bout/fieldops.hxx:302: Calling 'ReductionView::valueAtRegionPos'

+i) {
                                      ^

include/bout/fieldops.hxx:171: Array access (via field 'indices') results in a null pointer dereference

    return expr(indices[idx]);
                ^

Comment thread include/bout/fieldops.hxx
template <typename ExprView>
ReductionView<ExprView> makeReductionView(const ExprView& expr,
const Array<int>& indices) {
return ReductionView<ExprView>{expr, indices.size() > 0 ? &indices[0] : nullptr,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]

Suggested change
return ReductionView<ExprView>{expr, indices.size() > 0 ? &indices[0] : nullptr,
return ReductionView<ExprView>{expr, !indices.empty() ? &indices[0] : nullptr,
Additional context

include/bout/array.hxx:277: method 'Array'::empty() defined here

  bool empty() const noexcept { return ptr == nullptr; }
       ^

Comment thread include/bout/fieldops.hxx
auto reduceExpr(const ExprView& expr_view) -> typename Reducer::State {
using State = typename Reducer::State;

ASSERT1(expr_view.size() > 0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

include/bout/fieldops.hxx:1:

- #ifndef BOUT_FIELDOPS_HXX
+ #include "bout/assert.hxx"
+ #ifndef BOUT_FIELDOPS_HXX

Caches the indicex array rather than recreating it each time.
Evaluates the expression to its result field type (ResT)
and stores that in the Option.
Binary expressions that take a boolean and branch
between two expressions.

`if_else_zero(bool, Expr)` evaluates an expression only if `bool` is
true, otherwise evaluates to zero.
If operating on a BinaryExpr, lazily evaluate field functions like
`abs`, `sin`, `tanh`. This avoids allocating a field and evaluating
the expression.
Mostly specifying when to force evaluation of expressions (BinaryExpr
trees) into fields.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

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

Comment thread include/bout/fieldops.hxx

BOUT_HOST_DEVICE BOUT_FORCEINLINE int size() const { return num_indices; }
BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal valueAtRegionPos(int idx) const {
return expr(indices[idx]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Array access (via field 'indices') results in a null pointer dereference [clang-analyzer-core.NullDereference]

    return expr(indices[idx]);
                ^
Additional context

src/invert/laplace/impls/naulin/naulin_laplace.cxx:230: Assuming the condition is true

  ASSERT1(rhs.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:230: Taking false branch

  ASSERT1(rhs.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:231: Assuming the condition is true

  ASSERT1(x0.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:231: Taking false branch

  ASSERT1(x0.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:232: Assuming the condition is true

  ASSERT1(Dcoef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:232: Taking false branch

  ASSERT1(Dcoef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:233: Assuming the condition is true

  ASSERT1(C1coef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:233: Taking false branch

  ASSERT1(C1coef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:234: Assuming the condition is true

  ASSERT1(C2coef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:234: Taking false branch

  ASSERT1(C2coef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:235: Assuming the condition is true

  ASSERT1(Acoef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:235: Taking false branch

  ASSERT1(Acoef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Assuming the condition is true

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Left side of '&&' is true

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
          ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Assuming the condition is true

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
                                        ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Taking false branch

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:318: Loop condition is true. Entering loop body

  while (true) {
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:322: Calling 'max<Field3D, Field3D, Field3D, bout::op::abs>'

    error_abs = max(abs(error3D, "RGN_NOBNDRY"), true, "RGN_NOBNDRY");
                ^

include/bout/field.hxx:473: 'reduction_view.indices' initialized to a null pointer value

;
    ^

include/bout/field.hxx:477: Calling 'reduceExpr<bout::reduce::Max, ReductionView<BinaryExpr<Field3D, Field3D, Field3D, bout::op::abs>::View>>'

=
                                    ^

include/bout/fieldops.hxx:294: Assuming the condition is true

ate;
                ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

include/bout/fieldops.hxx:294: Taking false branch

ate;
        ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

include/bout/fieldops.hxx:313: Loop condition is true. Entering loop body

ty();
        ^

include/bout/fieldops.hxx:314: Calling 'ReductionView::valueAtRegionPos'

+i) {
                                      ^

include/bout/fieldops.hxx:183: Array access (via field 'indices') results in a null pointer dereference

    return expr(indices[idx]);
                ^

Comment thread include/bout/fieldops.hxx

BOUT_HOST_DEVICE BOUT_FORCEINLINE int size() const { return num_indices; }
BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal valueAtRegionPos(int idx) const {
return expr(indices[idx]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Array access (via field 'indices') results in a null pointer dereference [clang-analyzer-core.NullDereference]

    return expr(indices[idx]);
                ^
Additional context

src/mesh/coordinates.cxx:1300: Loop condition is false. Execution continues on line 1325

  BOUT_FOR_SERIAL(i, g_11.getRegion(region)) {
  ^

include/bout/region.hxx:120: expanded from macro 'BOUT_FOR_SERIAL'

  for (auto block = (region).getBlocks().cbegin(), end = (region).getBlocks().cend(); \
  ^

src/mesh/coordinates.cxx:1325: Calling 'max<Field2D, Field2D, Field2D, bout::op::abs>'

  maxerr = BOUTMAX(max(abs(FieldMetric{(g_11 * g11 + g_12 * g12 + g_13 * g13) - 1})),
                   ^

include/bout/field.hxx:473: 'reduction_view.indices' initialized to a null pointer value

;
    ^

include/bout/field.hxx:477: Calling 'reduceExpr<bout::reduce::Max, ReductionView<BinaryExpr<Field2D, Field2D, Field2D, bout::op::abs>::View>>'

=
                                    ^

include/bout/fieldops.hxx:294: Assuming the condition is true

ate;
                ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

include/bout/fieldops.hxx:294: Taking false branch

ate;
        ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

include/bout/fieldops.hxx:313: Loop condition is true. Entering loop body

ty();
        ^

include/bout/fieldops.hxx:314: Calling 'ReductionView::valueAtRegionPos'

+i) {
                                      ^

include/bout/fieldops.hxx:183: Array access (via field 'indices') results in a null pointer dereference

    return expr(indices[idx]);
                ^

Comment thread include/bout/fieldops.hxx
}

template <typename ResT, typename L, typename R, typename Func>
struct BinaryExpr {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: class 'BinaryExpr' defines a copy assignment operator and a move assignment operator but does not define a destructor, a copy constructor or a move constructor [cppcoreguidelines-special-member-functions]

Func>
             ^

Comment thread include/bout/fieldops.hxx
Mesh* mesh;
CELL_LOC location = CELL_CENTRE;
DirectionTypes directions;
std::optional<size_t> regionID;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

include/bout/fieldops.hxx:1:

- #ifndef BOUT_FIELDOPS_HXX
+ #include <cstddef>
+ #ifndef BOUT_FIELDOPS_HXX

Comment thread include/bout/fieldops.hxx Outdated
template <typename IndType>
BinaryExpr(const typename L::View& lhs, const typename R::View& rhs, Func f, Mesh* mesh,
CELL_LOC location, DirectionTypes directions, std::optional<size_t> regionID,
const Region<IndType>& region)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no template named 'Region' [clang-diagnostic-error]

onID,
                         ^

template <typename T>
const T interp_to(const T& var, CELL_LOC loc, const std::string region = "RGN_ALL") {

std::enable_if_t<bout::utils::is_Field2D_v<T> || bout::utils::is_Field3D_v<T>, const T>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

include/bout/interpolation.hxx:30:

+ #include <type_traits>

Comment thread include/bout/interpolation.hxx Outdated
const T interp_to(const T& var, CELL_LOC loc, const std::string region = "RGN_ALL") {

std::enable_if_t<bout::utils::is_Field2D_v<T> || bout::utils::is_Field3D_v<T>, const T>
interp_to(const T& var, CELL_LOC loc, const std::string region = "RGN_ALL") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

interp_to(const T& var, CELL_LOC loc, const std::string region = "RGN_ALL") {
                        ^

Comment thread include/bout/interpolation.hxx Outdated
const T interp_to(const T& var, CELL_LOC loc, const std::string region = "RGN_ALL") {

std::enable_if_t<bout::utils::is_Field2D_v<T> || bout::utils::is_Field3D_v<T>, const T>
interp_to(const T& var, CELL_LOC loc, const std::string region = "RGN_ALL") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

include/bout/interpolation.hxx:30:

+ #include <string>

}

template <typename E>
std::enable_if_t<is_expr_field3d_v<E> && !bout::utils::is_Field3D_v<E>, const Field3D>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

include/bout/interpolation.hxx:28:

- #include "bout/mesh.hxx"
+ #include "bout/field3d.hxx"
+ #include "bout/mesh.hxx"

Comment thread include/bout/interpolation.hxx Outdated

template <typename E>
std::enable_if_t<is_expr_field3d_v<E> && !bout::utils::is_Field3D_v<E>, const Field3D>
interp_to(const E& expr, CELL_LOC loc, const std::string rgn = "RGN_ALL") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: the const qualified parameter 'rgn' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]

Suggested change
interp_to(const E& expr, CELL_LOC loc, const std::string rgn = "RGN_ALL") {
interp_to(const E& expr, CELL_LOC loc, const std::string& rgn = "RGN_ALL") {

- Replaced `auto` with `Field3D` in a couple of places, to force
  evaluation of a BinaryExpr.
- Removed some unnecessary intermediates, now that `abs` and `max`
  operate on BinaryExpr.
- Clang tidy changes to headers etc.

Integrated tests are now compiling locally for 2D and 3D metrics.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

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

Comment thread include/bout/fieldops.hxx

BOUT_HOST_DEVICE BOUT_FORCEINLINE int size() const { return num_indices; }
BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal valueAtRegionPos(int idx) const {
return expr(indices[idx]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Array access (via field 'indices') results in a null pointer dereference [clang-analyzer-core.NullDereference]

    return expr(indices[idx]);
                ^
Additional context

src/invert/laplace/impls/naulin/naulin_laplace.cxx:230: Assuming the condition is true

  ASSERT1(rhs.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:230: Taking false branch

  ASSERT1(rhs.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:231: Assuming the condition is true

  ASSERT1(x0.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:231: Taking false branch

  ASSERT1(x0.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:232: Assuming the condition is true

  ASSERT1(Dcoef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:232: Taking false branch

  ASSERT1(Dcoef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:233: Assuming the condition is true

  ASSERT1(C1coef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:233: Taking false branch

  ASSERT1(C1coef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:234: Assuming the condition is true

  ASSERT1(C2coef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:234: Taking false branch

  ASSERT1(C2coef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:235: Assuming the condition is true

  ASSERT1(Acoef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:235: Taking false branch

  ASSERT1(Acoef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Assuming the condition is true

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Left side of '&&' is true

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
          ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Assuming the condition is true

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
                                        ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Taking false branch

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:276: Calling 'mean<Field3D, Field3D, Field3D, bout::op::Square>'

  BoutReal RMS_rhsOverD = sqrt(mean(
                               ^

include/bout/field.hxx:526: 'reduction_view.indices' initialized to a null pointer value

;
    ^

include/bout/field.hxx:529: Calling 'reduceExpr<bout::reduce::Mean, ReductionView<BinaryExpr<Field3D, Field3D, Field3D, bout::op::Square>::View>>'

;
                 ^

include/bout/fieldops.hxx:292: Assuming the condition is true

e;
              ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

include/bout/fieldops.hxx:292: Taking false branch

e;
      ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

include/bout/fieldops.hxx:311: Loop condition is true. Entering loop body

();
      ^

include/bout/fieldops.hxx:312: Calling 'ReductionView::valueAtRegionPos'

) {
                                    ^

include/bout/fieldops.hxx:187: Array access (via field 'indices') results in a null pointer dereference

    return expr(indices[idx]);
                ^

Comment thread include/bout/fieldops.hxx
}

template <typename ResT, typename L, typename R, typename Func>
struct BinaryExpr {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: class 'BinaryExpr' defines a copy assignment operator and a move assignment operator but does not define a destructor, a copy constructor or a move constructor [cppcoreguidelines-special-member-functions]

nc>
           ^

Comment thread include/bout/fieldops.hxx Outdated
BinaryExpr& operator=(const BinaryExpr&) = delete;
BinaryExpr& operator=(BinaryExpr&&) = delete;

inline int size() const { return indices.size(); }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'size' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]

Suggested change
inline int size() const { return indices.size(); }
e;

Comment thread include/bout/fieldops.hxx Outdated
BinaryExpr& operator=(BinaryExpr&&) = delete;

inline int size() const { return indices.size(); }
inline BoutReal operator()(int idx) const {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'operator()' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]

Suggested change
inline BoutReal operator()(int idx) const {
; }

Comment thread include/bout/fieldops.hxx Outdated
return operator()(d.ind);
}
}
inline int regionIdx(int idx) const { return indices[idx]; }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'regionIdx' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]

Suggested change
inline int regionIdx(int idx) const { return indices[idx]; }
}

Comment thread include/bout/twiddle.hxx
{0.9807852804032303, 0.1950903220161287}, // k=31
};

__constant__ double2 c_twiddle_64[64] = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: unknown type name 'constant' [clang-diagnostic-error]

__constant__ double2 c_twiddle_64[64] = {
^

Comment thread include/bout/twiddle.hxx
{0.9807852804032303, 0.1950903220161287}, // k=31
};

__constant__ double2 c_twiddle_64[64] = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'double2' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

__constant__ double2 c_twiddle_64[64] = {
             ^

Comment thread include/bout/twiddle.hxx
{0.9951847266721969, 0.0980171403295605}, // k=63
};

__constant__ double2 c_twiddle_128[128] = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: expected ';' after top level declarator [clang-diagnostic-error]

Suggested change
__constant__ double2 c_twiddle_128[128] = {
__constant__ double2; c_twiddle_128[128] = {

Comment thread include/bout/twiddle.hxx
{0.9951847266721969, 0.0980171403295605}, // k=63
};

__constant__ double2 c_twiddle_128[128] = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: unknown type name 'constant' [clang-diagnostic-error]

__constant__ double2 c_twiddle_128[128] = {
^

Comment thread include/bout/twiddle.hxx
{0.9951847266721969, 0.0980171403295605}, // k=63
};

__constant__ double2 c_twiddle_128[128] = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'double2' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

__constant__ double2 c_twiddle_128[128] = {
             ^

The data arrays should be the full size of the field,
not the size of the expression domain.
Force evaluation of BinaryExpr by assigning to a FieldMetric.
strLocation doesn't seem to be used and may return pointer
to temporary string.

Added missing headers.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

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

Comment thread include/bout/fieldops.hxx

BOUT_HOST_DEVICE BOUT_FORCEINLINE int size() const { return num_indices; }
BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal valueAtRegionPos(int idx) const {
return expr(indices[idx]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Array access (via field 'indices') results in a null pointer dereference [clang-analyzer-core.NullDereference]

    return expr(indices[idx]);
                ^
Additional context

src/mesh/coordinates.cxx:1301: Loop condition is false. Execution continues on line 1326

  BOUT_FOR_SERIAL(i, g_11.getRegion(region)) {
  ^

include/bout/region.hxx:120: expanded from macro 'BOUT_FOR_SERIAL'

  for (auto block = (region).getBlocks().cbegin(), end = (region).getBlocks().cend(); \
  ^

src/mesh/coordinates.cxx:1326: Calling 'max<Field2D, Field2D, Field2D, bout::op::abs>'

  maxerr = BOUTMAX(max(abs(FieldMetric{(g_11 * g11 + g_12 * g12 + g_13 * g13) - 1})),
                   ^

include/bout/field.hxx:473: 'reduction_view.indices' initialized to a null pointer value

;
    ^

include/bout/field.hxx:477: Calling 'reduceExpr<bout::reduce::Max, ReductionView<BinaryExpr<Field2D, Field2D, Field2D, bout::op::abs>::View>>'

=
                                    ^

include/bout/fieldops.hxx:292: Assuming the condition is true

e;
              ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

include/bout/fieldops.hxx:292: Taking false branch

e;
      ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

include/bout/fieldops.hxx:311: Loop condition is true. Entering loop body

();
      ^

include/bout/fieldops.hxx:312: Calling 'ReductionView::valueAtRegionPos'

) {
                                    ^

include/bout/fieldops.hxx:187: Array access (via field 'indices') results in a null pointer dereference

    return expr(indices[idx]);
                ^

Comment thread include/bout/twiddle.hxx
{0.9987954562051724, 0.0490676743274181}, // k=127
};

__constant__ double2 c_twiddle_256[256] = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: expected ';' after top level declarator [clang-diagnostic-error]

Suggested change
__constant__ double2 c_twiddle_256[256] = {
__constant__ double2; c_twiddle_256[256] = {

Comment thread include/bout/twiddle.hxx
{0.9987954562051724, 0.0490676743274181}, // k=127
};

__constant__ double2 c_twiddle_256[256] = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: unknown type name 'constant' [clang-diagnostic-error]

__constant__ double2 c_twiddle_256[256] = {
^

Comment thread include/bout/twiddle.hxx
{0.9987954562051724, 0.0490676743274181}, // k=127
};

__constant__ double2 c_twiddle_256[256] = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'double2' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

__constant__ double2 c_twiddle_256[256] = {
             ^

Comment thread include/bout/twiddle.hxx
{0.9996988186962042, 0.0245412285229124}, // k=255
};

__constant__ double2 c_twiddle_512[512] = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: expected ';' after top level declarator [clang-diagnostic-error]

Suggested change
__constant__ double2 c_twiddle_512[512] = {
__constant__ double2; c_twiddle_512[512] = {

data[stripe_size * ind.ind + static_cast<int>(Offset::Byup)] =
coords->Bxy.yup()[ind];
if (coords->Bxy.ydown().isAllocated())
data[stripe_size * ind.ind + static_cast<int>(Offset::Bydown)] =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]

Suggested change
data[stripe_size * ind.ind + static_cast<int>(Offset::Bydown)] =
data[(stripe_size * ind.ind) + static_cast<int>(Offset::Bydown)] =

Comment thread src/mesh/difops.cxx

if (!f.hasParallelSlices()) {
return metric->Bxy * FDDY(v, f / Bxy_floc, outloc, method) / sqrt(metric->g_22);
Field3D f_B = f / Bxy_floc;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'f_B' of type 'Field3D' can be declared 'const' [misc-const-correctness]

Suggested change
Field3D f_B = f / Bxy_floc;
Field3D const f_B = f / Bxy_floc;

#include <bout/output.hxx>
#include <bout/unused.hxx>
#include <bout/utils.hxx>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: included header utils.hxx is not used directly [misc-include-cleaner]

Suggested change


#include <cmath>

#if BOUT_HAS_CUDA

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

src/mesh/parallel/shiftedmetric.cxx:8:

- #include "bout/parallel_boundary_region.hxx"
+ #include "bout/build_defines.hxx"
+ #include "bout/parallel_boundary_region.hxx"

template <typename T,
typename = std::enable_if_t<bout::utils::is_Field_v<std::decay_t<T>>>>
auto evaluateFieldExpr(const T& field) -> const T& {
return field;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: returning a constant reference parameter may cause use-after-free when the parameter is constructed from a temporary [bugprone-return-const-ref-from-parameter]

  return field;
         ^

Assigning an expression to a field should update the field
metadata (location, Y slice for FieldPerp) to the value
from the BinaryExpr.

Unit tests now pass locally with CHECK=4.
@bendudson bendudson changed the title WIP: Support GPU lazy field operators Support GPU lazy field arithmetic operators Jun 19, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

Comment thread include/bout/field.hxx
namespace bout::detail {
template <typename T>
std::optional<int> getPerpYIndex(const T& value) {
if constexpr (std::is_same_v<std::decay_t<T>, ::FieldPerp>) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

include/bout/field.hxx:26:

- class Field;
+ #include <type_traits>
+ class Field;

Comment thread include/bout/field2d.hxx
Field2D& asField3DParallel() { return *this; }
const Field2D& asField3DParallel() const { return *this; }

struct View {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: constructor does not initialize these fields: data [cppcoreguidelines-pro-type-member-init]

include/bout/field2d.hxx:331:

-     BoutReal* data;
+     BoutReal* data{};

Comment thread include/bout/field2d.hxx
int mul = 1;
int div = 1;
BOUT_HOST_DEVICE inline BoutReal operator()(int idx) const {
return data[(idx * mul / div)];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Array access (via field 'data') results in a null pointer dereference [clang-analyzer-core.NullDereference]

      return data[(idx * mul / div)];
             ^
Additional context

include/bout/field2d.hxx:351: Null pointer value stored to field 'data'

  BOUT_DEVICE inline BoutReal operator()(int i) const { return View()(i); }
                                                               ^

include/bout/field2d.hxx:351: Calling 'View::operator()'

  BOUT_DEVICE inline BoutReal operator()(int i) const { return View()(i); }
                                                               ^

include/bout/field2d.hxx:335: Array access (via field 'data') results in a null pointer dereference

      return data[(idx * mul / div)];
             ^

Comment thread include/bout/fieldops.hxx

BOUT_HOST_DEVICE BOUT_FORCEINLINE int size() const { return num_indices; }
BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal valueAtRegionPos(int idx) const {
return expr(indices[idx]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Array access (via field 'indices') results in a null pointer dereference [clang-analyzer-core.NullDereference]

    return expr(indices[idx]);
                ^
Additional context

src/invert/laplace/impls/naulin/naulin_laplace.cxx:230: Assuming the condition is true

  ASSERT1(rhs.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:230: Taking false branch

  ASSERT1(rhs.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:231: Assuming the condition is true

  ASSERT1(x0.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:231: Taking false branch

  ASSERT1(x0.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:232: Assuming the condition is true

  ASSERT1(Dcoef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:232: Taking false branch

  ASSERT1(Dcoef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:233: Assuming the condition is true

  ASSERT1(C1coef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:233: Taking false branch

  ASSERT1(C1coef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:234: Assuming the condition is true

  ASSERT1(C2coef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:234: Taking false branch

  ASSERT1(C2coef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:235: Assuming the condition is true

  ASSERT1(Acoef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:235: Taking false branch

  ASSERT1(Acoef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Assuming the condition is true

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Left side of '&&' is true

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
          ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Assuming the condition is true

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
                                        ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Taking false branch

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:276: Calling 'mean<Field3D, Field3D, Field3D, bout::op::Square>'

  BoutReal RMS_rhsOverD = sqrt(mean(
                               ^

include/bout/field.hxx:526: 'reduction_view.indices' initialized to a null pointer value

;
    ^

include/bout/field.hxx:529: Calling 'reduceExpr<bout::reduce::Mean, ReductionView<BinaryExpr<Field3D, Field3D, Field3D, bout::op::Square>::View>>'

;
                 ^

include/bout/fieldops.hxx:293: Assuming the condition is true

e;
              ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

include/bout/fieldops.hxx:293: Taking false branch

e;
      ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

include/bout/fieldops.hxx:312: Loop condition is true. Entering loop body

();
      ^

include/bout/fieldops.hxx:313: Calling 'ReductionView::valueAtRegionPos'

) {
                                    ^

include/bout/fieldops.hxx:188: Array access (via field 'indices') results in a null pointer dereference

    return expr(indices[idx]);
                ^

Comment thread include/bout/fieldops.hxx

BOUT_HOST_DEVICE BOUT_FORCEINLINE int size() const { return num_indices; }
BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal valueAtRegionPos(int idx) const {
return expr(indices[idx]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Array access (via field 'indices') results in a null pointer dereference [clang-analyzer-core.NullDereference]

    return expr(indices[idx]);
                ^
Additional context

src/mesh/coordinates.cxx:1301: Loop condition is false. Execution continues on line 1326

  BOUT_FOR_SERIAL(i, g_11.getRegion(region)) {
  ^

include/bout/region.hxx:120: expanded from macro 'BOUT_FOR_SERIAL'

  for (auto block = (region).getBlocks().cbegin(), end = (region).getBlocks().cend(); \
  ^

src/mesh/coordinates.cxx:1326: Calling 'max<Field2D, Field2D, Field2D, bout::op::abs>'

  maxerr = BOUTMAX(max(abs(FieldMetric{(g_11 * g11 + g_12 * g12 + g_13 * g13) - 1})),
                   ^

include/bout/field.hxx:473: 'reduction_view.indices' initialized to a null pointer value

;
    ^

include/bout/field.hxx:477: Calling 'reduceExpr<bout::reduce::Max, ReductionView<BinaryExpr<Field2D, Field2D, Field2D, bout::op::abs>::View>>'

=
                                    ^

include/bout/fieldops.hxx:293: Assuming the condition is true

e;
              ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

include/bout/fieldops.hxx:293: Taking false branch

e;
      ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

include/bout/fieldops.hxx:312: Loop condition is true. Entering loop body

();
      ^

include/bout/fieldops.hxx:313: Calling 'ReductionView::valueAtRegionPos'

) {
                                    ^

include/bout/fieldops.hxx:188: Array access (via field 'indices') results in a null pointer dereference

    return expr(indices[idx]);
                ^

Comment thread include/bout/fieldops.hxx
inline int regionIdx(int idx) const { return indices[idx]; }

//operator ResT() { return ResT{*this}; }
struct View {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: constructor does not initialize these fields: indices, num_indices [cppcoreguidelines-pro-type-member-init]

include/bout/fieldops.hxx:368:

- ices;
- ices;
+ ices{};
+ ices;{}

Comment thread include/bout/fieldperp.hxx Outdated
FieldPerp& operator=(FieldPerp&& rhs) = default;
FieldPerp& operator=(BoutReal rhs);
template <typename ResT, typename L, typename R, typename Func>
std::enable_if_t<is_expr_fieldperp_v<L>, FieldPerp&>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: operator=() should return 'FieldPerp&' [cppcoreguidelines-c-copy-assignment-signature]

  std::enable_if_t<is_expr_fieldperp_v<L>, FieldPerp&>
  ^


int size() const override { return nx * nz; };

struct View {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: constructor does not initialize these fields: data [cppcoreguidelines-pro-type-member-init]

include/bout/fieldperp.hxx:337:

-     BoutReal* data;
+     BoutReal* data{};

Comment thread src/mesh/coordinates.cxx Outdated

auto g = g11 * g22 * g33 + 2.0 * g12 * g13 * g23 - g11 * g23 * g23 - g22 * g13 * g13
- g33 * g12 * g12;
FieldMetric g = g11 * g22 * g33 + 2.0 * g12 * g13 * g23 - g11 * g23 * g23

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'g' of type 'FieldMetric' (aka 'Field2D') can be declared 'const' [misc-const-correctness]

Suggested change
FieldMetric g = g11 * g22 * g33 + 2.0 * g12 * g13 * g23 - g11 * g23 * g23
FieldMetric const g = g11 * g22 * g33 + 2.0 * g12 * g13 * g23 - g11 * g23 * g23


template <typename ResT, typename L, typename R, typename Func,
typename = std::enable_if_t<bout::utils::is_Field_v<ResT>>>
auto evaluateFieldExpr(const BinaryExpr<ResT, L, R, Func>& expr) -> ResT {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

tests/unit/test_extras.hxx:16:

- #include "bout/fieldperp.hxx"
+ #include "bout/fieldops.hxx"
+ #include "bout/fieldperp.hxx"

Argument to `where` needs to be converted to a Field
from a BinaryExpr.
Document field expressions and GPU support changes.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions


// Set U to zero where P0 < vacuum_pressure
U = where(P0 - vacuum_pressure, U, 0.0);
U = where(Field2D{P0 - vacuum_pressure}, U, 0.0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

examples/elm-pb/elm_pb.cxx:7:

- #include <bout/bout.hxx>
+ #include "bout/where.hxx"
+ #include <bout/bout.hxx>

max(abs(BinaryExpr)) will now perform the reduction without allocating
and filling a field.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

Comment thread include/bout/fieldops.hxx

BOUT_HOST_DEVICE BOUT_FORCEINLINE int size() const { return num_indices; }
BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal valueAtRegionPos(int idx) const {
return expr(indices[idx]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Array access (via field 'indices') results in a null pointer dereference [clang-analyzer-core.NullDereference]

    return expr(indices[idx]);
                ^
Additional context

src/field/fieldperp.cxx:208: Assuming the condition is false

  if (!a.isAllocated() || !b.isAllocated()) {
      ^

src/field/fieldperp.cxx:208: Left side of '||' is false

  if (!a.isAllocated() || !b.isAllocated()) {
      ^

src/field/fieldperp.cxx:208: Assuming the condition is false

  if (!a.isAllocated() || !b.isAllocated()) {
                          ^

src/field/fieldperp.cxx:208: Taking false branch

  if (!a.isAllocated() || !b.isAllocated()) {
  ^

src/field/fieldperp.cxx:211: Assuming the condition is true

  return (a.getIndex() == b.getIndex()) and (min(abs(a - b)) < 1e-10);
          ^

src/field/fieldperp.cxx:211: Left side of '&&' is true

  return (a.getIndex() == b.getIndex()) and (min(abs(a - b)) < 1e-10);
         ^

src/field/fieldperp.cxx:211: Calling 'min<FieldPerp, FieldPerp, FieldPerp, bout::op::abs>'

  return (a.getIndex() == b.getIndex()) and (min(abs(a - b)) < 1e-10);
                                             ^

include/bout/field.hxx:371: 'reduction_view.indices' initialized to a null pointer value

  const auto reduction_view =
  ^

include/bout/field.hxx:375: Calling 'reduceExpr<bout::reduce::Min, ReductionView<BinaryExpr<FieldPerp, FieldPerp, FieldPerp, bout::op::abs>::View>>'

      bout::reduce::Min::finalize(reduceExpr<bout::reduce::Min>(reduction_view));
                                  ^

include/bout/fieldops.hxx:293: Assuming the condition is true

e;
              ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

include/bout/fieldops.hxx:293: Taking false branch

e;
      ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

include/bout/fieldops.hxx:312: Loop condition is true. Entering loop body

();
      ^

include/bout/fieldops.hxx:313: Calling 'ReductionView::valueAtRegionPos'

) {
                                    ^

include/bout/fieldops.hxx:188: Array access (via field 'indices') results in a null pointer dereference

    return expr(indices[idx]);
                ^

Comment thread include/bout/fieldops.hxx

BOUT_HOST_DEVICE BOUT_FORCEINLINE int size() const { return num_indices; }
BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal valueAtRegionPos(int idx) const {
return expr(indices[idx]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Array access (via field 'indices') results in a null pointer dereference [clang-analyzer-core.NullDereference]

    return expr(indices[idx]);
                ^
Additional context

src/mesh/coordinates.cxx:1301: Loop condition is false. Execution continues on line 1326

  BOUT_FOR_SERIAL(i, g_11.getRegion(region)) {
  ^

include/bout/region.hxx:120: expanded from macro 'BOUT_FOR_SERIAL'

  for (auto block = (region).getBlocks().cbegin(), end = (region).getBlocks().cend(); \
  ^

src/mesh/coordinates.cxx:1326: Calling 'max<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, bout::op::Add>, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, bout::op::Add>, Constant, bout::op::Sub>, BinaryExpr<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, bout::op::Add>, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, bout::op::Add>, Constant, bout::op::Sub>, bout::op::abs>'

  maxerr = BOUTMAX(max(abs((g_11 * g11 + g_12 * g12 + g_13 * g13) - 1)),
                   ^

include/bout/field.hxx:473: 'reduction_view.indices' initialized to a null pointer value

;
    ^

include/bout/field.hxx:477: Calling 'reduceExpr<bout::reduce::Max, ReductionView<BinaryExpr<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, bout::op::Add>, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, bout::op::Add>, Constant, bout::op::Sub>, BinaryExpr<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, bout::op::Add>, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, bout::op::Add>, Constant, bout::op::Sub>, bout::op::abs>::View>>'

=
                                    ^

include/bout/fieldops.hxx:293: Assuming the condition is true

e;
              ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

include/bout/fieldops.hxx:293: Taking false branch

e;
      ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

include/bout/fieldops.hxx:312: Loop condition is true. Entering loop body

();
      ^

include/bout/fieldops.hxx:313: Calling 'ReductionView::valueAtRegionPos'

) {
                                    ^

include/bout/fieldops.hxx:188: Array access (via field 'indices') results in a null pointer dereference

    return expr(indices[idx]);
                ^

@bendudson

Copy link
Copy Markdown
Contributor Author

The (slight) performance reduction in elm_pb does not seem to be due to switching from region blocks to index array lookup. I tried an experiment where BinaryExpr::evaluate contains:

if constexpr (std::is_same_v<ResT, Field3D>) {
      if (const auto* region = bout::detail::getField3DRegion(mesh, regionID)) {
        BOUT_FOR_SERIAL(i, *region) {
          data[i.ind] = operator()(i.ind); // single-pass fusion
        }
        return;
      }
    }

so that for Field3D operations it used the previous loop style. This slowed the run further (50s vs 48 vs 38 original). Chatty suggests perhaps (a) Lost auto-vectorization. I remember this was very fragile previously. (b) More inlining pressure and code complexity.(c) Extra integer work on every access: (idx * mul) / div in `Field3D::View::operator(); (d) Simple expressions may dominate the benchmark, outweighing the benefits of merging.

Enables inlining, and should now return a lazy BinaryExpr
rather than eagerly evaluating.
If using Clang or GCC set the always_inline attribute
Fix unitary minus so that it avoids dependency loop with Mesh.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

Comment thread include/bout/field2d.hxx
BoutReal* data;
int mul = 1;
int div = 1;
BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal operator()(int idx) const {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

    BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal operator()(int idx) const {
                     ^

Comment thread include/bout/field2d.hxx
BoutReal* data;
int mul = 1;
int div = 1;
BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal operator()(int idx) const {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

include/bout/field2d.hxx:26:

- #include "bout/utils.hxx"
+ #include "bout/build_config.hxx"
+ #include "bout/utils.hxx"

Comment thread include/bout/fieldops.hxx

BOUT_HOST_DEVICE BOUT_FORCEINLINE int size() const { return num_indices; }
BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal valueAtRegionPos(int idx) const {
return expr(indices[idx]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Array access (via field 'indices') results in a null pointer dereference [clang-analyzer-core.NullDereference]

    return expr(indices[idx]);
                ^
Additional context

src/field/field2d.cxx:373: Assuming the condition is false

  if (!a.isAllocated() || !b.isAllocated()) {
      ^

src/field/field2d.cxx:373: Left side of '||' is false

  if (!a.isAllocated() || !b.isAllocated()) {
      ^

src/field/field2d.cxx:373: Assuming the condition is false

  if (!a.isAllocated() || !b.isAllocated()) {
                          ^

src/field/field2d.cxx:373: Taking false branch

  if (!a.isAllocated() || !b.isAllocated()) {
  ^

src/field/field2d.cxx:376: Calling 'min<Field2D, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Sub>, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Sub>, bout::op::abs>'

  return min(abs(a - b)) < 1e-10;
         ^

include/bout/field.hxx:371: 'reduction_view.indices' initialized to a null pointer value

  const auto reduction_view =
  ^

include/bout/field.hxx:375: Calling 'reduceExpr<bout::reduce::Min, ReductionView<BinaryExpr<Field2D, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Sub>, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Sub>, bout::op::abs>::View>>'

      bout::reduce::Min::finalize(reduceExpr<bout::reduce::Min>(reduction_view));
                                  ^

include/bout/fieldops.hxx:306: Assuming the condition is true

e;
              ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

include/bout/fieldops.hxx:306: Taking false branch

e;
      ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

include/bout/fieldops.hxx:325: Loop condition is true. Entering loop body

();
      ^

include/bout/fieldops.hxx:326: Calling 'ReductionView::valueAtRegionPos'

) {
                                    ^

include/bout/fieldops.hxx:201: Array access (via field 'indices') results in a null pointer dereference

    return expr(indices[idx]);
                ^

Comment thread include/bout/fieldops.hxx

BOUT_HOST_DEVICE BOUT_FORCEINLINE int size() const { return num_indices; }
BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal valueAtRegionPos(int idx) const {
return expr(indices[idx]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Array access (via field 'indices') results in a null pointer dereference [clang-analyzer-core.NullDereference]

    return expr(indices[idx]);
                ^
Additional context

src/field/fieldperp.cxx:205: Assuming the condition is false

  if (!a.isAllocated() || !b.isAllocated()) {
      ^

src/field/fieldperp.cxx:205: Left side of '||' is false

  if (!a.isAllocated() || !b.isAllocated()) {
      ^

src/field/fieldperp.cxx:205: Assuming the condition is false

  if (!a.isAllocated() || !b.isAllocated()) {
                          ^

src/field/fieldperp.cxx:205: Taking false branch

  if (!a.isAllocated() || !b.isAllocated()) {
  ^

src/field/fieldperp.cxx:208: Assuming the condition is true

  return (a.getIndex() == b.getIndex()) and (min(abs(a - b)) < 1e-10);
          ^

src/field/fieldperp.cxx:208: Left side of '&&' is true

  return (a.getIndex() == b.getIndex()) and (min(abs(a - b)) < 1e-10);
         ^

src/field/fieldperp.cxx:208: Calling 'min<FieldPerp, FieldPerp, FieldPerp, bout::op::abs>'

  return (a.getIndex() == b.getIndex()) and (min(abs(a - b)) < 1e-10);
                                             ^

include/bout/field.hxx:371: 'reduction_view.indices' initialized to a null pointer value

  const auto reduction_view =
  ^

include/bout/field.hxx:375: Calling 'reduceExpr<bout::reduce::Min, ReductionView<BinaryExpr<FieldPerp, FieldPerp, FieldPerp, bout::op::abs>::View>>'

      bout::reduce::Min::finalize(reduceExpr<bout::reduce::Min>(reduction_view));
                                  ^

include/bout/fieldops.hxx:306: Assuming the condition is true

e;
              ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

include/bout/fieldops.hxx:306: Taking false branch

e;
      ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

include/bout/fieldops.hxx:325: Loop condition is true. Entering loop body

();
      ^

include/bout/fieldops.hxx:326: Calling 'ReductionView::valueAtRegionPos'

) {
                                    ^

include/bout/fieldops.hxx:201: Array access (via field 'indices') results in a null pointer dereference

    return expr(indices[idx]);
                ^

Comment thread include/bout/fieldops.hxx

BOUT_HOST_DEVICE BOUT_FORCEINLINE int size() const { return num_indices; }
BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal valueAtRegionPos(int idx) const {
return expr(indices[idx]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Array access (via field 'indices') results in a null pointer dereference [clang-analyzer-core.NullDereference]

    return expr(indices[idx]);
                ^
Additional context

src/invert/laplace/impls/naulin/naulin_laplace.cxx:230: Assuming the condition is true

  ASSERT1(rhs.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:230: Taking false branch

  ASSERT1(rhs.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:231: Assuming the condition is true

  ASSERT1(x0.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:231: Taking false branch

  ASSERT1(x0.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:232: Assuming the condition is true

  ASSERT1(Dcoef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:232: Taking false branch

  ASSERT1(Dcoef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:233: Assuming the condition is true

  ASSERT1(C1coef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:233: Taking false branch

  ASSERT1(C1coef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:234: Assuming the condition is true

  ASSERT1(C2coef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:234: Taking false branch

  ASSERT1(C2coef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:235: Assuming the condition is true

  ASSERT1(Acoef.getLocation() == location);
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:235: Taking false branch

  ASSERT1(Acoef.getLocation() == location);
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Assuming the condition is true

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
          ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Left side of '&&' is true

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
          ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Assuming the condition is true

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
                                        ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:236: Taking false branch

  ASSERT1(localmesh == rhs.getMesh() && localmesh == x0.getMesh());
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/laplace/impls/naulin/naulin_laplace.cxx:276: Calling 'mean<Field3D, Field3D, Field3D, bout::op::Square>'

  BoutReal RMS_rhsOverD = sqrt(mean(
                               ^

include/bout/field.hxx:526: 'reduction_view.indices' initialized to a null pointer value

;
    ^

include/bout/field.hxx:529: Calling 'reduceExpr<bout::reduce::Mean, ReductionView<BinaryExpr<Field3D, Field3D, Field3D, bout::op::Square>::View>>'

;
                 ^

include/bout/fieldops.hxx:306: Assuming the condition is true

e;
              ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

include/bout/fieldops.hxx:306: Taking false branch

e;
      ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

include/bout/fieldops.hxx:325: Loop condition is true. Entering loop body

();
      ^

include/bout/fieldops.hxx:326: Calling 'ReductionView::valueAtRegionPos'

) {
                                    ^

include/bout/fieldops.hxx:201: Array access (via field 'indices') results in a null pointer dereference

    return expr(indices[idx]);
                ^

Comment thread include/bout/fieldops.hxx

BOUT_HOST_DEVICE BOUT_FORCEINLINE int size() const { return num_indices; }
BOUT_HOST_DEVICE BOUT_FORCEINLINE BoutReal valueAtRegionPos(int idx) const {
return expr(indices[idx]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Array access (via field 'indices') results in a null pointer dereference [clang-analyzer-core.NullDereference]

    return expr(indices[idx]);
                ^
Additional context

src/mesh/coordinates.cxx:1301: Loop condition is false. Execution continues on line 1326

  BOUT_FOR_SERIAL(i, g_11.getRegion(region)) {
  ^

include/bout/region.hxx:120: expanded from macro 'BOUT_FOR_SERIAL'

  for (auto block = (region).getBlocks().cbegin(), end = (region).getBlocks().cend(); \
  ^

src/mesh/coordinates.cxx:1326: Calling 'max<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, bout::op::Add>, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, bout::op::Add>, Constant, bout::op::Sub>, BinaryExpr<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, bout::op::Add>, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, bout::op::Add>, Constant, bout::op::Sub>, bout::op::abs>'

  maxerr = BOUTMAX(max(abs((g_11 * g11 + g_12 * g12 + g_13 * g13) - 1)),
                   ^

include/bout/field.hxx:473: 'reduction_view.indices' initialized to a null pointer value

;
    ^

include/bout/field.hxx:477: Calling 'reduceExpr<bout::reduce::Max, ReductionView<BinaryExpr<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, bout::op::Add>, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, bout::op::Add>, Constant, bout::op::Sub>, BinaryExpr<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, bout::op::Add>, BinaryExpr<Field2D, Field2D, Field2D, bout::op::Mul>, bout::op::Add>, Constant, bout::op::Sub>, bout::op::abs>::View>>'

=
                                    ^

include/bout/fieldops.hxx:306: Assuming the condition is true

e;
              ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
        ^

include/bout/fieldops.hxx:306: Taking false branch

e;
      ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

include/bout/fieldops.hxx:325: Loop condition is true. Entering loop body

();
      ^

include/bout/fieldops.hxx:326: Calling 'ReductionView::valueAtRegionPos'

) {
                                    ^

include/bout/fieldops.hxx:201: Array access (via field 'indices') results in a null pointer dereference

    return expr(indices[idx]);
                ^

Comment thread include/bout/fieldops.hxx
BOUT_HOST_DEVICE BOUT_FORCEINLINE int regionIdx(int idx) const { return indices[idx]; }

//operator ResT() { return ResT{*this}; }
struct View {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: constructor does not initialize these fields: indices, num_indices [cppcoreguidelines-pro-type-member-init]

include/bout/fieldops.hxx:381:

- ices;
- ices;
+ ices{};
+ ices;{}

FieldPerp& operator=(FieldPerp&& rhs) = default;
FieldPerp& operator=(BoutReal rhs);
template <typename ResT, typename L, typename R, typename Func>
std::enable_if_t<is_expr_fieldperp_v<L> || is_expr_constant_v<L>, FieldPerp&>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: operator=() should return 'FieldPerp&' [cppcoreguidelines-c-copy-assignment-signature]

  std::enable_if_t<is_expr_fieldperp_v<L> || is_expr_constant_v<L>, FieldPerp&>
  ^

@bendudson bendudson merged commit 92ddb1b into next Jun 20, 2026
23 checks passed
@bendudson bendudson deleted the support-gpu-field-operators-lazy branch June 20, 2026 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants