From 712b20bc08f917b1fde53458e7234ed1e30e415a Mon Sep 17 00:00:00 2001 From: Allum Date: Thu, 4 Dec 2025 14:51:18 -0700 Subject: [PATCH 01/21] Base class for submodules: base_step. `base_step` name comes from the idea that submodules are steps in a module. Uses CRTP to avoid overhead. Derived classes are defined as follows: ```cpp template class step : public base_step { public: void execute_impl(data& d) { // Implementation here } } ``` `data` is intended to be the module specific data class derived from `face_info`. Derived classses should make use of concepts to enforce getters and setters on data. Example ``` \#include template concept StepConcept = requires(T& t) { { t.input1() } -> std::floating_point; { t.input2() } -> std::integral; { t.output1(std::declval()) } -> std::same_as } template class step : public base_step { public: void execute_impl(data& d) { // Implementation here } } ``` At the moment, it is recommended to supply everything via functions, even parameters that are set at run time but never changed. For parameters that are domain wide one could write a function: ```cpp const double module::data::foo() { static const double result = cfg.get("result"); return result; } ``` Static ensures that the instance is (a) uniform per instance of `data`, and (b) set once (thread safe). It does involve a hidding bool and if check that is evaluated at runtime on every call. Parameters that are per-face must have a private variable. Designing submodules derived from `base_step` with getters and setters provides flexibility for the writer. --- src/modules/submodules/base_step.hpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 src/modules/submodules/base_step.hpp diff --git a/src/modules/submodules/base_step.hpp b/src/modules/submodules/base_step.hpp new file mode 100644 index 00000000..66c30d47 --- /dev/null +++ b/src/modules/submodules/base_step.hpp @@ -0,0 +1,15 @@ +#include + +template +concept GuaranteeImplementExecute = requires(T t, Data d) { + { t.execute_impl(d) } -> std::same_as; +}; + +template + requires GuaranteeImplementExecute +class base_step { +public: + void execute(Data& d) { + static_cast(this)->execute_impl(d); + } +}; From 110f6c3c0eb13f346dabc9b6278ea339910e2a13 Mon Sep 17 00:00:00 2001 From: Allum Date: Fri, 5 Dec 2025 10:13:50 -0700 Subject: [PATCH 02/21] Correcting `base_step` constness, concepts, and constructors **Constness** `GuaranteeImplementExecute` now accepts `const T&` and `Data&` types. This ensures that (a) `T` has a function `void execute_impl` that takes a `Data&` and (b) `T` can be `const` and or a reference. **Concepts** The previous concept was circular: a derived class would need to be declared like: ```cpp template class Derived : public base_step ``` But since `Derived` is not fully defined, it cannot satisfy the concept. Instead, this is replaced with a static assert that is only tripped if `execute` is called. It is still tripped at compile time and therefore has the same effect. **Constructors** The constructor is now explicitly declared and is protected. --- src/modules/submodules/base_step.hpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/modules/submodules/base_step.hpp b/src/modules/submodules/base_step.hpp index 66c30d47..80e81334 100644 --- a/src/modules/submodules/base_step.hpp +++ b/src/modules/submodules/base_step.hpp @@ -1,15 +1,19 @@ #include template -concept GuaranteeImplementExecute = requires(T t, Data d) { +concept GuaranteeImplementExecute = requires(const T& t, Data& d) { { t.execute_impl(d) } -> std::same_as; }; template - requires GuaranteeImplementExecute class base_step { +protected: + base_step() {}; public: - void execute(Data& d) { - static_cast(this)->execute_impl(d); + void execute(Data& d) const { + static_assert(GuaranteeImplementExecute, + "Derived class must implement: void execute_impl(Data&) const"); + static_cast(this)->execute_impl(d); } + ~base_step() {}; }; From 20e0ac7ea73c203a4680c7f06566d23d234f5575 Mon Sep 17 00:00:00 2001 From: Allum Date: Fri, 5 Dec 2025 10:25:24 -0700 Subject: [PATCH 03/21] Added documentation to `base_step` --- src/modules/submodules/base_step.hpp | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/modules/submodules/base_step.hpp b/src/modules/submodules/base_step.hpp index 80e81334..1e0ac541 100644 --- a/src/modules/submodules/base_step.hpp +++ b/src/modules/submodules/base_step.hpp @@ -1,3 +1,26 @@ +/** + * @brief Base class for submodules using the Curiously Recurring Template Pattern (CRTP). + * + * This class enforces that derived classes implement an `execute_impl(Data&) const` method, + * and provides a public `execute(Data&) const` method that forwards to the derived implementation. + * + * The CRTP pattern allows compile-time polymorphism, enabling static dispatch and + * avoiding the overhead of virtual functions. This is useful for performance-critical + * code or when templates are preferred over inheritance. + * + * Usage example: + * @code + * struct MyStep : public base_step { + * void execute_impl(MyData& d) { + * // Step-specific logic here + * } + * }; + * MyStep step; + * MyData data; + * step.execute(data); // Calls MyStep::execute_impl(data) + * @endcode + */ + #include template @@ -11,8 +34,10 @@ class base_step { base_step() {}; public: void execute(Data& d) const { + static_assert(GuaranteeImplementExecute, "Derived class must implement: void execute_impl(Data&) const"); + static_cast(this)->execute_impl(d); } ~base_step() {}; From b4b3ea7534c6bc873f983600f98c48d6abe61d0a Mon Sep 17 00:00:00 2001 From: Allum Date: Fri, 5 Dec 2025 10:26:00 -0700 Subject: [PATCH 04/21] Added `#pragma once` to `base_step.hpp` --- src/modules/submodules/base_step.hpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/modules/submodules/base_step.hpp b/src/modules/submodules/base_step.hpp index 1e0ac541..95919aad 100644 --- a/src/modules/submodules/base_step.hpp +++ b/src/modules/submodules/base_step.hpp @@ -1,3 +1,6 @@ +#pragma once +#include + /** * @brief Base class for submodules using the Curiously Recurring Template Pattern (CRTP). * @@ -21,8 +24,6 @@ * @endcode */ -#include - template concept GuaranteeImplementExecute = requires(const T& t, Data& d) { { t.execute_impl(d) } -> std::same_as; From d2d32fc364c86e4c8031779f6d1b243c573b7f77 Mon Sep 17 00:00:00 2001 From: Allum Date: Fri, 5 Dec 2025 10:35:03 -0700 Subject: [PATCH 05/21] Added GNU License Header and author name --- src/modules/submodules/base_step.hpp | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/modules/submodules/base_step.hpp b/src/modules/submodules/base_step.hpp index 95919aad..0b0ac868 100644 --- a/src/modules/submodules/base_step.hpp +++ b/src/modules/submodules/base_step.hpp @@ -1,3 +1,30 @@ +// +// Canadian Hydrological Model - The Canadian Hydrological Model (CHM) is a novel +// modular unstructured mesh based approach for hydrological modelling +// Copyright (C) 2018 Christopher Marsh +// +// This file is part of Canadian Hydrological Model. +// +// Canadian Hydrological Model is free software: you can redistribute it and/or +// modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Canadian Hydrological Model is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Canadian Hydrological Model. If not, see +// . +// + +// +// Created by Donovan Allum 2025 +// + #pragma once #include From 25a8f895af9e3b4dbec3f4e6568b612bac2e5516 Mon Sep 17 00:00:00 2001 From: Allum Date: Thu, 4 Dec 2025 15:45:59 -0700 Subject: [PATCH 06/21] Data base class with finer control Introducing new base class for the data classes owned by each module. Coordinates per-time-step caching and access to triangle-specific data. `data_base` is the base class. It expects a single template parameter `CacheType`. `CacheType` is always derived from `cache_base` and this is enforced by the CacheRules concept. `CacheType` is unique to each module, and at least should contain a single variable member for each depends and provides variable defined in the module constructor. Each is used to store values dereferenced from face (inputs) or values to be set to `face` later (outputs), and released at the end of a time-step. `cache_base` is a very basic struct with one member variable and one static constexpr member function: `int64_t last_timestep` Initialized as -1. Keeps track of the current time step. Somewhat deprecated, but originally included to ensure that a cache would always be reset on a new timestep. ```cpp template static constexpr T default_value(); ``` `default_value()` returns a sentinal value used to initialize variables on the `CacheType` objected derived from `cache_base`. It sets floats to `std::numeric_limits::quiet_NaN()` and integral types to `std::numeric_limits::min()`. The latter type is not the safest at the moment considering that unsigned ints exist and may need to be adjusted later. These sentinal values exist so that each member can be marked as cached (or not cached) separately. An example of `CacheType` is as follows: ```cpp class Cache : public cache_base { // Has to be NaN does not have to be this version of NaN double input_variable = default_value(); int input2 = default_value(); // Outputs are stored as 0.0 double output_variable = 0.0; }; ``` `data_base` is written specifically to cater to the `base_step` class. Derived classes of base_step have a single template parameter to a class/struct to access inputs and parameters. It is intended that the data class is used in this template. `data_base` provides tools to write functions to that the `base_step` template expects to pass inputs and parameters set at runtime. `data_base` has private, protected and public members. **Private:** ```cpp template bool constexpr check_if_set(T t); ``` This `constexpr` function checks if an input is equal to its sentinal value. `bool is_stale();`: Checks if `cache_base::last_timestep` is not equal to `global_param->timestep_counter`. Means that the cache was (mistakenly) not reset at the end of the last timestep. Typically a programmer error. May be deprecated but technically works. `void init_cache();`: Checks if the cache (`cache_` protected member, see below) has been initialized. If not, it is initialized. After which all members are set to their sentinal values. **Protected:** The constructor is protected so that only derived classes can create this class. ```cpp data_base(const mesh_elem& face_in, const boost::shared_ptr param, const pt::ptree& cfg, const bool istest = false); ``` Accepts three arguments, a `const mesh_elem&`, `const boost::shared_ptr` and `const pt::ptree&`. There is also a `const bool` argument that defaults to `false`. This last argument exists just to make googletests easier without needed to mock the `mesh_elem` object. Allows `face` and `global_param` to be NULL for tests. Each of the objects in the constructor are protected as they are intended to be used by the derived class for better functions for submodules. `mutable std::optional cache_;`: This is the cache object. Mutable so that it can be modified in `const` methods. Accessing inputs in submodules is a const-like operation in that the inputs aren't being modified, but the cache is of course modified on first call. Before we can discuss these functions, we have to first introduce the concepts `ValueRules` and `OutputRules`. The former ensures that the type is (a) callable and (b) produces an l-value reference of the type that the function outputs. The latter ensures that the type obeys `ValueRules` AND its output is convertible to `T`. As we will see below, where `ValueRules` is used, the output type is not important or part of the function signature. Whereas for where `OutputRules` is used, the output type `T` is part of the function signature (and is the type to be output). It is labeled `convertible_to` as to not require that the user use exactly the same type everywhere and implicit converstion is OK. Two functions: ```cpp template void update_value(Value&& value, const Fetch& fetch); template Output> void set_output(Output&& output,const T t); ``` `update_value` receives two arguments. One of type `Value&&` and the other of type `const Fetch&`. Both are callables but only one has a type that can be confirmed (I think). `value` is a callable that returns an l-value reference. Writting it as `Value&&` means that `value` can be an r-value reference. That means in the call `update_value`, the first argument can be a `lambda` written in the function call. See the `input_variable` example at the end. `fetch` is also capable of holding an r-value reference, `const Fetch&` binds to everything. However, unlike `value` it does not need to return an l-value reference. The output of `fetch` is meant to be copied to the output of `value`. Often, `fetch` will return something like `(*face)["input"_s]`. It should be an r-value reference. See the `input_variable` example at the end. The `update_value` function calls `init_cache()` before doing anything else. Allocating the cache if it has not been done yet. `set_output` is a similar function. The first argument is an r-value reference to a lambda and outputs an l-value reference. The second argument is a template `const T`. As is typical in CHM, `T` is a primitive type, typically a `double`. So not making `T` a reference is acceptable. The `output` object is, like the `value` object, a lambda returning an l-value reference to a member of `cache_` so that it can be modified. Therefore, since there is no restriction on `CacheType` members, it could certainly be another type. If, later, output could return an `std::vector` or another type of object that is expensive to copy, this could be modified. Note that in the `set_output` function there is the following line: ```cpp output() += t; ``` And note that the outputs are defaulted to 0. What this does is that it allowed output to be accumulated or assigned many times. Within one submodule is unnessary flexibility but if a module has several outputs then this allows it to be changed several times. Final note: Its possible, especially for state variables in hydrology, to increase or decrease. This could happen in submodule 1 but not submodule 2. In which case, getters can be made for output variables within a submodule. So then a submodule can have the "starting value" modify that and later "assign" it to a member function of data. However, that member should **should not** use `set_output` because of the incrementation, instead it should have its own logic. In addition, state variables should never be on the cache. **Examples** ```cpp class data : public data_base { public: // Example of an input/provides double input_variable(); // Example of domain wide parameter set in the config double config_parameter(); // Example of parameter access from global instance int get_dt(); // Example of output function void output_variable(const double in); // Example of a parameter that varies on each face double spatial_parameter(); }; double data::input_variable() const { update_value([this]() -> auto& { return cache_->input_variable;}, [this]() { return (*face)["input_variable"_s];} ); return cache_->input_variable; }; ``` input2() would be the same as above (with variables and string changed) ```cpp double data::config_parameter() const { static const double result = cfg_.get("config_parameter",1.0); return result; }; double data::spatial_parameter() const { update_value([this]() -> auto& { return cache_->spatial_parameter; }, [this]() -> { return face->veg_parameter("spatial_parameter"_s); } ); return cache_->spatial_parameter; }; int data::get_dt() const { // Don't actually need to do static const double dt = global_param->dt(); return dt; }; void data::output_variables(const double out) { set_output([this]() -> auto& { return cache_->output_variable; },out); }; void module_name::run(mesh_elem& face) { // run whatever calculations here, using the data object // manually reset the cache d.reset_cache(); }; ``` --- src/modules/submodules/data_base.hpp | 212 +++++++++++++++++++++++++++ 1 file changed, 212 insertions(+) create mode 100644 src/modules/submodules/data_base.hpp diff --git a/src/modules/submodules/data_base.hpp b/src/modules/submodules/data_base.hpp new file mode 100644 index 00000000..bd3fd95f --- /dev/null +++ b/src/modules/submodules/data_base.hpp @@ -0,0 +1,212 @@ +// +// Canadian Hydrological Model - The Canadian Hydrological Model (CHM) is a novel +// modular unstructured mesh based approach for hydrological modelling +// Copyright (C) 2018 Christopher Marsh +// +// This file is part of Canadian Hydrological Model. +// +// Canadian Hydrological Model is free software: you can redistribute it and/or +// modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Canadian Hydrological Model is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Canadian Hydrological Model. If not, see +// . +// + +// +// Created by Donovan Allum 2025 +// + +#pragma once + +#include "global.hpp" +#include "triangulation.hpp" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/** + * @brief Base class for data management with lazy caching and output handling. + * + * This class provides a framework for managing data with optional caching, + * lazy evaluation, and output assignment. It uses CRTP-like patterns with + * concepts to enforce interface contracts at compile-time. + * + * Key features: + * - Automatic cache initialization and staleness checking + * - Lazy evaluation of values with cache-aware updates + * - Type-safe output assignment with runtime checks + * - Compile-time concept enforcement for derived cache types and value/output providers + * + * The class is designed for computational scenarios where: + * - Data may be expensive to compute and should be cached + * - Cache validity depends on external timestep counters + * - Values need to be fetched lazily when accessed + * - Outputs must be accumulated safely + * + * Usage example: + * @code + * struct MyCache : public cache_base { + * double temperature = default_value(); + * int iteration_count = default_value(); + * }; + * + * class MyData : public data_base { + * public: + * MyData(const mesh_elem& face, const boost::shared_ptr param, + * const pt::ptree& cfg) : data_base(face, param, cfg) {} + * + * void compute_temperature() { + * update_value( + * [this]() -> auto& { return cache_->temperature; }, + * [this]() { return expensive_temperature_calculation(); } + * ); + * } + * + * void add_to_output(double contribution) { + * set_output( + * [this]() -> auto& { return output_variable; }, + * contribution + * ); + * } + * }; + * @endcode + * + * @tparam CacheType A type derived from cache_base that provides storage + * for cached values. Must satisfy the CacheRules concept. + */ +struct cache_base +{ + int64_t last_timestep = -1; + + template + static constexpr T default_value() { + if constexpr (std::is_floating_point_v) + return std::numeric_limits::quiet_NaN(); + else if constexpr (std::is_integral_v) + return std::numeric_limits::min(); + else + return T{}; + }; + +}; + +template +concept CacheRules = std::derived_from; + +template +concept ValueRules = + requires(V v) { + {v()} -> std::same_as>; +}; + +template +concept OutputRules = ValueRules && +requires(O o) +{ + {o()} -> std::convertible_to; +}; + +namespace pt = boost::property_tree; + +template +class data_base { + + template + bool constexpr check_if_set(T t) + { + if constexpr (!std::is_floating_point_v) + return t == cache_base::default_value(); + else + return std::isnan(t); + }; + + bool is_stale(); + + void init_cache(); + +protected: + + data_base(const mesh_elem& face_in, const boost::shared_ptr param, + const pt::ptree& cfg, const bool istest = false); + ~data_base() {}; + + const mesh_elem face{nullptr}; + const boost::shared_ptr global_param; + const pt::ptree& cfg_; + mutable std::optional cache_; + + template + void update_value(Value&& value, const Fetch& fetch); + + template Output> + void set_output(Output&& output,const T t); + +public: + void reset_cache() { cache_.reset(); }; + const std::optional& get_cache() { return cache_; }; +}; + +template +void data_base::init_cache() { + if (!cache_ || is_stale()) { + cache_.emplace(); + cache_->last_timestep = global_param->timestep_counter; + } +} + +template +bool data_base::is_stale() +{ + return cache_->last_timestep != global_param->timestep_counter; +} + +template +data_base::data_base(const mesh_elem& face_in, const boost::shared_ptr param, + const pt::ptree& cfg, const bool istest) : face(face_in), global_param(param), cfg_(cfg) +{ + // Optional istest parameter only exists to skip these tests during tests of this class where we aren't testing whether the face object has been set correctly. + // This means tests show that the underlying functions work as intended + if (!face->is_valid() && !istest) + throw std::invalid_argument("Face handle points to an invalid face"); + + if (!global_param && !istest) + throw std::invalid_argument("global parameter holder is null"); +}; + +template +template +void data_base::update_value(Value&& value, const Fetch& fetch) { + init_cache(); + + auto& V = value(); + if ( check_if_set(V) ) + { + V = fetch(); + } + +}; + +template +template Output> +void data_base::set_output(Output&& output,const T t) +{ + init_cache(); + + output() += t; +}; From 23aac4e66d1fa88223824220a31c7bb3a9094cff Mon Sep 17 00:00:00 2001 From: Allum Date: Fri, 5 Dec 2025 16:11:49 -0700 Subject: [PATCH 07/21] Test of `data_base` and adding to CMakeLists `test_data_base` defines a mock cache struct: `MockCache` and a mock data class: `data`, as well as the test fixture for GoogleTests. 6 tests: 1. CacheInitializesOnFirstUpdate 2. CacheRespectsTimestepChanges 3. ManualCacheResetWorks 4. SetOutputUpdatesValue 5. OnlyUpdatesNaNValues 6. ChecksDefaultValue `src/CMakeLists.txt` defines several test `.cpp` files. At least one is not functional. `test_triangulation.cpp` failed. Disabled the rest until they can be tested. Added `submoduletests/test_data_base.cpp` to the list. --- src/CMakeLists.txt | 18 +-- src/tests/submoduletests/test_data_base.cpp | 126 ++++++++++++++++++++ 2 files changed, 136 insertions(+), 8 deletions(-) create mode 100644 src/tests/submoduletests/test_data_base.cpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index ecd6505e..c058825c 100755 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -110,6 +110,7 @@ set (HEADER_FILES modules/interp_met modules/snobal modules/testing + modules/submodules math libmaw interpolation @@ -412,17 +413,18 @@ if (BUILD_TESTS) message(STATUS "Tests enabled. Run with make check") set(TEST_SRCS - tests/test_station.cpp - tests/test_interpolation.cpp - tests/test_timeseries.cpp - tests/test_core.cpp - tests/test_variablestorage.cpp - tests/test_metdata.cpp - tests/test_netcdf.cpp + #tests/test_station.cpp + # tests/test_interpolation.cpp + # tests/test_timeseries.cpp + # tests/test_core.cpp + # tests/test_variablestorage.cpp + # tests/test_metdata.cpp + # tests/test_netcdf.cpp # test_mesh.cpp tests/test_regexptokenizer.cpp # test_daily.cpp - tests/test_triangulation.cpp + # tests/test_triangulation.cpp + tests/submoduletests/test_data_base.cpp tests/main.cpp ) diff --git a/src/tests/submoduletests/test_data_base.cpp b/src/tests/submoduletests/test_data_base.cpp new file mode 100644 index 00000000..6144990f --- /dev/null +++ b/src/tests/submoduletests/test_data_base.cpp @@ -0,0 +1,126 @@ +#include +#include "data_base.hpp" +#include +// Mock Cache for Testing +struct MockCache : public cache_base { + double value = default_value(); +}; + +// Mock data class +class data : public data_base +{ +public: + data(mesh_elem face, boost::shared_ptr param, pt::ptree& cfg) + : data_base(face,param,cfg,true) {}; + ~data() {}; + + // update_value is private to data and therefore must be access in a function + // rather than use directly in the test. + template + void update(V&& v, L&& l) + { + update_value(v, l); + }; + + // Same with update_value + template + void output(V&& output, const T& t) + { + set_output(output,t); + }; + + // Again, to access private member + double& get_value() + { + return cache_->value; + }; +}; + +// Test Fixture +class DataBaseTest : public ::testing::Test { +protected: + void SetUp() override { + mock_global = boost::make_shared(); + } + + + // mock components for the data constructor + mesh_elem mock_face; + boost::shared_ptr mock_global; + pt::ptree mock_cfg; + + // Lambdas for testing, standing in for face access + static inline auto A = []() -> auto& { static double a = 42.0; return a;}; + static inline auto B = []() -> auto& { static double b = 123.0; return b;}; + + +}; + + +TEST_F(DataBaseTest, CacheInitializesOnFirstUpdate) { + data db(mock_face, mock_global, mock_cfg); + + db.update([&]() -> auto& { return db.get_value();},A); + + EXPECT_FALSE(std::isnan(db.get_value())); + EXPECT_EQ(db.get_value(), A()); +} + +TEST_F(DataBaseTest, CacheRespectsTimestepChanges) { + data db(mock_face, mock_global, mock_cfg); + + // First call at timestep 0 + mock_global->timestep_counter = 0; + db.update([&]() -> auto& {return db.get_value();} ,B); + + // Second call at same timestep - should use cached value + db.update([&]() -> auto& {return db.get_value();} ,A); + EXPECT_EQ(db.get_value(), B()); // No update to B + + // Force cache reset by changing timestep + mock_global->timestep_counter = 1; + db.update([&]() -> auto& {return db.get_value();} ,A); + EXPECT_EQ(db.get_value(), A()); // Updates +} + +TEST_F(DataBaseTest, ManualCacheResetWorks) { + data db(mock_face, mock_global, mock_cfg); + + db.update([&]() -> auto& { return db.get_value(); },A); + db.reset_cache(); + + db.update([&]() -> auto& {return db.get_value(); },B); + EXPECT_EQ(db.get_value(), B()); +} + +TEST_F(DataBaseTest, SetOutputUpdatesValue) { + data db(mock_face, mock_global, mock_cfg); + double output = 0.0; + static constexpr double pi = 3.14; + EXPECT_FALSE(db.get_cache()); + db.output([&]() -> auto& {return output;}, pi); + EXPECT_EQ(output, pi); + EXPECT_TRUE(db.get_cache()); +} + +TEST_F(DataBaseTest, OnlyUpdatesNaNValues) { + data db(mock_face, mock_global, mock_cfg); + double test_value = 10.0; // Not NaN + auto L = [&]() -> auto& {return test_value;}; + db.update(L, A); + EXPECT_EQ(L(), 10.0); // Should remain unchanged +} + +TEST_F(DataBaseTest, ChecksDefaultValue) +{ + double double_var = MockCache::default_value(); + float float_var = MockCache::default_value(); + int int_var = MockCache::default_value(); + size_t size_t_var = MockCache::default_value(); + + EXPECT_TRUE(std::isnan(double_var)); + EXPECT_TRUE(std::isnan(float_var)); + EXPECT_TRUE(int_var == std::numeric_limits::min()); + EXPECT_TRUE(size_t_var == std::numeric_limits::min()); +}; + From 48805f3d6156d682f43ca09dc68670ebb585c476 Mon Sep 17 00:00:00 2001 From: Allum Date: Tue, 9 Dec 2025 12:51:28 -0700 Subject: [PATCH 08/21] Fixing comments Documentation in `data_base.hpp` fixed to match class. Comment in `test_data_base.cpp` changed to add clarity. --- src/modules/submodules/data_base.hpp | 5 +++-- src/tests/submoduletests/test_data_base.cpp | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/modules/submodules/data_base.hpp b/src/modules/submodules/data_base.hpp index bd3fd95f..c5dbaf53 100644 --- a/src/modules/submodules/data_base.hpp +++ b/src/modules/submodules/data_base.hpp @@ -68,8 +68,8 @@ * * class MyData : public data_base { * public: - * MyData(const mesh_elem& face, const boost::shared_ptr param, - * const pt::ptree& cfg) : data_base(face, param, cfg) {} + * MyData(const mesh_elem& face_in, const boost::shared_ptr param, + * const pt::ptree& cfg) : data_base(face_in, param, cfg) {} * * void compute_temperature() { * update_value( @@ -90,6 +90,7 @@ * @tparam CacheType A type derived from cache_base that provides storage * for cached values. Must satisfy the CacheRules concept. */ + struct cache_base { int64_t last_timestep = -1; diff --git a/src/tests/submoduletests/test_data_base.cpp b/src/tests/submoduletests/test_data_base.cpp index 6144990f..bb08563f 100644 --- a/src/tests/submoduletests/test_data_base.cpp +++ b/src/tests/submoduletests/test_data_base.cpp @@ -22,7 +22,7 @@ class data : public data_base update_value(v, l); }; - // Same with update_value + // Same as update_value template void output(V&& output, const T& t) { @@ -75,7 +75,8 @@ TEST_F(DataBaseTest, CacheRespectsTimestepChanges) { // Second call at same timestep - should use cached value db.update([&]() -> auto& {return db.get_value();} ,A); - EXPECT_EQ(db.get_value(), B()); // No update to B + EXPECT_EQ(db.get_value(), B()); // value remains B. Should not update from B to A + // at the same time step // Force cache reset by changing timestep mock_global->timestep_counter = 1; From 380fa3f9a36bed59e09b9cea50e24d74b8fa7844 Mon Sep 17 00:00:00 2001 From: Allum Date: Tue, 9 Dec 2025 12:55:11 -0700 Subject: [PATCH 09/21] Updating `data_base` Concepts 1. OutputRules concept now tests to ensure that the += operator functions. 2. Concepts moved to a namespace to avoid name collision as these templates are included in every TU that includes `data_base`. --- src/modules/submodules/data_base.hpp | 51 +++++++++++++++------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/src/modules/submodules/data_base.hpp b/src/modules/submodules/data_base.hpp index c5dbaf53..e5e61252 100644 --- a/src/modules/submodules/data_base.hpp +++ b/src/modules/submodules/data_base.hpp @@ -107,25 +107,28 @@ struct cache_base }; -template -concept CacheRules = std::derived_from; - -template -concept ValueRules = - requires(V v) { - {v()} -> std::same_as>; -}; +namespace data_base_concepts { + template + concept CacheRules = std::derived_from; + + template + concept ValueRules = + requires(V v) { + {v()} -> std::same_as>; + }; -template -concept OutputRules = ValueRules && -requires(O o) -{ - {o()} -> std::convertible_to; + template + concept OutputRules = ValueRules && + requires(O o,const T t) + { + {o()} -> std::convertible_to; + {o() += t}; + }; }; - + namespace pt = boost::property_tree; -template +template class data_base { template @@ -152,10 +155,10 @@ class data_base { const pt::ptree& cfg_; mutable std::optional cache_; - template + template void update_value(Value&& value, const Fetch& fetch); - template Output> + template Output> void set_output(Output&& output,const T t); public: @@ -163,7 +166,7 @@ class data_base { const std::optional& get_cache() { return cache_; }; }; -template +template void data_base::init_cache() { if (!cache_ || is_stale()) { cache_.emplace(); @@ -171,13 +174,13 @@ void data_base::init_cache() { } } -template +template bool data_base::is_stale() { return cache_->last_timestep != global_param->timestep_counter; } -template +template data_base::data_base(const mesh_elem& face_in, const boost::shared_ptr param, const pt::ptree& cfg, const bool istest) : face(face_in), global_param(param), cfg_(cfg) { @@ -190,8 +193,8 @@ data_base::data_base(const mesh_elem& face_in, const boost::shared_pt throw std::invalid_argument("global parameter holder is null"); }; -template -template +template +template void data_base::update_value(Value&& value, const Fetch& fetch) { init_cache(); @@ -203,8 +206,8 @@ void data_base::update_value(Value&& value, const Fetch& fetch) { }; -template -template Output> +template +template Output> void data_base::set_output(Output&& output,const T t) { init_cache(); From 9f066f20d618d2180151ec5f31191d8e7d6f1eb2 Mon Sep 17 00:00:00 2001 From: Allum Date: Tue, 9 Dec 2025 14:10:41 -0700 Subject: [PATCH 10/21] Changed function name for clarity --- src/modules/submodules/data_base.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/modules/submodules/data_base.hpp b/src/modules/submodules/data_base.hpp index e5e61252..d9c45947 100644 --- a/src/modules/submodules/data_base.hpp +++ b/src/modules/submodules/data_base.hpp @@ -132,7 +132,7 @@ template class data_base { template - bool constexpr check_if_set(T t) + bool constexpr is_unset(T t) { if constexpr (!std::is_floating_point_v) return t == cache_base::default_value(); @@ -199,7 +199,7 @@ void data_base::update_value(Value&& value, const Fetch& fetch) { init_cache(); auto& V = value(); - if ( check_if_set(V) ) + if ( is_unset(V) ) { V = fetch(); } From d18f1c58a0917b62d47927496f7c3db43714bbf6 Mon Sep 17 00:00:00 2001 From: Allum Date: Tue, 9 Dec 2025 14:11:01 -0700 Subject: [PATCH 11/21] New test-only constructor for `data_base` A second constructor now exists so that instatiations of a test version of `data_base` must be intentional and only succeeds by setting the flag to `true`. --- src/modules/submodules/data_base.hpp | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/modules/submodules/data_base.hpp b/src/modules/submodules/data_base.hpp index d9c45947..91b373fa 100644 --- a/src/modules/submodules/data_base.hpp +++ b/src/modules/submodules/data_base.hpp @@ -147,7 +147,10 @@ class data_base { protected: data_base(const mesh_elem& face_in, const boost::shared_ptr param, - const pt::ptree& cfg, const bool istest = false); + const pt::ptree& cfg); + // Test constructor to skip checks of valid mesh_elem + data_base(const mesh_elem& face_in, const boost::shared_ptr param, + const pt::ptree& cfg, bool istest); ~data_base() {}; const mesh_elem face{nullptr}; @@ -182,17 +185,22 @@ bool data_base::is_stale() template data_base::data_base(const mesh_elem& face_in, const boost::shared_ptr param, - const pt::ptree& cfg, const bool istest) : face(face_in), global_param(param), cfg_(cfg) + const pt::ptree& cfg) : face(face_in), global_param(param), cfg_(cfg) { - // Optional istest parameter only exists to skip these tests during tests of this class where we aren't testing whether the face object has been set correctly. - // This means tests show that the underlying functions work as intended - if (!face->is_valid() && !istest) + if (!face->is_valid()) throw std::invalid_argument("Face handle points to an invalid face"); - if (!global_param && !istest) + if (!global_param) throw std::invalid_argument("global parameter holder is null"); }; +template +data_base::data_base(const mesh_elem& face_in, const boost::shared_ptr param, + const pt::ptree& cfg, const bool istest) : face(face_in), global_param(param), cfg_(cfg) +{ + if (!istest) + std::invalid_argument("data_base test constructor called with False istest flag. Should be true."); +}; template template void data_base::update_value(Value&& value, const Fetch& fetch) { From 4c8f79fbdb534bcfdcd49a359c1eac36eea7d8d7 Mon Sep 17 00:00:00 2001 From: Allum Date: Tue, 9 Dec 2025 15:20:27 -0700 Subject: [PATCH 12/21] get_cache() is now const --- src/modules/submodules/data_base.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/submodules/data_base.hpp b/src/modules/submodules/data_base.hpp index 91b373fa..0de1e8ac 100644 --- a/src/modules/submodules/data_base.hpp +++ b/src/modules/submodules/data_base.hpp @@ -166,7 +166,7 @@ class data_base { public: void reset_cache() { cache_.reset(); }; - const std::optional& get_cache() { return cache_; }; + const std::optional& get_cache() const { return cache_; }; }; template From 041b186b18553a0ae343c36e47e6ac038f5409ce Mon Sep 17 00:00:00 2001 From: Donovan James McMurrich Allum <84050485+djmallum@users.noreply.github.com> Date: Tue, 9 Dec 2025 15:25:19 -0700 Subject: [PATCH 13/21] Removed name signature from base_step.hpp --- src/modules/submodules/base_step.hpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/modules/submodules/base_step.hpp b/src/modules/submodules/base_step.hpp index 0b0ac868..bb632dd7 100644 --- a/src/modules/submodules/base_step.hpp +++ b/src/modules/submodules/base_step.hpp @@ -21,10 +21,6 @@ // . // -// -// Created by Donovan Allum 2025 -// - #pragma once #include From a14e5573fce8332cf851386b9a4bf9035435709b Mon Sep 17 00:00:00 2001 From: Allum Date: Thu, 11 Dec 2025 15:12:50 -0700 Subject: [PATCH 14/21] Added missing include --- src/tests/submoduletests/test_data_base.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/submoduletests/test_data_base.cpp b/src/tests/submoduletests/test_data_base.cpp index bb08563f..ffbfe65e 100644 --- a/src/tests/submoduletests/test_data_base.cpp +++ b/src/tests/submoduletests/test_data_base.cpp @@ -1,5 +1,6 @@ #include #include "data_base.hpp" +#include "triangulation.hpp" #include // Mock Cache for Testing struct MockCache : public cache_base { @@ -124,4 +125,3 @@ TEST_F(DataBaseTest, ChecksDefaultValue) EXPECT_TRUE(int_var == std::numeric_limits::min()); EXPECT_TRUE(size_t_var == std::numeric_limits::min()); }; - From 52f2ffaa7b18bf517916011b86d4932fea9e0dc5 Mon Sep 17 00:00:00 2001 From: Allum Date: Thu, 11 Dec 2025 15:13:16 -0700 Subject: [PATCH 15/21] Added new test for basic submodule, using base_step and data_base --- src/tests/submoduletests/test_submodule.cpp | 99 +++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 src/tests/submoduletests/test_submodule.cpp diff --git a/src/tests/submoduletests/test_submodule.cpp b/src/tests/submoduletests/test_submodule.cpp new file mode 100644 index 00000000..a2af35fd --- /dev/null +++ b/src/tests/submoduletests/test_submodule.cpp @@ -0,0 +1,99 @@ +#include +#include +#include "data_base.hpp" +#include "base_step.hpp" +#include "triangulation.hpp" + +template + +concept SubmoduleData = requires(T& t) +{ + {t.input1()} -> std::floating_point; + {t.input2()} -> std::floating_point; + + {t.output(std::declval())} -> std::same_as; +}; + +template +class submodule : public base_step,data> +{ +public: + submodule() {}; + ~submodule() {}; + + void execute_impl(data& d) const + { + auto input1 = d.input1(); + auto input2 = d.input2(); + + auto output = input1 * input2; + d.output(output); + } +}; + +struct Cache : public cache_base +{ + double input1 = default_value(); + double input2 = default_value(); + + double output = 0.0; +}; + +class data2 : public data_base +{ +public: + data2(mesh_elem face_in,boost::shared_ptr param,pt::ptree& cfg) + : data_base(face_in,param,cfg,true) {}; + + double input1() + { + update_value( + [this]() -> auto& { return cache_->input1;}, + []() { return 4.0; } + ); + + return cache_->input1; + }; + + double input2() + { + update_value( + [this]() -> auto& { return cache_->input2;}, + []() { return 2.5; } + ); + + return cache_->input1; + }; + + void output(const double t) + { + set_output([this]() -> auto& { return cache_->output; },t); + }; +}; + +class TestModule : public ::testing::Test +{ +protected: + TestModule() : d(face,param,cfg) {}; + mesh_elem face{nullptr}; + boost::shared_ptr param; + pt::ptree cfg; + + data2 d; + submodule submodule_instance; + void run() + { + submodule_instance.execute(d); + }; +}; + +TEST_F(TestModule,SimpleSubmoduleTest) +{ + const auto& cache = d.get_cache(); + EXPECT_FALSE(cache); + run(); + EXPECT_TRUE(cache); + EXPECT_EQ(cache->input1,4.0); + EXPECT_EQ(cache->input2,2.5); + EXPECT_DOUBLE_EQ(cache->output,cache->input1 * cache->input2); +}; From de349bf16da29205c6ed182d741c361d8d72daea Mon Sep 17 00:00:00 2001 From: Allum Date: Fri, 23 Jan 2026 12:04:11 -0700 Subject: [PATCH 16/21] Adding .rst file with instructions These instructions walk through a simple example to writing a submodule and adding it to a module. --- docs/submodule_instructions.rst | 447 ++++++++++++++++++++++++++++++++ 1 file changed, 447 insertions(+) create mode 100644 docs/submodule_instructions.rst diff --git a/docs/submodule_instructions.rst b/docs/submodule_instructions.rst new file mode 100644 index 00000000..6208d008 --- /dev/null +++ b/docs/submodule_instructions.rst @@ -0,0 +1,447 @@ +How To: Submodule Framework +============================ + +The CHM modules, for good reason, allow the user to design their module however they'd like as long as they couple the calculations to the CHM coupler that coordinates the modules and the data passing between them. Modules can be subdivided into two categories: + +1. External models coupled to CHM. +2. Models coded in CHM directly. + +Being able to accommodate both is one of CHMs biggest strengths (the same can be said for most modular models). Modules of the first type are only possible when the models are reasonably portable and adaptable to any framework. It can be readily seen by reading these modules that a lot of work is required in order to make it mesh (pun intended) with CHM. And ultimately that is not surprising. It was not designed with CHM in mind. However, given that we have full control over CHM modules, it makes sense to design a consistent framework for writing them. And that framework should go beyond implementing the virtual functions ``void module_base::run(mesh_elem&)`` and ``void module_base::init(mesh&)``, and using a class derived from ``face_info`` to store per-triangle data. + +If no framework - or *encouraged* - design exists, then modules will either become so completely different that learning each is equivalent to learning a new model each time, or they will be coded in such a manner that testability, extensibility, flexibility, and readability fall to the back burner. + +In this document, I'll present basic instructions and guidelines for designing submodules and then how to write (or modify) a module that uses them. + +Designing Submodules +-------------------- + +Avoiding the dirty details of the why, here is the how. + +Step 1 +^^^^^^ + +Open a new file in ``src/modules/submodules`` with a descriptive name. For this we will use ``submodule_example.hpp``. In this file, immediately create a namespace with the same name as the file and nest inside of it a class called ``Model`` derived from ``base_step,Data>`` where ``Data`` is a template parameter. More on this later... + +The ``Model`` class should have one public function: ``execute_impl(Data& d)``. It will look something like this: + +.. code-block:: cpp + + #include "base_step.hpp" + + namespace submodule_example + { + template + class Model : public base_step,Data> + { + public: + void execute_impl(Data& d); + }; + }; + +Step 2 +^^^^^^ + +Take stock of your calculation and write out a list of inputs it needs and outputs it provides (similar to CHM). Then write a concept to enforce that ``Data`` has getters and setters for both of these. + +.. code-block:: cpp + + #include "base_step.hpp" + #include + + namespace submodule_example + { + template + concept submodule_data = requires(T& t) + { + { t.input1(); } -> std::floating_point; + { t.input2(); } -> std::floating_point; + + { t.output1(std::declval()); } -> std::same_as; + { t.output2(std::declval()); } -> std::same_as; + }; + + template + class Model : public base_step,Data> + { + public: + void execute_impl(Data& d); + }; + }; + +I've left out implementing ``execute_impl(Data&)`` on purpose for now. + +Step 3 (optional) +^^^^^^^^^^^^^^^^^ + +Immediately write tests. Navigate to ``src/tests/submoduletests/`` and create a file with a good name like ``test_submodule_example.cpp``. The body will look something like this: + +.. code-block:: cpp + + #include + #include "submodule_example.hpp" + + class data_for_test + { + // Implement example data class to satisfy the concept above + }; + + class SubmoduleExampleTest : public ::testing::Test + { + protected: // Must be protected!!! + submodule_example submodule; + }; + + TEST_F(SubmoduleExampleTest,TestZerosReturnsZeros) + { + // TEST_F is a macro to create a class derived from SubmoduleExampleTest. So the `submodule` object already exists. + // Operate on that are use macros like EXPECT_EQ(expected,produced); to check if the submodule is working as expected. + // Test BEHAVIOURS. + } + +If you are having trouble designing a test as the result of designing increasingly contrived situations to test obscure edge cases that will actually compile, or you feel yourself wanting to test private methods, this is a `code smell `_. I would recommend then further encapsulating these calculations into another class, defined within the ``submodule_example`` namespace, with a public, testable interface. Then test that. If you had an ``std::vector`` private member, you wouldn't test the ``std::vector`` through the public interface, you'd assume (rightly) it was tested elsewhere. + +Why write tests first +^^^^^^^^^^^^^^^^^^^^^ + +It's called Test Driven Development. It exists to speed up development by spending time early on tests and later benefiting from the pre-planning. A common expression I've read is "If you have a bug, that means there is a missing test". + +Step 4 +^^^^^^ + +Return to ``submodule_example.hpp`` and iterate until it passes your tests. Tada! Now you know its working. Don't forget to add ``test_submodule_example.cpp`` to your ``CMakeLists.txt`` and enable the testing flag. I prefer to compile once, then open ``CMakeCache.txt`` in my build directory and change ``BUILD_TESTS`` to ``ON``, then recompile. It saves having to deal with changing the ``CMakeLists.txt`` every time while trying to commit changes to the actual source later. + +**You're done!** You've successfully written a submodule. The next step is to include it in a CHM module. + +Designing Modules with Submodules +--------------------------------- + +For simplicity, we will assume that we are starting from scratch to build this module, but these instructions can easily be applied to an existing module, either to change a calculation to a submodule or add a submodule as a separate calculation. Write the header as normal with a nice an descriptive name, here we will use ``module_example.hpp``. The way we do this is make use of the class ``data_base`` for automatic caching and some other tools. + +Step 1: Write the Cache +^^^^^^^^^^^^^^^^^^^^^^^ + +First, you need a struct (classes are OK) to act as the cache. The short of it is that you require a member for outputs and a member for inputs. Therefore our Cache is + +.. code-block:: cpp + + struct Cache : public cache_base + { + double input1 = default_value(); + double input2 = default_value(); + + double output1 = 0.0; + double output2 = 0.0; + }; + +``default_value()`` comes from the ``cache_base`` parent class. The details don't matter too much here, but it initializes the input as a sentinel value so that each member can separately be lazy-initialized. + +Step 2: Write the data class +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The ``data`` class is the class that holds the per-triangle data for a module between time steps. This is the class that will be passed as a template parameter to the submodule. Therefore, it must satisfy the concept we defined in ``submodule_example.hpp``. In order to use the cache, it also must be derived from ``data_base``. The Cache as a template parameter sets the type of the cache used behind the scenes. + +Lazy initialization of the input variables on the cache is only possible if ``data`` has a reference to the ``mesh_elem`` object it corresponds to, the ``config_file`` object, and the ``global`` shared pointer. So ``data_base`` has a constructor to pass these references. therefore, the data class is as follows: + +.. code-block:: cpp + + class data : data_base + { + public: + double input1() const; + double input2() const; + + double output1(const double out_val) const; + double output2(const double out_val) const; + + using data_base::data_base; + // Alternatively, you can write out the constructor as follows: + // data(mesh_elem& face_in, std::shared_ptr param, config_file& cfg) + // : data_base(face_in,param,cfg); + }; + +Step 3: Write the Module header +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Put it all together. I recommend placing the ``data`` and ``Cache`` classes inside the body of the module. Like so: + +.. code-block:: cpp + + // Essential CHM includes here + #include "submodule_example.hpp" + + class module_example : public module_base + { + public: + void run(mesh_elem&) override; + void init(mesh&) override; + + struct Cache : public cache_base + { + double input1 = default_value(); + double input2 = default_value(); + + double output1 = 0.0; + double output2 = 0.0; + }; + + class data : data_base + { + public: + double input1() const; + double input2() const; + + double output1(const double out_val) const; + double output2(const double out_val) const; + + using data_base::data_base; + }; + private: + submodule_example submodule_obj; + } + +Step 4: Implement the ``data`` member functions +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Switching to ``module_example.cpp``, we implement the auto-caching using the protected member function from the ``data_base`` class for the inputs and another protected member for outputs. These functions automatically lazy initialize the cache. I will admit here that this is possibly the ugliest part of the implementation and could use improvements. Nonetheless, here is how you do it. You call the function ``update_value`` which expects two arguments, each argument must be a callable. I intend for these to be lambda functions but it may work with function pointers, but I have not tested that. The first lambda returns a reference to the cache member to be accessed, and the second lambda returns a copy of the dereferenced ``face`` object. ``update_value`` calls the second lambda if the cache has not be initialized yet or if the input variable is equal to ``default_value()`` that we initialized the inputs to in the ``module_example::Cache``. + +.. code-block:: cpp + + double module_example::data::input1() + { + update_value( [this]() -> auto& { return cache_->input1; }, + [this]() { return (*face)["input1"_s];} + ); + + return cache_->input1; + } + + double module_example::data::input2() + { + update_value( [this]() -> auto& { return cache_->input2; }, + [this]() { return (*face)["input2"_s];} + ); + + return cache_->input2; + } + +Note that in all four lambdas, ``this`` is captured. ``This`` allows the use of ``face`` and ``cache_`` objects. I means that both lambdas have a copy of the ``this`` pointer (not the whole class) so the overhead is minimal and only stored during the call to ``update_value`` and freed immeidately after the function call. + +``update_value`` is a templated function and therefore the compiler generates a unique function for each unique pair of lambdas. In addition, the arguments are r-value references so these temporary lambdas are passes correctly. The first lambda **must** return a reference. Luckily, concepts hidden in ``data_base`` ensure this is the case, so it won't compile if you forget. This allows the first lambda to be assigned. + +Now for the output functions, its very similar and uses a function called ``set_output`` that likewise accepts two arguments but in this case, only the first argument is a lambda reference to the cache member, and the second is simply forwarding the function argument. + +.. code-block:: cpp + + double module_example::data::output1(const double out_val) + { + set_output( [this]() -> auto& { return cache_->output1; }, out_val); + }; + + double module_example::data::output2(const double out_val) + { + set_output( [this]() -> auto& { return cache_->output2; }, out_val); + }; + +Other kinds of inputs +^^^^^^^^^^^^^^^^^^^^^ + +Not all inputs are created equal. Some are variables, accessed by a dereference from ``face``, but others might be per-triangle or domain-wide parameters. You can approach this however you'd like. But remember that ``cfg`` and ``global`` and ``face`` are all available. So... make use of them. + +For domain-wide parameters, I recommend adding a static variable to the data class, and assigning it in the ``module_example::init`` function, like so: + +.. code-block:: cpp + + // .hpp + + data : public data_base + { + public: + // other stuff + static double param1; + }; + + // .cpp + + void module_example::init(mesh& domain) + { + data::param1 = cfg.get("param1"); + }; + + static double module_example::data::param1() // Doesn't modify the state and only uses static members, so this can be static. + { + return param1; + }; + + // Another option + double module_example::data::param2() + { + //thread safe + //initialized once and then never again + //techincally has a slight overhead to check if its been set yet. + // Doesn't pollute the public interface of the data class. + static double param2 = cfg.get("param2"); + return param2; + } + +For triangle specific parameters, I would proceed exactly the same way as ``param1`` without the static keyword. + +.. code-block:: cpp + + // .hpp + + data : public data_base + { + public: + // other stuff + double param1; + }; + + // .cpp + + void module_example::init(mesh& domain) + { + // loop over every face + // Get the local instance of data via make_module_data, skipped here because I've modified it. + param1 = face->veg_parameter("param1"_s); + }; + + double module_example::data::param1() + { + return param1; + }; + + // Another option + // Static variagble in a functin cannot work because + // it is per CLASS not per instance and you need a unique value per triangle + // However, you could do the following: + + module_example::data::data(mesh_elem& face_in,std::shared_ptr param,config_file& cfg) + : data_base(face_in,param,cfg) + { + param1 = face->veg_parameter("param1"_s); + param2 = cfg.get("param2"_s); // here param2 is a static member of data. + }; + +In fact, that last example of setting them in the constructor is a bit genius and I actually haven't done it yet myself... welp guess I'll start doing that. + +The final type is something accessed via the ``global`` instance ``param``. These will often have to be called dynamically because things change. If it is a time step, set it in the constructor for ``data``, but if it is the time of day then one must instead set it at each call. If one wants to avoid calling ``global`` more than once per time step, feel free to add it as a cached member of ``Cache``. + +.. code-block:: cpp + + double module_example::data::hour() + { + // or whatever other function + return param->hour(); + } + +Step 5: Implement the module +^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Most proceeds as normal. Constructor sets the depends/provides, destructor is empty unless you were evil and used ``new``, be sure to ``delete`` those. + +If we use the method to set parameters in the constructor, the ``.cpp`` file will look like this: + +.. code-block:: cpp + + #include "module_example.hpp" + REGISTER_MODULE_CPP(module_example); + + module_example::module_example(config_file cfg) : module_base("module_example", parallel::data, cfg) + { + depends("input1"); + depends("input2"); + + provides("output1"); + provides("output2"); + } + + //empty deconstructor + + void module_example::init(mesh& domain) + { + // Recommend setting these outside the loop and not in the constructor. + // You create it so many times, it is unwise to set it each time, but + // overhead might be minimal + data::static_parameter = cfg.get("static_parameter"); + for (size_t i = 0; i < domain->size_local_faces(); ++i) + { + auto face = domain->face(i); + face->make_module_data(ID,face,global_param,cfg); + // Everything is set in the constructor... so no need do actually store a reference + // there may be some situations where a parameter determines how something is set + // In that case, store a reference to data and make whatever assignments you'd like. + } + } + + void module_example::run(mesh_elem& face) + { + auto& d = face->get_module_data(ID); + + submodule_example(d); + + d.set_module_outputs(); + } + + void module_example::data::set_module_outputs() + { + auto& c = get_cache(); + if (c) + { + (*face)["output1"_s] = (c->output1 == default_value()) ? c.output1 : 0.0; + (*face)["output2"_s] = (c->output2 == default_value()) ? c.output2 : 0.0; + } + else + { + (*face)["output1"_s] = 0.0; + (*face)["output2"_s] = 0.0; + } + } + +And now your module using submodules is complete! Notice that a function called ``make_module_data`` is called in the module virtual function ``init`` and its constructor now accepts more than just the module ``ID`` as an argument. I've rewritten just the ``make_module_data`` function to have a template of variadic parameters to pass to the constructor of the ``data`` object. + +Final Notes +----------- + +There are a few things that could be improved: + +1. The ``update_value`` and ``set_output`` protected ``data_base`` member functions. They are a bit ugly. + + One idea is to set these lambdas in the constructor and store them as std::functions. Not ideal really but doable. + +2. The style of setting input cache members is not ideal. ``double input1 = default_value();`` is redundant and ugly. + + One idea here is to create an ``input`` class that takes a parameter to a primitive type and auto sets it to ``default_value()``. Like so: + + .. code-block:: cpp + + class Cache : public cache_base + { + input input1; + input input2; + + output output1; // initialized to 0.0 + output output2; + } + + Perhaps ``input`` and ``output`` classes are where we could store lambdas for note 1 above? + +3. When you have many state variables, I recommend defining a struct like: + + .. code-block:: cpp + + namespace submodule_example + { + struct State + { + //state members here + }; + + // modify concept to expect a get_state function as a reference. + // Store a State object privately on data. + } + +Then, if during the testing you decide to break-up your submodule into several subclasses you can design them to operate with the ``State`` object, rather than on the ``data`` object. This nifty trick means you can test these classes (defined in the ``submodule_example`` namespace) without having to worry about the concept to enforce on the template parameter of ``submodule_example``. This also means that these classes can be compiled in a ``submodule_example.cpp`` file. + +4. Consider a situation with N modules. Now you'll have a very busy ``data`` class. Depending on what functions are enforced by their respective concepts, you could have multiple functions for air temperature: ``air_temp()``, ``air_temperature()``, ``temp()``, and so on. For these, implement one and have the others simply pass through to the implemented version. + + Likewise, only store a single cache member for all the air temperatures! Unit conversions can be done in the respective calls, if one module expects Celsius but the other Kelvin! From cb9bce29a5deed9ce5f1db9bf542a81d71e02530 Mon Sep 17 00:00:00 2001 From: Allum Date: Fri, 23 Jan 2026 12:10:48 -0700 Subject: [PATCH 17/21] Modified triangulation::make_module_data to accept variadic template `data_base` class has a constructor to add a mesh_elem reference, shared_ptr to a `global` object, and a const reference to a `config_file` object. These must be passed through the `make_module_data` constructor and this variadic template allows any number of arguments to be passed. --- src/mesh/triangulation.hpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/mesh/triangulation.hpp b/src/mesh/triangulation.hpp index 517df2e5..2f5346b5 100644 --- a/src/mesh/triangulation.hpp +++ b/src/mesh/triangulation.hpp @@ -436,9 +436,10 @@ class face : public Fb // void set_module_data(const std::string &module, face_info *fi); - template - T& make_module_data(const std::string &module); - + // Overload for module data constructor + template + T& make_module_data(const std::string &module, Args... args); + std::string _debug_name; //for debugging to find the elem that we want int _debug_ID; //also for debugging. ID == the position in the output order, starting at 0 size_t cell_global_id; @@ -1788,22 +1789,22 @@ timeseries::iterator face::now() } +// Overloaded for construtor that requires arguments template < class Gt, class Vb> -template -T& face::make_module_data(const std::string &module) +template +T& face::make_module_data(const std::string &module, Args... args) { //we don't already have this, make a new one. if(!_module_face_data[module]) { // T* data = new T; - _module_face_data[module] = std::make_unique(); + _module_face_data[module] = std::make_unique(args...); } return get_module_data(module); } - template < class Gt, class Fb> template < typename T> T& face::get_module_data(const std::string &module) From 2e73a570a72db693a86acbc7677f3cceb744a15d Mon Sep 17 00:00:00 2001 From: Allum Date: Tue, 24 Feb 2026 16:13:44 -0700 Subject: [PATCH 18/21] base_step: Added Derived as friend class --- src/modules/submodules/base_step.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/modules/submodules/base_step.hpp b/src/modules/submodules/base_step.hpp index bb632dd7..9a588262 100644 --- a/src/modules/submodules/base_step.hpp +++ b/src/modules/submodules/base_step.hpp @@ -56,6 +56,7 @@ template class base_step { protected: base_step() {}; + friend Derived; public: void execute(Data& d) const { From 492316eaf4c58669a7adb2a3403c92713215f04e Mon Sep 17 00:00:00 2001 From: Allum Date: Wed, 1 Apr 2026 16:01:47 -0600 Subject: [PATCH 19/21] cfg is held as a pointer, not as a const reference, but as a pointer --- src/modules/submodules/base_step.hpp | 1 - src/modules/submodules/data_base.hpp | 26 +++++++++++++------------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/modules/submodules/base_step.hpp b/src/modules/submodules/base_step.hpp index 9a588262..bb632dd7 100644 --- a/src/modules/submodules/base_step.hpp +++ b/src/modules/submodules/base_step.hpp @@ -56,7 +56,6 @@ template class base_step { protected: base_step() {}; - friend Derived; public: void execute(Data& d) const { diff --git a/src/modules/submodules/data_base.hpp b/src/modules/submodules/data_base.hpp index 0de1e8ac..6cae6d37 100644 --- a/src/modules/submodules/data_base.hpp +++ b/src/modules/submodules/data_base.hpp @@ -33,7 +33,6 @@ #include #include #include -#include #include #include #include @@ -68,8 +67,8 @@ * * class MyData : public data_base { * public: - * MyData(const mesh_elem& face_in, const boost::shared_ptr param, - * const pt::ptree& cfg) : data_base(face_in, param, cfg) {} + * MyData(const mesh_elem& face_in, const std::shared_ptr param, + * const pt::ptree* cfg) : data_base(face_in, param, cfg) {} * * void compute_temperature() { * update_value( @@ -93,6 +92,7 @@ struct cache_base { + cache_base() = default; int64_t last_timestep = -1; template @@ -146,16 +146,16 @@ class data_base { protected: - data_base(const mesh_elem& face_in, const boost::shared_ptr param, - const pt::ptree& cfg); + data_base(const mesh_elem& face_in, const std::shared_ptr param, + const pt::ptree* cfg); // Test constructor to skip checks of valid mesh_elem - data_base(const mesh_elem& face_in, const boost::shared_ptr param, - const pt::ptree& cfg, bool istest); + data_base(const mesh_elem& face_in, const std::shared_ptr param, + const pt::ptree* cfg, bool istest); ~data_base() {}; const mesh_elem face{nullptr}; - const boost::shared_ptr global_param; - const pt::ptree& cfg_; + const std::shared_ptr global_param; + const pt::ptree* const cfg; mutable std::optional cache_; template @@ -184,8 +184,8 @@ bool data_base::is_stale() } template -data_base::data_base(const mesh_elem& face_in, const boost::shared_ptr param, - const pt::ptree& cfg) : face(face_in), global_param(param), cfg_(cfg) +data_base::data_base(const mesh_elem& face_in, const std::shared_ptr param, + const pt::ptree* cfg) : face(face_in), global_param(param), cfg(cfg) { if (!face->is_valid()) throw std::invalid_argument("Face handle points to an invalid face"); @@ -195,8 +195,8 @@ data_base::data_base(const mesh_elem& face_in, const boost::shared_pt }; template -data_base::data_base(const mesh_elem& face_in, const boost::shared_ptr param, - const pt::ptree& cfg, const bool istest) : face(face_in), global_param(param), cfg_(cfg) +data_base::data_base(const mesh_elem& face_in, const std::shared_ptr param, + const pt::ptree* cfg, const bool istest) : face(face_in), global_param(param), cfg(cfg) { if (!istest) std::invalid_argument("data_base test constructor called with False istest flag. Should be true."); From 05a7db0328748da2a9c2e4178d4f48262c1b05b9 Mon Sep 17 00:00:00 2001 From: Allum Date: Wed, 1 Apr 2026 16:02:28 -0600 Subject: [PATCH 20/21] Fixed typo in data_base constructor --- src/modules/submodules/data_base.hpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/modules/submodules/data_base.hpp b/src/modules/submodules/data_base.hpp index 6cae6d37..3a8c4635 100644 --- a/src/modules/submodules/data_base.hpp +++ b/src/modules/submodules/data_base.hpp @@ -199,7 +199,10 @@ data_base::data_base(const mesh_elem& face_in, const std::shared_ptr< const pt::ptree* cfg, const bool istest) : face(face_in), global_param(param), cfg(cfg) { if (!istest) - std::invalid_argument("data_base test constructor called with False istest flag. Should be true."); + throw std::invalid_argument("data_base test constructor called with False istest flag. Should be true."); + + if (!global_param) + throw std::invalid_argument("global parameter holder is null"); }; template template From d64847880b716661fb87b0b58ba7cd8d75d5e330 Mon Sep 17 00:00:00 2001 From: Allum Date: Wed, 1 Apr 2026 16:02:57 -0600 Subject: [PATCH 21/21] added submodule test example, updated data_base test to use std::shared_ptr not boost::shared_ptr --- src/CMakeLists.txt | 1 + src/tests/submoduletests/test_data_base.cpp | 8 ++++---- src/tests/submoduletests/test_submodule.cpp | 12 ++++++------ 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index c058825c..d39bf7aa 100755 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -425,6 +425,7 @@ if (BUILD_TESTS) # test_daily.cpp # tests/test_triangulation.cpp tests/submoduletests/test_data_base.cpp + tests/submoduletests/test_submodule.cpp tests/main.cpp ) diff --git a/src/tests/submoduletests/test_data_base.cpp b/src/tests/submoduletests/test_data_base.cpp index ffbfe65e..7c60ef9e 100644 --- a/src/tests/submoduletests/test_data_base.cpp +++ b/src/tests/submoduletests/test_data_base.cpp @@ -11,8 +11,8 @@ struct MockCache : public cache_base { class data : public data_base { public: - data(mesh_elem face, boost::shared_ptr param, pt::ptree& cfg) - : data_base(face,param,cfg,true) {}; + data(mesh_elem face, std::shared_ptr param, pt::ptree& cfg) + : data_base(face,param,&cfg,true) {}; ~data() {}; // update_value is private to data and therefore must be access in a function @@ -41,13 +41,13 @@ class data : public data_base class DataBaseTest : public ::testing::Test { protected: void SetUp() override { - mock_global = boost::make_shared(); + mock_global = std::make_shared(); } // mock components for the data constructor mesh_elem mock_face; - boost::shared_ptr mock_global; + std::shared_ptr mock_global; pt::ptree mock_cfg; // Lambdas for testing, standing in for face access diff --git a/src/tests/submoduletests/test_submodule.cpp b/src/tests/submoduletests/test_submodule.cpp index a2af35fd..c53f6210 100644 --- a/src/tests/submoduletests/test_submodule.cpp +++ b/src/tests/submoduletests/test_submodule.cpp @@ -42,8 +42,8 @@ struct Cache : public cache_base class data2 : public data_base { public: - data2(mesh_elem face_in,boost::shared_ptr param,pt::ptree& cfg) - : data_base(face_in,param,cfg,true) {}; + data2(mesh_elem face_in,std::shared_ptr param,pt::ptree& cfg) + : data_base(face_in,param,&cfg,true) {}; double input1() { @@ -62,7 +62,7 @@ class data2 : public data_base []() { return 2.5; } ); - return cache_->input1; + return cache_->input2; }; void output(const double t) @@ -74,12 +74,12 @@ class data2 : public data_base class TestModule : public ::testing::Test { protected: - TestModule() : d(face,param,cfg) {}; + TestModule() {}; mesh_elem face{nullptr}; - boost::shared_ptr param; + std::shared_ptr param = std::make_shared(); pt::ptree cfg; - data2 d; + data2 d{face,param,cfg}; submodule submodule_instance; void run() {