Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/field/field3d.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* Class for 3D X-Y-Z scalar fields
*
**************************************************************************
* Copyright 2010 - 2025 BOUT++ developers
* Copyright 2010 - 2026 BOUT++ developers
*
* Contact: Ben Dudson, dudson2@llnl.gov
*
Expand Down Expand Up @@ -36,6 +36,7 @@
#include <cstddef>
#include <memory>
#include <optional>
#include <string>
#include <utility>

#include "bout/parallel_boundary_op.hxx"
Expand Down Expand Up @@ -398,6 +399,12 @@ void Field3D::applyBoundary(const std::string& condition) {
/// Get the boundary factory (singleton)
BoundaryFactory* bfact = BoundaryFactory::getInstance();

// Create a boundary for a dummy region.
// This ensures that options are parsed/used on all processors
BoundaryRegionXIn dummy_region{"dummy_region", 0, 0, fieldmesh};
auto* bndry = bfact->create(condition, &dummy_region);
delete bndry;
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: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead [cppcoreguidelines-owning-memory]

  delete bndry;
  ^
Additional context

src/field/field3d.cxx:404: variable declared here

  auto* bndry = bfact->create(condition, &dummy_region);
  ^


/// Loop over the mesh boundary regions
for (const auto& reg : fieldmesh->getBoundaries()) {
auto op = std::unique_ptr<BoundaryOp>{
Expand Down
16 changes: 14 additions & 2 deletions src/field/field_data.cxx
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@

#include "bout/parallel_boundary_op.hxx"
#include "bout/parallel_boundary_region.hxx"
#include "bout/unused.hxx"
#include <bout/boundary_factory.hxx>
#include <bout/bout_types.hxx>
#include <bout/boutexception.hxx>
#include <bout/field_data.hxx>
#include <bout/field_factory.hxx>
#include <bout/globals.hxx>
#include <bout/mesh.hxx>
#include <bout/options.hxx>
#include <bout/output.hxx>

#include <string>
#include <utility>

namespace bout {
/// Make sure \p location is a sensible value for \p mesh
///
Expand Down Expand Up @@ -142,7 +147,14 @@ void FieldData::setBoundary(const std::string& name) {
markBoundariesAsConditionallyUsed(mesh, Options::root()["all"]);

output_info << "Setting boundary for variable " << name << endl;
/// Loop over the mesh boundary regions

// Create a boundary for a dummy region.
// This ensures that options are parsed/used on all processors
BoundaryRegionXIn dummy_region{"dummy_region", 0, 0, mesh};
auto* bndry = bfact->createFromOptions(name, &dummy_region);
delete bndry;
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: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead [cppcoreguidelines-owning-memory]

  delete bndry;
  ^
Additional context

src/field/field_data.cxx:153: variable declared here

  auto* bndry = bfact->createFromOptions(name, &dummy_region);
  ^


// Loop over the mesh boundary regions
for (const auto& reg : mesh->getBoundaries()) {
auto* op = dynamic_cast<BoundaryOp*>(bfact->createFromOptions(name, reg));
if (op != nullptr) {
Expand Down
21 changes: 14 additions & 7 deletions src/field/field_factory.cxx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**************************************************************************
* Copyright 2010-2025 BOUT++ contributors
* Copyright 2010-2026 BOUT++ contributors
*
* Contact: Ben Dudson, dudson2@llnl.gov
*
Expand All @@ -25,14 +25,21 @@

#include <cmath>

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

#include "bout/bout_types.hxx"
#include "bout/boutexception.hxx"
#include "bout/constants.hxx"
#include "bout/options.hxx"
#include "bout/output.hxx"
#include "bout/sys/expressionparser.hxx"
#include "bout/sys/generator_context.hxx"

#include "fieldgenerators.hxx"

#include <exception>
#include <memory>
#include <string>
#include <utility>

using bout::generator::Context;

/// Helper function to create a FieldValue generator from a BoutReal
Expand All @@ -52,7 +59,7 @@ class FieldIndirect : public FieldGenerator {
public:
/// depth_limit sets the maximum iteration depth. Set to < 0 for no limit
FieldIndirect(std::string name, int depth_limit = 0)
: name(name), depth_limit(depth_limit) {}
: name(std::move(name)), depth_limit(depth_limit) {}

/// Set the target, to be called when generator is called
void setTarget(FieldGeneratorPtr fieldgen) { target = fieldgen; }
Expand All @@ -63,7 +70,7 @@ class FieldIndirect : public FieldGenerator {
name, depth_counter, depth_limit);
}
++depth_counter;
BoutReal result = target->generate(ctx);
const BoutReal result = target->generate(ctx);
--depth_counter;
return result;
}
Expand Down
60 changes: 41 additions & 19 deletions tests/unit/field/test_field3d.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@
#include "gtest/gtest.h"

#include "test_extras.hxx"
#include "bout/bout_types.hxx"
#include "bout/boutexception.hxx"
#include "bout/constants.hxx"
#include "bout/field3d.hxx"
#include "bout/globals.hxx"
#include "bout/mesh.hxx"
#include "bout/output.hxx"
#include "bout/sys/expressionparser.hxx"
#include "bout/unused.hxx"
#include "bout/utils.hxx"

Expand All @@ -28,13 +31,13 @@ using namespace bout::globals;
using Field3DTest = FakeMeshFixture;

TEST_F(Field3DTest, Is3D) {
Field3D field;
const Field3D field;

EXPECT_TRUE(field.is3D());
}

TEST_F(Field3DTest, BoutRealSize) {
Field3D field;
const Field3D field;

EXPECT_EQ(field.elementSize(), 1);
}
Expand Down Expand Up @@ -74,14 +77,14 @@ TEST_F(Field3DTest, GetGridSizes) {
}

TEST_F(Field3DTest, CreateOnGivenMesh) {
int test_nx = Field3DTest::nx + 2;
int test_ny = Field3DTest::ny + 2;
int test_nz = Field3DTest::nz + 2;
const int test_nx = Field3DTest::nx + 2;
const int test_ny = Field3DTest::ny + 2;
const int test_nz = Field3DTest::nz + 2;

FakeMesh fieldmesh{test_nx, test_ny, test_nz};
fieldmesh.setCoordinates(nullptr);

Field3D field{&fieldmesh};
const Field3D field{&fieldmesh};

EXPECT_EQ(field.getNx(), test_nx);
EXPECT_EQ(field.getNy(), test_ny);
Expand All @@ -99,9 +102,9 @@ TEST_F(Field3DTest, CopyCheckFieldmesh) {
fieldmesh.setCoordinates(nullptr);
fieldmesh.createDefaultRegions();

Field3D field{0.0, &fieldmesh};
const Field3D field{0.0, &fieldmesh};

Field3D field2{field};
const Field3D field2{field};

EXPECT_EQ(field2.getNx(), test_nx);
EXPECT_EQ(field2.getNy(), test_ny);
Expand Down Expand Up @@ -166,10 +169,10 @@ TEST_F(Field3DTest, CreateCopyOnNullMesh) {
TEST_F(Field3DTest, TimeDeriv) {
Field3D field;

auto deriv = field.timeDeriv();
auto* deriv = field.timeDeriv();
EXPECT_NE(&field, deriv);

auto deriv2 = field.timeDeriv();
auto* deriv2 = field.timeDeriv();
EXPECT_EQ(deriv, deriv2);

EXPECT_EQ(&(ddt(field)), deriv);
Expand Down Expand Up @@ -305,9 +308,9 @@ TEST_F(Field3DTest, ConstYnext) {
}

TEST_F(Field3DTest, GetGlobalMesh) {
Field3D field;
const Field3D field;

auto localmesh = field.getMesh();
auto* localmesh = field.getMesh();

EXPECT_EQ(localmesh, mesh);
}
Expand All @@ -316,9 +319,9 @@ TEST_F(Field3DTest, GetLocalMesh) {
FakeMesh myMesh{nx + 1, ny + 2, nz + 3};
myMesh.setCoordinates(nullptr);

Field3D field(&myMesh);
const Field3D field(&myMesh);

auto localmesh = field.getMesh();
auto* localmesh = field.getMesh();

EXPECT_EQ(localmesh, &myMesh);
}
Expand Down Expand Up @@ -1027,7 +1030,7 @@ TEST_F(Field3DTest, IndexingToZPointer) {

for (int i = 0; i < nx; ++i) {
for (int j = 0; j < ny; ++j) {
auto tmp = field(i, j);
auto* tmp = field(i, j);
for (int k = 0; k < nz; ++k) {
EXPECT_EQ(tmp[k], i + j + k);
tmp[k] = -1.0;
Expand Down Expand Up @@ -1056,7 +1059,7 @@ TEST_F(Field3DTest, ConstIndexingToZPointer) {

for (int i = 0; i < nx; ++i) {
for (int j = 0; j < ny; ++j) {
auto tmp = field(i, j);
const auto* tmp = field(i, j);
for (int k = 0; k < nz; ++k) {
EXPECT_EQ(tmp[k], 1.0);
field2(i, j, k) = tmp[k];
Expand Down Expand Up @@ -2404,7 +2407,7 @@ TEST_F(Field3DTest, OperatorEqualsField3D) {
// to 'field'.
// Note that Average z-direction type is not really allowed for Field3D, but
// we don't check anywhere at the moment.
Field3D field2{
const Field3D field2{
mesh_staggered, CELL_XLOW, {YDirectionType::Aligned, ZDirectionType::Average}};

field = field2;
Expand All @@ -2425,7 +2428,7 @@ TEST_F(Field3DTest, EmptyFrom) {
mesh_staggered, CELL_XLOW, {YDirectionType::Aligned, ZDirectionType::Average}};
field = 5.;

Field3D field2{emptyFrom(field)};
const Field3D field2{emptyFrom(field)};
EXPECT_EQ(field2.getMesh(), mesh_staggered);
EXPECT_EQ(field2.getLocation(), CELL_XLOW);
EXPECT_EQ(field2.getDirectionY(), YDirectionType::Aligned);
Expand All @@ -2442,13 +2445,32 @@ TEST_F(Field3DTest, ZeroFrom) {
mesh_staggered, CELL_XLOW, {YDirectionType::Aligned, ZDirectionType::Average}};
field = 5.;

Field3D field2{zeroFrom(field)};
const Field3D field2{zeroFrom(field)};
EXPECT_EQ(field2.getMesh(), mesh_staggered);
EXPECT_EQ(field2.getLocation(), CELL_XLOW);
EXPECT_EQ(field2.getDirectionY(), YDirectionType::Aligned);
EXPECT_EQ(field2.getDirectionZ(), ZDirectionType::Average);
EXPECT_TRUE(field2.isAllocated());
EXPECT_TRUE(IsFieldEqual(field2, 0.));
}

TEST_F(Field3DTest, ApplyBoundaryMissingOptions) {
Options::root() = {};

Field3D f{1.0};
// No boundaries but should still parse value
EXPECT_THROW(f.applyBoundary("dirichlet(some_boundary_value)"), ParseException);
}

TEST_F(Field3DTest, SetBoundaryUsesOption) {
Options::root() = {{"toplevel_value", 1.0},
{"var", {{"bndry_all", "dirichlet(toplevel_value)"}}}};

Field3D f{1.0};
f.setBoundary("var");

ASSERT_TRUE(Options::root()["toplevel_value"].valueUsed());
}

// Restore compiler warnings
#pragma GCC diagnostic pop
31 changes: 28 additions & 3 deletions tests/unit/mesh/test_boundary_factory.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,20 @@
#include "bout/boundary_factory.hxx"
#include "bout/boundary_op.hxx"
#include "bout/boundary_region.hxx"
#include "bout/field2d.hxx"
#include "bout/field3d.hxx"
#include "bout/sys/expressionparser.hxx"

#include "fake_mesh.hxx"

#include <list>
#include <map>
#include <string>

// The unit tests use the global mesh
using namespace bout::globals;

namespace {
class TestBoundary : public BoundaryOp {
public:
BoundaryOp* clone(BoundaryRegion* UNUSED(region), const std::list<std::string>& args,
Expand Down Expand Up @@ -50,10 +58,11 @@ class BoundaryFactoryTest : public ::testing::Test {
BoundaryRegionXIn* region{nullptr};
BoundaryOpBase* boundary{nullptr};
};
} // namespace

TEST_F(BoundaryFactoryTest, IsSingleton) {
BoundaryFactory* fac1 = BoundaryFactory::getInstance();
BoundaryFactory* fac2 = BoundaryFactory::getInstance();
const BoundaryFactory* fac1 = BoundaryFactory::getInstance();
const BoundaryFactory* fac2 = BoundaryFactory::getInstance();

EXPECT_EQ(fac1, fac2);
}
Expand Down Expand Up @@ -97,11 +106,27 @@ TEST_F(BoundaryFactoryTest, CreateTestBoundaryPositionalArgumentsAndKeywords) {

auto* tb = dynamic_cast<TestBoundary*>(boundary);

std::list<std::string> args_expected = {"0.23", "something()", "a + sin(1.2)"};
const std::list<std::string> args_expected = {"0.23", "something()", "a + sin(1.2)"};

EXPECT_EQ(tb->args, args_expected);

EXPECT_EQ(tb->keywords.size(), 2);
EXPECT_EQ(tb->keywords.at("key"), "1+2");
EXPECT_EQ(tb->keywords.at("b"), "value");
}

TEST_F(BoundaryFactoryTest, DirichletMissingValue) {
Options::root() = {};

EXPECT_THROW(fac->create("dirichlet(value)", region), ParseException);
}

TEST_F(BoundaryFactoryTest, DirichletUsesValue) {
Options::root() = {{"value", 1.0}};

EXPECT_FALSE(Options::root()["value"].valueUsed());

auto* boundary = fac->create("dirichlet(value)", region);

ASSERT_TRUE(Options::root()["value"].valueUsed());
}
Loading