From 95a90403f8180c7ef320ed473603bf3850322359 Mon Sep 17 00:00:00 2001 From: Ben Dudson Date: Fri, 24 Apr 2026 15:47:56 -0700 Subject: [PATCH 1/2] Field setBoundary and applyBoundary use options When a boundary value uses options then these were only parsed on processors with boundaries. This uses a dummy boundary region to force all processors to parse options. This should fix the problem that means unused options checking needs to be disabled in many transport simulations. --- src/field/field3d.cxx | 9 +++- src/field/field_data.cxx | 16 +++++- src/field/field_factory.cxx | 21 +++++--- tests/unit/field/test_field3d.cxx | 60 ++++++++++++++++------- tests/unit/mesh/test_boundary_factory.cxx | 31 ++++++++++-- 5 files changed, 105 insertions(+), 32 deletions(-) diff --git a/src/field/field3d.cxx b/src/field/field3d.cxx index 9154f78a4f..144806ac16 100644 --- a/src/field/field3d.cxx +++ b/src/field/field3d.cxx @@ -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 * @@ -36,6 +36,7 @@ #include #include #include +#include #include #include "bout/parallel_boundary_op.hxx" @@ -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, 1, fieldmesh}; + auto* bndry = bfact->create(condition, &dummy_region); + delete bndry; + /// Loop over the mesh boundary regions for (const auto& reg : fieldmesh->getBoundaries()) { auto op = std::unique_ptr{ diff --git a/src/field/field_data.cxx b/src/field/field_data.cxx index b925cb1426..1204056c63 100644 --- a/src/field/field_data.cxx +++ b/src/field/field_data.cxx @@ -1,14 +1,19 @@ #include "bout/parallel_boundary_op.hxx" #include "bout/parallel_boundary_region.hxx" -#include "bout/unused.hxx" #include +#include +#include #include #include #include #include +#include #include +#include +#include + namespace bout { /// Make sure \p location is a sensible value for \p mesh /// @@ -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, 1, mesh}; + auto* bndry = bfact->createFromOptions(name, &dummy_region); + delete bndry; + + // Loop over the mesh boundary regions for (const auto& reg : mesh->getBoundaries()) { auto* op = dynamic_cast(bfact->createFromOptions(name, reg)); if (op != nullptr) { diff --git a/src/field/field_factory.cxx b/src/field/field_factory.cxx index 188a64cf0c..6c01715574 100644 --- a/src/field/field_factory.cxx +++ b/src/field/field_factory.cxx @@ -1,5 +1,5 @@ /************************************************************************** - * Copyright 2010-2025 BOUT++ contributors + * Copyright 2010-2026 BOUT++ contributors * * Contact: Ben Dudson, dudson2@llnl.gov * @@ -25,14 +25,21 @@ #include -#include -#include -#include - +#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 +#include +#include +#include + using bout::generator::Context; /// Helper function to create a FieldValue generator from a BoutReal @@ -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; } @@ -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; } diff --git a/tests/unit/field/test_field3d.cxx b/tests/unit/field/test_field3d.cxx index d80affc3f7..af46dffda0 100644 --- a/tests/unit/field/test_field3d.cxx +++ b/tests/unit/field/test_field3d.cxx @@ -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" @@ -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); } @@ -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); @@ -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); @@ -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); @@ -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); } @@ -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); } @@ -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; @@ -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]; @@ -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; @@ -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); @@ -2442,7 +2445,7 @@ 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); @@ -2450,5 +2453,24 @@ TEST_F(Field3DTest, ZeroFrom) { 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 diff --git a/tests/unit/mesh/test_boundary_factory.cxx b/tests/unit/mesh/test_boundary_factory.cxx index a36b7da9d9..0d9d85dde5 100644 --- a/tests/unit/mesh/test_boundary_factory.cxx +++ b/tests/unit/mesh/test_boundary_factory.cxx @@ -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 +#include +#include + // 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& args, @@ -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); } @@ -97,7 +106,7 @@ TEST_F(BoundaryFactoryTest, CreateTestBoundaryPositionalArgumentsAndKeywords) { auto* tb = dynamic_cast(boundary); - std::list args_expected = {"0.23", "something()", "a + sin(1.2)"}; + const std::list args_expected = {"0.23", "something()", "a + sin(1.2)"}; EXPECT_EQ(tb->args, args_expected); @@ -105,3 +114,19 @@ TEST_F(BoundaryFactoryTest, CreateTestBoundaryPositionalArgumentsAndKeywords) { 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()); +} From 98ce2348c9bbbc6e298ca8d2612eea9fed60b985 Mon Sep 17 00:00:00 2001 From: Ben Dudson Date: Fri, 24 Apr 2026 16:23:35 -0700 Subject: [PATCH 2/2] Set dummy boundary to zero width --- src/field/field3d.cxx | 2 +- src/field/field_data.cxx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/field/field3d.cxx b/src/field/field3d.cxx index 144806ac16..310097e0b3 100644 --- a/src/field/field3d.cxx +++ b/src/field/field3d.cxx @@ -401,7 +401,7 @@ void Field3D::applyBoundary(const std::string& condition) { // Create a boundary for a dummy region. // This ensures that options are parsed/used on all processors - BoundaryRegionXIn dummy_region{"dummy_region", 0, 1, fieldmesh}; + BoundaryRegionXIn dummy_region{"dummy_region", 0, 0, fieldmesh}; auto* bndry = bfact->create(condition, &dummy_region); delete bndry; diff --git a/src/field/field_data.cxx b/src/field/field_data.cxx index 1204056c63..6a8ce162af 100644 --- a/src/field/field_data.cxx +++ b/src/field/field_data.cxx @@ -150,7 +150,7 @@ void FieldData::setBoundary(const std::string& name) { // Create a boundary for a dummy region. // This ensures that options are parsed/used on all processors - BoundaryRegionXIn dummy_region{"dummy_region", 0, 1, mesh}; + BoundaryRegionXIn dummy_region{"dummy_region", 0, 0, mesh}; auto* bndry = bfact->createFromOptions(name, &dummy_region); delete bndry;