From 9126172e82c8773968e4bcb0f94bbb611a10560f Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Tue, 10 Mar 2026 09:32:15 -0400 Subject: [PATCH 01/28] Create and use struct for BMI module var details. Creating new Bmi_Var_Details struct to hold BMI module variable details that the framework needs for performing certain operations, so that these things can be stored in memory for faster execution; also, creating a new member variable for Bmi_Module_Formulation class that is a vector of these new Bmi_Var_Details instances containing details on the given module's BMI input variables. --- .../catchment/Bmi_Module_Formulation.hpp | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/include/realizations/catchment/Bmi_Module_Formulation.hpp b/include/realizations/catchment/Bmi_Module_Formulation.hpp index 6aecf7dbc1..1a77cb6aa3 100644 --- a/include/realizations/catchment/Bmi_Module_Formulation.hpp +++ b/include/realizations/catchment/Bmi_Module_Formulation.hpp @@ -22,6 +22,36 @@ class Bmi_Cpp_Multi_Array_Test; namespace realization { + /** Type to hold some certain details about a BMI module variable that the framework will need used repeatedly. */ + struct Bmi_Var_Details { + /** The module's publicized name for this variable. */ + std::string name; + /** The framework's configured alias for the variable. */ + std::string mapped_alias; + /** Reference to applicable data provided that is source for input values. */ + std::shared_ptr provider; + /** The size of individual items for this variable. */ + int item_size; + /** The number of items for this variable. */ + int num_items; + /** String for the C++ type corresponding to this variable's type. */ + std::string cpp_type; + + Bmi_Var_Details() : Bmi_Var_Details("", "", nullptr, -1, -1, "") { } + + Bmi_Var_Details(std::string name, std::string alias, std::shared_ptr provider, int item_size, int num_items, std::string cpp_type) + : name(name), mapped_alias(alias), provider(provider), item_size(item_size), num_items(num_items), cpp_type(cpp_type) { } + + Bmi_Var_Details(const Bmi_Var_Details& source) { + name = source.name; + mapped_alias = source.mapped_alias; + provider = source.provider; + item_size = source.item_size; + num_items = source.num_items; + cpp_type = source.cpp_type; + } + }; + /** * Abstraction of a formulation with a single backing model object that implements the BMI. */ @@ -448,6 +478,14 @@ namespace realization { int next_time_step_index = 0; private: + /** + * BMI input variables details, cached to improve compute performance when setting values prior to updates. + * + * This will hold cached details on input variables needed during @ref set_model_inputs_prior_to_update at each + * time step. It will be populated lazily on the first time step. + */ + std::vector bmi_input_var_details; + models::bmi::protocols::NgenBmiProtocols bmi_protocols; /** * Whether model ``Update`` calls are allowed and handled in some way by the backing model for time steps after From 08924b9b54f5f2788248925fe1efb7a7e7b47ec8 Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Tue, 10 Mar 2026 09:55:11 -0400 Subject: [PATCH 02/28] Optimize set_model_inputs_prior_to_update. Modify Bmi_Module_Formulation.set_model_inputs_prior_to_update to store BMI input variable details (e.g., data provider) on first execution and then reuse these saved values, rather than executing module BMI function calls to re-retrieve these values on every time step. --- .../catchment/Bmi_Module_Formulation.cpp | 81 ++++++++++--------- 1 file changed, 41 insertions(+), 40 deletions(-) diff --git a/src/realizations/catchment/Bmi_Module_Formulation.cpp b/src/realizations/catchment/Bmi_Module_Formulation.cpp index 75b143992c..910460d541 100644 --- a/src/realizations/catchment/Bmi_Module_Formulation.cpp +++ b/src/realizations/catchment/Bmi_Module_Formulation.cpp @@ -655,77 +655,78 @@ namespace realization { } void Bmi_Module_Formulation::set_model_inputs_prior_to_update(const double &model_init_time, time_step_t t_delta) { - std::vector in_var_names = get_bmi_model()->GetInputVarNames(); time_t model_epoch_time = convert_model_time(model_init_time) + get_bmi_model_start_time_forcing_offset_s(); - for (std::string & var_name : in_var_names) { - data_access::GenericDataProvider *provider; - std::string var_map_alias = get_config_mapped_variable_name(var_name); - if (input_forcing_providers.find(var_map_alias) != input_forcing_providers.end()) { - provider = input_forcing_providers[var_map_alias].get(); - } - else if (var_map_alias != var_name && input_forcing_providers.find(var_name) != input_forcing_providers.end()) { - provider = input_forcing_providers[var_name].get(); - } - else { - provider = forcing.get(); - } + if (bmi_input_var_details.empty()) { + for (std::string & var_name : get_bmi_model()->GetInputVarNames()) { + Bmi_Var_Details var_details; + var_details.name = var_name; + var_details.mapped_alias = get_config_mapped_variable_name(var_name); + var_details.item_size = get_bmi_model()->GetVarItemsize(var_name); + var_details.num_items = get_bmi_model()->GetVarNbytes(var_name) / var_details.item_size; + var_details.cpp_type = get_bmi_model()->get_analogous_cxx_type(get_bmi_model()->GetVarType(var_name), + var_details.item_size); + + if (input_forcing_providers.find(var_details.mapped_alias) != input_forcing_providers.end()) { + var_details.provider = input_forcing_providers[var_details.mapped_alias]; + } + else if (var_details.mapped_alias != var_name && input_forcing_providers.find(var_name) != input_forcing_providers.end()) { + var_details.provider = input_forcing_providers[var_name]; + } + else { + var_details.provider = forcing; + } - // TODO: probably need to actually allow this by default and warn, but have config option to activate - // this type of behavior - // TODO: account for arrays later - int nbytes = get_bmi_model()->GetVarNbytes(var_name); - int varItemSize = get_bmi_model()->GetVarItemsize(var_name); - int numItems = nbytes / varItemSize; - assert(nbytes % varItemSize == 0); + bmi_input_var_details.push_back(var_details); + } + } + for (Bmi_Var_Details input_var_details : bmi_input_var_details) { std::shared_ptr value_ptr; - // Finally, use the value obtained to set the model input - std::string type = get_bmi_model()->get_analogous_cxx_type(get_bmi_model()->GetVarType(var_name), - varItemSize); - if (numItems != 1) { + if (input_var_details.num_items != 1) { //more than a single value needed for var_name - auto values = provider->get_values(CatchmentAggrDataSelector(this->get_catchment_id(),var_map_alias, model_epoch_time, t_delta, - get_bmi_model()->GetVarUnits(var_name))); + std::vector values = input_var_details.provider->get_values( + CatchmentAggrDataSelector(this->get_catchment_id(), input_var_details.mapped_alias, + model_epoch_time, t_delta, + get_bmi_model()->GetVarUnits(input_var_details.name))); //need to marshal data types to the receiver as well //this could be done a little more elegantly if the provider interface were //"type aware", but for now, this will do (but requires yet another copy) if(values.size() == 1){ //FIXME this isn't generic broadcasting, but works for scalar implementations #ifndef NGEN_QUIET - std::cerr << "WARN: broadcasting variable '" << var_name << "' from scalar to expected array\n"; + std::cerr << "WARN: broadcasting variable '" << input_var_details.name << "' from scalar to expected array\n"; #endif - values.resize(numItems, values[0]); - } else if (values.size() != numItems) { - throw std::runtime_error("Mismatch in item count for variable '" + var_name + "': model expects " + - std::to_string(numItems) + ", provider returned " + std::to_string(values.size()) + + values.resize(input_var_details.num_items, values[0]); + } else if (values.size() != input_var_details.num_items) { + throw std::runtime_error("Mismatch in item count for variable '" + input_var_details.name + "': model expects " + + std::to_string(input_var_details.num_items) + ", provider returned " + std::to_string(values.size()) + " items\n"); } - value_ptr = get_values_as_type( type, values.begin(), values.end() ); + value_ptr = get_values_as_type(input_var_details.cpp_type, values.begin(), values.end() ); } else { try { //scalar value - double value = provider->get_value(CatchmentAggrDataSelector(this->get_catchment_id(),var_map_alias, model_epoch_time, t_delta, - get_bmi_model()->GetVarUnits(var_name))); - value_ptr = get_value_as_type(type, value); + double value = input_var_details.provider->get_value(CatchmentAggrDataSelector(this->get_catchment_id(),input_var_details.mapped_alias, model_epoch_time, t_delta, get_bmi_model()->GetVarUnits(input_var_details.name))); + value_ptr = get_value_as_type(input_var_details.cpp_type, value); } catch (UnitsHelper::unit_conversion_exception &uce) { - bool new_error = UnitsHelper::record_unit_conversion_fault(uce, "Bmi_Module_Formulation::set_model_inputs_prior_to_update", var_map_alias); + bool new_error = UnitsHelper::record_unit_conversion_fault(uce, "Bmi_Module_Formulation::set_model_inputs_prior_to_update", input_var_details.mapped_alias); if (new_error) { std::stringstream ss; ss << "Unit conversion failure:" << " requester {'" << get_bmi_model()->get_model_name() << "' catchment '" << get_catchment_id() - << "' variable '" << var_name << "'" << " (alias '" << var_map_alias << "')" - << " units '" << get_bmi_model()->GetVarUnits(var_name) << "'}" + << "' variable '" << input_var_details.name << "'" << " (alias '" << input_var_details.mapped_alias << "')" + << " units '" << get_bmi_model()->GetVarUnits(input_var_details.name) << "'}" << " provider {'" << uce.provider_model_name << "' source variable '" << uce.provider_var_name << "'" << " raw value " << uce.unconverted_values[0] << "}" << " message \"" << uce.what() << "\"\n"; logging::warning(ss.str().c_str()); ss.str(""); } - value_ptr = get_value_as_type(type, uce.unconverted_values[0]); + value_ptr = get_value_as_type(input_var_details.cpp_type, uce.unconverted_values[0]); } } - get_bmi_model()->SetValue(var_name, value_ptr.get()); + get_bmi_model()->SetValue(input_var_details.name, value_ptr.get()); } } } From c77794edd2342d83d2e8257c5a99c078cc4cc52a Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Tue, 10 Mar 2026 10:04:24 -0400 Subject: [PATCH 03/28] Fix sig/docs for set_model_inputs_prior_to_update. Adjust function signature to change param name for model time to avoid confusion regarding what might be meant by "init" or "initial" in BMI contexts, and fix docstring for param, which was wrong even before the name change. --- include/realizations/catchment/Bmi_Module_Formulation.hpp | 4 ++-- src/realizations/catchment/Bmi_Module_Formulation.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/realizations/catchment/Bmi_Module_Formulation.hpp b/include/realizations/catchment/Bmi_Module_Formulation.hpp index 1a77cb6aa3..f053f5bd80 100644 --- a/include/realizations/catchment/Bmi_Module_Formulation.hpp +++ b/include/realizations/catchment/Bmi_Module_Formulation.hpp @@ -434,11 +434,11 @@ namespace realization { /** * Set BMI input variable values for the model appropriately prior to calling its `BMI `update()``. * - * @param model_initial_time The model's time prior to the update, in its internal units and representation. + * @param model_time The model's time prior to the update, in its internal units and representation. * @param t_delta The size of the time step over which the formulation is going to update the model, which might * be different than the model's internal time step. */ - void set_model_inputs_prior_to_update(const double &model_init_time, time_step_t t_delta); + void set_model_inputs_prior_to_update(const double &model_time, time_step_t t_delta); /** The delta of the last model update execution (typically, this is time step size). */ time_step_t last_model_response_delta = 0; diff --git a/src/realizations/catchment/Bmi_Module_Formulation.cpp b/src/realizations/catchment/Bmi_Module_Formulation.cpp index 910460d541..96e396432c 100644 --- a/src/realizations/catchment/Bmi_Module_Formulation.cpp +++ b/src/realizations/catchment/Bmi_Module_Formulation.cpp @@ -654,8 +654,8 @@ namespace realization { "': no logic for converting value to variable's type."); } - void Bmi_Module_Formulation::set_model_inputs_prior_to_update(const double &model_init_time, time_step_t t_delta) { - time_t model_epoch_time = convert_model_time(model_init_time) + get_bmi_model_start_time_forcing_offset_s(); + void Bmi_Module_Formulation::set_model_inputs_prior_to_update(const double &model_time, time_step_t t_delta) { + time_t model_epoch_time = convert_model_time(model_time) + get_bmi_model_start_time_forcing_offset_s(); if (bmi_input_var_details.empty()) { for (std::string & var_name : get_bmi_model()->GetInputVarNames()) { From bd66db1fe6d8128e74670b02efa973a6d5442e7e Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Tue, 10 Mar 2026 12:49:58 -0400 Subject: [PATCH 04/28] Reorder Bmi_Var_Details member variables. --- include/realizations/catchment/Bmi_Module_Formulation.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/realizations/catchment/Bmi_Module_Formulation.hpp b/include/realizations/catchment/Bmi_Module_Formulation.hpp index f053f5bd80..a0053bbcf0 100644 --- a/include/realizations/catchment/Bmi_Module_Formulation.hpp +++ b/include/realizations/catchment/Bmi_Module_Formulation.hpp @@ -28,14 +28,14 @@ namespace realization { std::string name; /** The framework's configured alias for the variable. */ std::string mapped_alias; + /** String for the C++ type corresponding to this variable's type. */ + std::string cpp_type; /** Reference to applicable data provided that is source for input values. */ std::shared_ptr provider; /** The size of individual items for this variable. */ int item_size; /** The number of items for this variable. */ int num_items; - /** String for the C++ type corresponding to this variable's type. */ - std::string cpp_type; Bmi_Var_Details() : Bmi_Var_Details("", "", nullptr, -1, -1, "") { } From a91765280e5a6c009d6256a2771f85a0dc35743c Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Fri, 13 Mar 2026 11:14:24 -0400 Subject: [PATCH 05/28] Further adjust set_model_inputs_prior_to_update. Adjusting things further in set_model_inputs_prior_to_update improvements, including - support both executing with these new optimizations or without them (i.e., still fetching and re-calculating input var metadata on each call) - add member variable (default: `false`) and setter function to store whether to execute using new set_model_inputs_prior_to_update optimizations or original "always re-fetch" logic - modularize parts of functionality into several different functions for better organization and reusability - add variable units to metadata that is stored --- .../catchment/Bmi_Module_Formulation.hpp | 97 ++++++++++- .../catchment/Bmi_Module_Formulation.cpp | 163 ++++++++++++------ 2 files changed, 198 insertions(+), 62 deletions(-) diff --git a/include/realizations/catchment/Bmi_Module_Formulation.hpp b/include/realizations/catchment/Bmi_Module_Formulation.hpp index a0053bbcf0..37e7ae1c82 100644 --- a/include/realizations/catchment/Bmi_Module_Formulation.hpp +++ b/include/realizations/catchment/Bmi_Module_Formulation.hpp @@ -30,6 +30,8 @@ namespace realization { std::string mapped_alias; /** String for the C++ type corresponding to this variable's type. */ std::string cpp_type; + /** String for variable's units. */ + std::string units; /** Reference to applicable data provided that is source for input values. */ std::shared_ptr provider; /** The size of individual items for this variable. */ @@ -37,10 +39,10 @@ namespace realization { /** The number of items for this variable. */ int num_items; - Bmi_Var_Details() : Bmi_Var_Details("", "", nullptr, -1, -1, "") { } + Bmi_Var_Details() : Bmi_Var_Details("", "", nullptr, -1, -1, "", "") { } - Bmi_Var_Details(std::string name, std::string alias, std::shared_ptr provider, int item_size, int num_items, std::string cpp_type) - : name(name), mapped_alias(alias), provider(provider), item_size(item_size), num_items(num_items), cpp_type(cpp_type) { } + Bmi_Var_Details(const std::string& name, const std::string& alias, const std::shared_ptr& provider, const int item_size, const int num_items, const std::string& cpp_type, const std::string& units) + : name(name), mapped_alias(alias), cpp_type(cpp_type), units(units), provider(provider), item_size(item_size), num_items(num_items) { } Bmi_Var_Details(const Bmi_Var_Details& source) { name = source.name; @@ -49,6 +51,7 @@ namespace realization { item_size = source.item_size; num_items = source.num_items; cpp_type = source.cpp_type; + units = source.units; } }; @@ -434,12 +437,27 @@ namespace realization { /** * Set BMI input variable values for the model appropriately prior to calling its `BMI `update()``. * + * Depending on the value of @ref store_input_variable_metadata (`false` by default, but which can be controlled + * using @ref set_store_input_var_metadata), this will defer most of its execution to a call either to + * @ref do_bmi_sets_from_stored_metadata or @ref do_bmi_sets_with_full_refetch. + * * @param model_time The model's time prior to the update, in its internal units and representation. * @param t_delta The size of the time step over which the formulation is going to update the model, which might * be different than the model's internal time step. */ void set_model_inputs_prior_to_update(const double &model_time, time_step_t t_delta); + /** + * Set member variable indicating whether @ref set_model_inputs_prior_to_update should store and reuse metadata. + * + * Set the @ref store_input_variable_metadata member variable, which indicates whether + * @ref set_model_inputs_prior_to_update should store and reuse metadata, as opposed to refreshing such data + * each time @ref set_model_inputs_prior_to_update is called. + * + * @param store_input_var_metadata Whether @ref set_model_inputs_prior_to_update should store and reuse metadata + */ + void set_store_input_var_metadata(bool store_input_var_metadata); + /** The delta of the last model update execution (typically, this is time step size). */ time_step_t last_model_response_delta = 0; /** The epoch time of the model at the beginning of its last update. */ @@ -481,10 +499,10 @@ namespace realization { /** * BMI input variables details, cached to improve compute performance when setting values prior to updates. * - * This will hold cached details on input variables needed during @ref set_model_inputs_prior_to_update at each + * This will hold cached details on input variables needed during @ref do_bmi_sets_from_stored_metadata at each * time step. It will be populated lazily on the first time step. */ - std::vector bmi_input_var_details; + std::unique_ptr> bmi_input_var_details; models::bmi::protocols::NgenBmiProtocols bmi_protocols; /** @@ -524,6 +542,75 @@ namespace realization { BMI_REALIZATION_CFG_PARAM_REQ__MODEL_TYPE, }; + /** Whether @ref set_model_inputs_prior_to_update should store and reuse metadata. */ + bool store_input_variable_metadata = false; + + /** + * Set BMI input variables before `BMI update, using saved metadata rather than re-fetching or re-calculating. + * + * This is one of two available execution paths used by @ref set_model_inputs_prior_to_update for the bulk of + * its behavior. Certain metadata details about a BMI input variable must be available in order for the + * framework to execute a `set_value` call: e.g., the data provider that is the correct source of input values, + * the analogous C++ type, the number of items, etc. In this execution option, that data is obtained once and + * stored for subsequent reuse, optimizing compute at each time step a bit at the expense of memory. + * + * These details are stored within the @ref bmi_input_var_details vector, populated on the first call to this + * function. + * + * An important consideration is that this function is not strictly safe relying only on guarantees provided by + * BMI. Nothing within the BMI spec guarantees that, for example, a variable will not change the number of items + * it contains. It is therefore possible, in general, for stored data to become stale for a properly implemented + * BMI module. In practice, however, any selected module's implementation details and behavior will be known + * by the user, so users can elect to only use this execution path for @ref set_model_inputs_prior_to_update + * when the BMI module itself guarantees such data cannot become stale. + * + * @param src_data_start The start time (in seconds) to use when retrieving data from the appropriate provider to + * use for setting the model's variables. + * @param t_delta The size of the time step over which the formulation is going to update the model, which might + * be different than the model's internal time step. + */ + void do_bmi_sets_from_stored_metadata(const time_t &src_data_start, const time_step_t &t_delta); + + /** + * Set BMI input variables before `BMI update, re-fetching and re-calculating required metadata each time. + * + * This is one of two available execution paths used by @ref set_model_inputs_prior_to_update for the bulk of + * its behavior. Certain metadata details about a BMI input variable must be available in order for the + * framework to execute a `set_value` call: e.g., the data provider that is the correct source of input values, + * the analogous C++ type, the number of items, etc. In this execution option, that data is freshly obtained + * - either recalculated, redetermined, or refetched from the BMI module itself - on every call to this method. + * This results in less efficient compute but also reduced memory usage. + * + * An important consideration is that this function provides an execution option that is strictly safe relying + * only on guarantees provided by BMI. Nothing within the BMI spec guarantees that, for example, a variable will + * not change the number of items it contains. If a configured module does (or may) change input variable + * metadata, or if it is possible for a module to change the set of input variables, then this execution path + * for @ref set_model_inputs_prior_to_update should be selected. + * + * BMI module. In practice, however, any selected module's implementation details and behavior will be known + * by the user, so users can elect to only use this execution path for @ref set_model_inputs_prior_to_update + * when the BMI module itself guarantees such data cannot become stale. + * better memory and safe for no guarantees + * + * @param src_data_start The start time (in seconds) to use when retrieving data from the appropriate provider to + * use for setting the model's variables. + * @param t_delta The size of the time step over which the formulation is going to update the model, which might + * be different than the model's internal time step. + */ + void do_bmi_sets_with_full_refetch(const time_t &src_data_start, const time_step_t &t_delta); + + /** + * Get the appropriate data provider to set inputs for this BMI variable. + * + * Get the appropriate data provider for setting values for this BMI variable from @ref input_forcing_providers. + * + * @param var_name The BMI variable name as retrievable directly via BMI. + * @param mapped_alias The framework's internal mapped alias for this variable. + * @return The appropriate data provider + */ + std::shared_ptr& get_provider_for_input_var(const std::string& var_name, const std::string& mapped_alias); + + void perform_set(const time_t &src_data_start, const time_step_t &t_delta, const int num_items, data_access::GenericDataProvider *provider, const std::string &mapped_alias, const std::string &var_name, const std::string &cpp_type, const std::string &units) const; }; /* template diff --git a/src/realizations/catchment/Bmi_Module_Formulation.cpp b/src/realizations/catchment/Bmi_Module_Formulation.cpp index 96e396432c..ee9d1fb1e8 100644 --- a/src/realizations/catchment/Bmi_Module_Formulation.cpp +++ b/src/realizations/catchment/Bmi_Module_Formulation.cpp @@ -655,9 +655,22 @@ namespace realization { } void Bmi_Module_Formulation::set_model_inputs_prior_to_update(const double &model_time, time_step_t t_delta) { - time_t model_epoch_time = convert_model_time(model_time) + get_bmi_model_start_time_forcing_offset_s(); + time_t forcing_start = convert_model_time(model_time) + get_bmi_model_start_time_forcing_offset_s(); + if (store_input_variable_metadata) { + do_bmi_sets_from_stored_metadata(forcing_start, t_delta); + } + else { + do_bmi_sets_with_full_refetch(forcing_start, t_delta); + } + } - if (bmi_input_var_details.empty()) { + void Bmi_Module_Formulation::set_store_input_var_metadata(bool store_input_var_metadata) { + store_input_variable_metadata = store_input_var_metadata; + } + + void Bmi_Module_Formulation::do_bmi_sets_from_stored_metadata(const time_t &src_data_start, const time_step_t &t_delta) { + if (bmi_input_var_details == nullptr) { + bmi_input_var_details = std::make_unique>(); for (std::string & var_name : get_bmi_model()->GetInputVarNames()) { Bmi_Var_Details var_details; var_details.name = var_name; @@ -666,67 +679,103 @@ namespace realization { var_details.num_items = get_bmi_model()->GetVarNbytes(var_name) / var_details.item_size; var_details.cpp_type = get_bmi_model()->get_analogous_cxx_type(get_bmi_model()->GetVarType(var_name), var_details.item_size); + var_details.units = get_bmi_model()->GetVarUnits(var_name); + var_details.provider = get_provider_for_input_var(var_name, var_details.mapped_alias); - if (input_forcing_providers.find(var_details.mapped_alias) != input_forcing_providers.end()) { - var_details.provider = input_forcing_providers[var_details.mapped_alias]; - } - else if (var_details.mapped_alias != var_name && input_forcing_providers.find(var_name) != input_forcing_providers.end()) { - var_details.provider = input_forcing_providers[var_name]; - } - else { - var_details.provider = forcing; - } - - bmi_input_var_details.push_back(var_details); + bmi_input_var_details->push_back(var_details); } } - for (Bmi_Var_Details input_var_details : bmi_input_var_details) { - std::shared_ptr value_ptr; - if (input_var_details.num_items != 1) { - //more than a single value needed for var_name - std::vector values = input_var_details.provider->get_values( - CatchmentAggrDataSelector(this->get_catchment_id(), input_var_details.mapped_alias, - model_epoch_time, t_delta, - get_bmi_model()->GetVarUnits(input_var_details.name))); - //need to marshal data types to the receiver as well - //this could be done a little more elegantly if the provider interface were - //"type aware", but for now, this will do (but requires yet another copy) - if(values.size() == 1){ - //FIXME this isn't generic broadcasting, but works for scalar implementations - #ifndef NGEN_QUIET - std::cerr << "WARN: broadcasting variable '" << input_var_details.name << "' from scalar to expected array\n"; - #endif - values.resize(input_var_details.num_items, values[0]); - } else if (values.size() != input_var_details.num_items) { - throw std::runtime_error("Mismatch in item count for variable '" + input_var_details.name + "': model expects " + - std::to_string(input_var_details.num_items) + ", provider returned " + std::to_string(values.size()) + - " items\n"); - } - value_ptr = get_values_as_type(input_var_details.cpp_type, values.begin(), values.end() ); - - } else { - try { - //scalar value - double value = input_var_details.provider->get_value(CatchmentAggrDataSelector(this->get_catchment_id(),input_var_details.mapped_alias, model_epoch_time, t_delta, get_bmi_model()->GetVarUnits(input_var_details.name))); - value_ptr = get_value_as_type(input_var_details.cpp_type, value); - } catch (UnitsHelper::unit_conversion_exception &uce) { - bool new_error = UnitsHelper::record_unit_conversion_fault(uce, "Bmi_Module_Formulation::set_model_inputs_prior_to_update", input_var_details.mapped_alias); - if (new_error) { - std::stringstream ss; - ss << "Unit conversion failure:" - << " requester {'" << get_bmi_model()->get_model_name() << "' catchment '" << get_catchment_id() - << "' variable '" << input_var_details.name << "'" << " (alias '" << input_var_details.mapped_alias << "')" - << " units '" << get_bmi_model()->GetVarUnits(input_var_details.name) << "'}" - << " provider {'" << uce.provider_model_name << "' source variable '" << uce.provider_var_name << "'" - << " raw value " << uce.unconverted_values[0] << "}" - << " message \"" << uce.what() << "\"\n"; - logging::warning(ss.str().c_str()); ss.str(""); - } - value_ptr = get_value_as_type(input_var_details.cpp_type, uce.unconverted_values[0]); + for (Bmi_Var_Details input_var_details : *bmi_input_var_details) { + perform_set( + src_data_start, + t_delta, + input_var_details.num_items, + input_var_details.provider.get(), + input_var_details.mapped_alias, + input_var_details.name, + input_var_details.cpp_type, + input_var_details.units + ); + } + } + + void Bmi_Module_Formulation::do_bmi_sets_with_full_refetch(const time_t& src_data_start, const time_step_t& t_delta) { + for (std::string & var_name : get_bmi_model()->GetInputVarNames()) { + int item_size = get_bmi_model()->GetVarItemsize(var_name); + std::string mapped_alias = get_config_mapped_variable_name(var_name); + + perform_set( + src_data_start, + t_delta, + get_bmi_model()->GetVarNbytes(var_name) / item_size, + get_provider_for_input_var(var_name, mapped_alias).get(), + mapped_alias, + var_name, + get_bmi_model()->get_analogous_cxx_type(get_bmi_model()->GetVarType(var_name), item_size), + get_bmi_model()->GetVarUnits(var_name) + ); + } + } + + std::shared_ptr& Bmi_Module_Formulation::get_provider_for_input_var(const std::string& var_name, const std::string& mapped_alias) { + if (input_forcing_providers.find(mapped_alias) != input_forcing_providers.end()) + return input_forcing_providers[mapped_alias]; + + if (mapped_alias != var_name && input_forcing_providers.find(var_name) != input_forcing_providers.end()) + return input_forcing_providers[var_name]; + + return forcing; + } + + void Bmi_Module_Formulation::perform_set(const time_t &src_data_start, const time_step_t& t_delta, + const int num_items, data_access::GenericDataProvider* provider, const std::string& mapped_alias, + const std::string& var_name, const std::string& cpp_type, const std::string& units) const { + std::shared_ptr value_ptr; + if (num_items != 1) { + //more than a single value needed for var_name + std::vector values = provider->get_values( + CatchmentAggrDataSelector(get_catchment_id(), mapped_alias, src_data_start, t_delta,units) + ); + //need to marshal data types to the receiver as well + //this could be done a little more elegantly if the provider interface were + //"type aware", but for now, this will do (but requires yet another copy) + if(values.size() == 1){ + //FIXME this isn't generic broadcasting, but works for scalar implementations + #ifndef NGEN_QUIET + std::cerr << "WARN: broadcasting variable '" << var_name << "' from scalar to expected array\n"; + #endif + values.resize(num_items, values[0]); + } + else if (values.size() != num_items) { + throw std::runtime_error("Mismatch in item count for variable '" + var_name + "': model expects " + + std::to_string(num_items) + ", provider returned " + + std::to_string(values.size()) + " items\n"); + } + value_ptr = get_values_as_type(cpp_type, values.begin(), values.end() ); + + } + else { + try { + //scalar value + double value = provider->get_value(CatchmentAggrDataSelector(this->get_catchment_id(),mapped_alias, src_data_start, t_delta, units)); + value_ptr = get_value_as_type(cpp_type, value); + } catch (UnitsHelper::unit_conversion_exception &uce) { + bool new_error = UnitsHelper::record_unit_conversion_fault(uce, "Bmi_Module_Formulation::perform_set", mapped_alias); + if (new_error) { + std::stringstream ss; + ss << "Unit conversion failure:" + << " requester {'" << get_bmi_model()->get_model_name() << "' catchment '" << get_catchment_id() + << "' variable '" << var_name << "'" << " (alias '" << mapped_alias << "')" + << " units '" << units << "'}" + << " provider {'" << uce.provider_model_name << "' source variable '" << uce.provider_var_name << "'" + << " raw value " << uce.unconverted_values[0] << "}" + << " message \"" << uce.what() << "\"\n"; + logging::warning(ss.str().c_str()); ss.str(""); } + value_ptr = get_value_as_type(cpp_type, uce.unconverted_values[0]); } - get_bmi_model()->SetValue(input_var_details.name, value_ptr.get()); } + get_bmi_model()->SetValue(var_name, value_ptr.get()); } } From 481add41a6d8f560532aaef9bbc4cf10f89bf408 Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Fri, 13 Mar 2026 11:54:59 -0400 Subject: [PATCH 06/28] Config for set_model_inputs_prior_to_update opts. Adding config option for available set_model_inputs_prior_to_update optimizations that store metadata, along with documentation on use. --- doc/BMI_MODELS.md | 13 +++++++++++++ include/realizations/catchment/Bmi_Formulation.hpp | 1 + .../catchment/Bmi_Module_Formulation.cpp | 5 +++++ 3 files changed, 19 insertions(+) diff --git a/doc/BMI_MODELS.md b/doc/BMI_MODELS.md index 7a447d0316..ca94e33067 100644 --- a/doc/BMI_MODELS.md +++ b/doc/BMI_MODELS.md @@ -183,6 +183,19 @@ There are some special BMI formulation config parameters which are required in c * `fixed_time_step` * boolean value to indicate whether this model has a fixed time step size * implied to be `true` by default +* `store_input_variable_metadata` + * boolean value, `false` by default + * indicates whether BMI input variable metadata should be stored on first look-up and reused, thus optimizing execution + * BMI input variables have values set at each time step + * certain metadata is required when performing those set operations: + * name and mapped alias (if set) + * units + * memory size and type of individual items (e.g., `int`, `float`, `double`, etc.) + * whether the variable is an array, and if so, the number of individual items + * for BMI modules that will not change applicable metadata throughout a simulation, this can be stored and reused + * the BMI specification does not expressly guarantee metadata values will not change, so configuration must consider the particular BMI module in use + * also, these optimizations do result in additional memory usage + * while relatively small, can scale significantly for larger simulations, so this may not be usable for situations when memory is constrained ## BMI Models Written in C diff --git a/include/realizations/catchment/Bmi_Formulation.hpp b/include/realizations/catchment/Bmi_Formulation.hpp index 3d9336f54b..23e14e71fd 100644 --- a/include/realizations/catchment/Bmi_Formulation.hpp +++ b/include/realizations/catchment/Bmi_Formulation.hpp @@ -33,6 +33,7 @@ #define BMI_REALIZATION_CFG_PARAM_OPT__CPP_DESTROY_FUNC "destroy_function" #define BMI_REALIZATION_CFG_PARAM_OPT__CPP_CREATE_FUNC_DEFAULT "bmi_model_create" #define BMI_REALIZATION_CFG_PARAM_OPT__CPP_DESTROY_FUNC_DEFAULT "bmi_model_destroy" +#define BMI_REALIZATION_CFG_PARAM_OPT__STORE_INPUT_VAR_METADATA "store_input_variable_metadata" /* *************** See also the Forcing.h file for several CSDMS Standard Names definitions *************** */ diff --git a/src/realizations/catchment/Bmi_Module_Formulation.cpp b/src/realizations/catchment/Bmi_Module_Formulation.cpp index ee9d1fb1e8..533257f49f 100644 --- a/src/realizations/catchment/Bmi_Module_Formulation.cpp +++ b/src/realizations/catchment/Bmi_Module_Formulation.cpp @@ -345,6 +345,11 @@ namespace realization { properties.at(BMI_REALIZATION_CFG_PARAM_OPT__FIXED_TIME_STEP).as_boolean()); } + if (properties.find(BMI_REALIZATION_CFG_PARAM_OPT__STORE_INPUT_VAR_METADATA) != properties.end()) { + set_store_input_var_metadata( + properties.at(BMI_REALIZATION_CFG_PARAM_OPT__STORE_INPUT_VAR_METADATA).as_boolean()); + } + auto std_names_it = properties.find(BMI_REALIZATION_CFG_PARAM_OPT__VAR_STD_NAMES); if (std_names_it != properties.end()) { geojson::PropertyMap names_map = std_names_it->second.get_values(); From c550edf90ab714c4b1a1cc216b5c036d127393a5 Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Thu, 19 Mar 2026 11:27:06 -0400 Subject: [PATCH 07/28] Adjust how BMI input var metadata is stored. Making several adjustments to the optimizations for storing BMI input variable metadata in a formulation for reuse: - make Bmi_Var_Details (effectively immutable) class rather than struct - pull provider pointer out of Bmi_Var_Details and setup a second vector in the formulation object for providers that corresponds to the one for Bmi_Var_Details (providers can be formulation-specific, so relationship belongs there) - created static std::set of Bmi_Var_Details objects that gets added to during logic to populate instance's vector of input Bmi_Var_Details, with the latter vector only having values inserted that exist in this static set - move logic for initializing the metadata-containing collections to a dedicated function --- .../catchment/Bmi_Module_Formulation.hpp | 109 +++++++++++++----- .../catchment/Bmi_Module_Formulation.cpp | 57 +++++---- 2 files changed, 117 insertions(+), 49 deletions(-) diff --git a/include/realizations/catchment/Bmi_Module_Formulation.hpp b/include/realizations/catchment/Bmi_Module_Formulation.hpp index 37e7ae1c82..3e5e0dfaa8 100644 --- a/include/realizations/catchment/Bmi_Module_Formulation.hpp +++ b/include/realizations/catchment/Bmi_Module_Formulation.hpp @@ -23,36 +23,60 @@ class Bmi_Cpp_Multi_Array_Test; namespace realization { /** Type to hold some certain details about a BMI module variable that the framework will need used repeatedly. */ - struct Bmi_Var_Details { + class Bmi_Var_Details { + + public: + + //Bmi_Var_Details() : Bmi_Var_Details("", "", nullptr, -1, -1, "", "") { } + + Bmi_Var_Details(const std::string& name, const std::string& alias, const int item_size, const int num_items, const std::string& cpp_type, const std::string& units) + : name(name), mapped_alias(alias), cpp_type(cpp_type), units(units), item_size(item_size), num_items(num_items) { } + + Bmi_Var_Details(const Bmi_Var_Details& source) : Bmi_Var_Details(source.name, source.mapped_alias, source.item_size, source.num_items, source.cpp_type, source.units) { } + + friend bool operator<(const Bmi_Var_Details& lhs, const Bmi_Var_Details& rhs) { + return std::tie(lhs.name, lhs.mapped_alias, lhs.cpp_type, lhs.units, lhs.item_size, + lhs.num_items) < std::tie(rhs.name, rhs.mapped_alias, rhs.cpp_type, rhs.units, + rhs.item_size, rhs.num_items); + } + + const std::string& get_name() const { + return name; + } + + const std::string& get_mapped_alias() const { + return mapped_alias; + } + + const std::string& get_cpp_type() const { + return cpp_type; + } + + const std::string& get_units() const { + return units; + } + + int get_item_size() const { + return item_size; + } + + int get_num_items() const { + return num_items; + } + + private: /** The module's publicized name for this variable. */ - std::string name; + const std::string name; /** The framework's configured alias for the variable. */ - std::string mapped_alias; + const std::string mapped_alias; /** String for the C++ type corresponding to this variable's type. */ - std::string cpp_type; + const std::string cpp_type; /** String for variable's units. */ - std::string units; - /** Reference to applicable data provided that is source for input values. */ - std::shared_ptr provider; + const std::string units;; /** The size of individual items for this variable. */ int item_size; /** The number of items for this variable. */ int num_items; - - Bmi_Var_Details() : Bmi_Var_Details("", "", nullptr, -1, -1, "", "") { } - - Bmi_Var_Details(const std::string& name, const std::string& alias, const std::shared_ptr& provider, const int item_size, const int num_items, const std::string& cpp_type, const std::string& units) - : name(name), mapped_alias(alias), cpp_type(cpp_type), units(units), provider(provider), item_size(item_size), num_items(num_items) { } - - Bmi_Var_Details(const Bmi_Var_Details& source) { - name = source.name; - mapped_alias = source.mapped_alias; - provider = source.provider; - item_size = source.item_size; - num_items = source.num_items; - cpp_type = source.cpp_type; - units = source.units; - } }; /** @@ -497,12 +521,31 @@ namespace realization { private: /** - * BMI input variables details, cached to improve compute performance when setting values prior to updates. + * BMI input variables details for all instances, cached to improve compute performance when setting values + * prior to updates. + */ + static std::set known_bmi_input_vars; + + /** + * BMI input variables details for this instance, cached to improve compute performance when setting values + * prior to updates. * - * This will hold cached details on input variables needed during @ref do_bmi_sets_from_stored_metadata at each - * time step. It will be populated lazily on the first time step. + * This will hold cached details on input variables needed by this instance during + * @ref do_bmi_sets_from_stored_metadata at each time step. It will be populated lazily on the first time step, + * via a nested call to @ref initialize_bmi_input_var_metadata. + * + * These should be pointers to @ref Bmi_Var_Details instances in @ref known_bmi_input_vars. + */ + std::unique_ptr> bmi_input_var_details; + + /** + * Vector of data providers for BMI input vars, with the provider at an index corresponding to the var in + * @ref bmi_input_var_details at the same index. + * + * As with @ref bmi_input_var_details, these should be populated during the first call to + * @ref do_bmi_sets_from_stored_metadata via a nested call to @ref initialize_bmi_input_var_metadata. */ - std::unique_ptr> bmi_input_var_details; + std::unique_ptr>> bmi_input_providers; models::bmi::protocols::NgenBmiProtocols bmi_protocols; /** @@ -554,8 +597,9 @@ namespace realization { * the analogous C++ type, the number of items, etc. In this execution option, that data is obtained once and * stored for subsequent reuse, optimizing compute at each time step a bit at the expense of memory. * - * These details are stored within the @ref bmi_input_var_details vector, populated on the first call to this - * function. + * References to objects containing these details are stored within the @ref bmi_input_var_details vector, + * populated on the first call to this function. These are actually pointers to objects contained within the + * @ref known_bmi_input_vars static member variable. * * An important consideration is that this function is not strictly safe relying only on guarantees provided by * BMI. Nothing within the BMI spec guarantees that, for example, a variable will not change the number of items @@ -610,6 +654,15 @@ namespace realization { */ std::shared_ptr& get_provider_for_input_var(const std::string& var_name, const std::string& mapped_alias); + /** + * Initialize the metadata as needed for @ref do_bmi_sets_from_stored_metadata. + * + * This will populate the @ref bmi_input_var_details member. As metadata is gather into @ref Bmi_Var_Details + * objects, these will be inserted into to the @ref known_bmi_input_vars static member. Pointers to the values + * in that set are then added to @ref bmi_input_var_details. + */ + void initialize_bmi_input_var_metadata(); + void perform_set(const time_t &src_data_start, const time_step_t &t_delta, const int num_items, data_access::GenericDataProvider *provider, const std::string &mapped_alias, const std::string &var_name, const std::string &cpp_type, const std::string &units) const; }; /* diff --git a/src/realizations/catchment/Bmi_Module_Formulation.cpp b/src/realizations/catchment/Bmi_Module_Formulation.cpp index 533257f49f..aadbef312b 100644 --- a/src/realizations/catchment/Bmi_Module_Formulation.cpp +++ b/src/realizations/catchment/Bmi_Module_Formulation.cpp @@ -3,6 +3,9 @@ #include namespace realization { + + std::set Bmi_Module_Formulation::known_bmi_input_vars; + void Bmi_Module_Formulation::create_formulation(boost::property_tree::ptree &config, geojson::PropertyMap *global) { geojson::PropertyMap options = this->interpret_parameters(config, global); inner_create_formulation(options, false); @@ -675,32 +678,19 @@ namespace realization { void Bmi_Module_Formulation::do_bmi_sets_from_stored_metadata(const time_t &src_data_start, const time_step_t &t_delta) { if (bmi_input_var_details == nullptr) { - bmi_input_var_details = std::make_unique>(); - for (std::string & var_name : get_bmi_model()->GetInputVarNames()) { - Bmi_Var_Details var_details; - var_details.name = var_name; - var_details.mapped_alias = get_config_mapped_variable_name(var_name); - var_details.item_size = get_bmi_model()->GetVarItemsize(var_name); - var_details.num_items = get_bmi_model()->GetVarNbytes(var_name) / var_details.item_size; - var_details.cpp_type = get_bmi_model()->get_analogous_cxx_type(get_bmi_model()->GetVarType(var_name), - var_details.item_size); - var_details.units = get_bmi_model()->GetVarUnits(var_name); - var_details.provider = get_provider_for_input_var(var_name, var_details.mapped_alias); - - bmi_input_var_details->push_back(var_details); - } + initialize_bmi_input_var_metadata(); } - for (Bmi_Var_Details input_var_details : *bmi_input_var_details) { + for (size_t i = 0; i < bmi_input_var_details->size(); i++) { perform_set( src_data_start, t_delta, - input_var_details.num_items, - input_var_details.provider.get(), - input_var_details.mapped_alias, - input_var_details.name, - input_var_details.cpp_type, - input_var_details.units + bmi_input_var_details->at(i)->get_num_items(), + bmi_input_providers->at(i).get(), + bmi_input_var_details->at(i)->get_mapped_alias(), + bmi_input_var_details->at(i)->get_name(), + bmi_input_var_details->at(i)->get_cpp_type(), + bmi_input_var_details->at(i)->get_units() ); } } @@ -733,6 +723,31 @@ namespace realization { return forcing; } + void Bmi_Module_Formulation::initialize_bmi_input_var_metadata() { + if (bmi_input_var_details != nullptr) { + throw std::runtime_error("Cannot re-initialize module formulation bmi_input_var_details member"); + } + bmi_input_var_details = std::make_unique>(); + bmi_input_providers = std::make_unique>>(); + for (std::string & var_name : get_bmi_model()->GetInputVarNames()) { + int item_size = get_bmi_model()->GetVarItemsize(var_name); + std::string mapped_alias = get_config_mapped_variable_name(var_name); + + // First in pair will be iterator either to inserted item or to existing that prevents insert duplicate + std::pair::iterator, bool> iter_and_result = + known_bmi_input_vars.insert( + Bmi_Var_Details(var_name, + mapped_alias, + item_size, + get_bmi_model()->GetVarNbytes(var_name) / item_size, + get_bmi_model()->get_analogous_cxx_type(get_bmi_model()->GetVarType(var_name), item_size), + get_bmi_model()->GetVarUnits(var_name))); + + bmi_input_var_details->push_back(&(*(iter_and_result.first))); + bmi_input_providers->push_back(get_provider_for_input_var(var_name, mapped_alias)); + } + } + void Bmi_Module_Formulation::perform_set(const time_t &src_data_start, const time_step_t& t_delta, const int num_items, data_access::GenericDataProvider* provider, const std::string& mapped_alias, const std::string& var_name, const std::string& cpp_type, const std::string& units) const { From fd15f6507e74790b0df892e43d19461214e3e5e5 Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Thu, 19 Mar 2026 11:27:46 -0400 Subject: [PATCH 08/28] Add more tests for BMI input optimizations. --- .../realizations/Formulation_Manager_Test.cpp | 254 ++++++++++++++++- .../catchments/Bmi_C_Formulation_Test.cpp | 257 +++++++++++++++++- 2 files changed, 509 insertions(+), 2 deletions(-) diff --git a/test/realizations/Formulation_Manager_Test.cpp b/test/realizations/Formulation_Manager_Test.cpp index 6cce1dcc90..8c01cd11ba 100644 --- a/test/realizations/Formulation_Manager_Test.cpp +++ b/test/realizations/Formulation_Manager_Test.cpp @@ -882,6 +882,180 @@ const std::string EXAMPLE_8 = "{ " "}"; +const std::string EXAMPLE_9 = "{ " + "\"global\": { " + "\"formulations\": [ " + "{" + "\"name\":\"bmi_c++\"," + "\"params\": {" + "\"model_type_name\": \"test_bmi_cpp\"," + "\"library_file\": \"{{EXTERN_LIB_DIR_PATH}}" BMI_TEST_CPP_LIB_NAME "\"," + "\"init_config\": \"{{BMI_C_INIT_DIR_PATH}}/test_bmi_c_config_0.txt\"," + "\"main_output_variable\": \"OUTPUT_VAR_2\"," + "\"store_input_variable_metadata\": true," + "\"" BMI_REALIZATION_CFG_PARAM_OPT__VAR_STD_NAMES "\": { " + "\"INPUT_VAR_2\": \"" AORC_FIELD_NAME_TEMP_2M_AG "\"," + "\"INPUT_VAR_1\": \"" AORC_FIELD_NAME_PRECIP_RATE "\"" + "}," + "\"create_function\": \"bmi_model_create\"," + "\"destroy_function\": \"bmi_model_destroy\"," + "\"uses_forcing_file\": false" + "} " + "} " + "], " + "\"forcing\": { " + "\"file_pattern\": \".*{{id}}.*.csv\", " + "\"path\": \"./data/forcing/\", " + "\"provider\": \"CsvPerFeature\" " + "} " + "}, " + "\"time\": { " + "\"start_time\": \"2015-12-01 00:00:00\", " + "\"end_time\": \"2015-12-30 23:00:00\", " + "\"output_interval\": 3600 " + "}, " + "\"disable_catchment_output\": true," + "\"catchments\": { " + "\"cat-52\": { " + "\"formulations\": [ " + "{" + "\"name\":\"bmi_c++\"," + "\"params\": {" + "\"model_type_name\": \"test_bmi_cpp\"," + "\"library_file\": \"{{EXTERN_LIB_DIR_PATH}}" BMI_TEST_CPP_LIB_NAME "\"," + "\"init_config\": \"{{BMI_C_INIT_DIR_PATH}}/test_bmi_c_config_0.txt\"," + "\"main_output_variable\": \"OUTPUT_VAR_2\"," + "\"" BMI_REALIZATION_CFG_PARAM_OPT__VAR_STD_NAMES "\": { " + "\"INPUT_VAR_2\": \"" AORC_FIELD_NAME_TEMP_2M_AG "\"," + "\"INPUT_VAR_1\": \"" AORC_FIELD_NAME_PRECIP_RATE "\"" + "}," + "\"create_function\": \"bmi_model_create\"," + "\"destroy_function\": \"bmi_model_destroy\"," + "\"uses_forcing_file\": false" + "} " + "} " + "], " + "\"forcing\": { " + "\"file_pattern\": \".*{{id}}.*.csv\", " + "\"path\": \"./data/forcing/\", " + "\"provider\": \"CsvPerFeature\" " + "} " + "}, " + "\"cat-67\": { " + "\"formulations\": [ " + "{" + "\"name\":\"bmi_c++\"," + "\"params\": {" + "\"model_type_name\": \"test_bmi_cpp\"," + "\"library_file\": \"{{EXTERN_LIB_DIR_PATH}}" BMI_TEST_CPP_LIB_NAME "\"," + "\"init_config\": \"{{BMI_C_INIT_DIR_PATH}}/test_bmi_c_config_0.txt\"," + "\"main_output_variable\": \"OUTPUT_VAR_2\"," + "\"" BMI_REALIZATION_CFG_PARAM_OPT__VAR_STD_NAMES "\": { " + "\"INPUT_VAR_2\": \"" AORC_FIELD_NAME_TEMP_2M_AG "\"," + "\"INPUT_VAR_1\": \"" AORC_FIELD_NAME_PRECIP_RATE "\"" + "}," + "\"create_function\": \"bmi_model_create\"," + "\"destroy_function\": \"bmi_model_destroy\"," + "\"uses_forcing_file\": false" + "} " + "} " + "], " + "\"forcing\": { " + "\"file_pattern\": \".*{{id}}.*.csv\", " + "\"path\": \"./data/forcing/\", " + "\"provider\": \"CsvPerFeature\" " + "} " + "} " + "} " +"}"; + +const std::string EXAMPLE_10 = "{ " + "\"global\": { " + "\"formulations\": [ " + "{" + "\"name\":\"bmi_c++\"," + "\"params\": {" + "\"model_type_name\": \"test_bmi_cpp\"," + "\"library_file\": \"{{EXTERN_LIB_DIR_PATH}}" BMI_TEST_CPP_LIB_NAME "\"," + "\"init_config\": \"{{BMI_C_INIT_DIR_PATH}}/test_bmi_c_config_0.txt\"," + "\"main_output_variable\": \"OUTPUT_VAR_2\"," + "\"store_input_variable_metadata\": true," + "\"" BMI_REALIZATION_CFG_PARAM_OPT__VAR_STD_NAMES "\": { " + "\"INPUT_VAR_2\": \"" AORC_FIELD_NAME_TEMP_2M_AG "\"," + "\"INPUT_VAR_1\": \"" AORC_FIELD_NAME_PRECIP_RATE "\"" + "}," + "\"create_function\": \"bmi_model_create\"," + "\"destroy_function\": \"bmi_model_destroy\"," + "\"uses_forcing_file\": false" + "} " + "} " + "], " + "\"forcing\": { " + "\"file_pattern\": \".*{{ID}}.*.csv\", " + "\"path\": \"./data/forcing/\", " + "\"provider\": \"CsvPerFeature\" " + "} " + "}, " + "\"time\": { " + "\"start_time\": \"2015-12-01 00:00:00\", " + "\"end_time\": \"2015-12-30 23:00:00\", " + "\"output_interval\": 3600 " + "}, " + "\"catchments\": { " + "\"cat-52\": { " + "\"formulations\": [ " + "{" + "\"name\":\"bmi_c++\"," + "\"params\": {" + "\"model_type_name\": \"test_bmi_cpp\"," + "\"library_file\": \"{{EXTERN_LIB_DIR_PATH}}" BMI_TEST_CPP_LIB_NAME "\"," + "\"init_config\": \"{{BMI_C_INIT_DIR_PATH}}/test_bmi_c_config_0.txt\"," + "\"main_output_variable\": \"OUTPUT_VAR_2\"," + "\"store_input_variable_metadata\": true," + "\"" BMI_REALIZATION_CFG_PARAM_OPT__VAR_STD_NAMES "\": { " + "\"INPUT_VAR_2\": \"" AORC_FIELD_NAME_TEMP_2M_AG "\"," + "\"INPUT_VAR_1\": \"" AORC_FIELD_NAME_PRECIP_RATE "\"" + "}," + "\"create_function\": \"bmi_model_create\"," + "\"destroy_function\": \"bmi_model_destroy\"," + "\"uses_forcing_file\": false" + "} " + "} " + "], " + "\"forcing\": { " + "\"file_pattern\": \".*{{id}}.*.csv\", " + "\"path\": \"./data/forcing/\", " + "\"provider\": \"CsvPerFeature\" " + "} " + "}, " + "\"cat-67\": { " + "\"formulations\": [ " + "{" + "\"name\":\"bmi_c++\"," + "\"params\": {" + "\"model_type_name\": \"test_bmi_cpp\"," + "\"library_file\": \"{{EXTERN_LIB_DIR_PATH}}" BMI_TEST_CPP_LIB_NAME "\"," + "\"init_config\": \"{{BMI_C_INIT_DIR_PATH}}/test_bmi_c_config_0.txt\"," + "\"main_output_variable\": \"OUTPUT_VAR_2\"," + "\"" BMI_REALIZATION_CFG_PARAM_OPT__VAR_STD_NAMES "\": { " + "\"INPUT_VAR_2\": \"" AORC_FIELD_NAME_TEMP_2M_AG "\"," + "\"INPUT_VAR_1\": \"" AORC_FIELD_NAME_PRECIP_RATE "\"" + "}," + "\"create_function\": \"bmi_model_create\"," + "\"destroy_function\": \"bmi_model_destroy\"," + "\"uses_forcing_file\": false" + "} " + "} " + "], " + "\"forcing\": { " + "\"file_pattern\": \".*{{id}}.*.csv\", " + "\"path\": \"./data/forcing/\", " + "\"provider\": \"CsvPerFeature\" " + "} " + "} " + "} " +"}"; + TEST_F(Formulation_Manager_Test, basic_reading_1) { std::stringstream stream; @@ -1039,6 +1213,84 @@ TEST_F(Formulation_Manager_Test, basic_run_3) { } } +/** + * Testing config the same as EX 1 (like in basic_run_1) but with `store_input_variable_metadata` true for global + * formulation config (which is all formulations). + */ +TEST_F(Formulation_Manager_Test, basic_run_9) { + std::stringstream stream; + stream << fix_paths(EXAMPLE_9); + + std::ostream* raw_pointer = &std::cout; + std::shared_ptr s_ptr(raw_pointer, [](void*) {}); + utils::StreamHandler catchment_output(s_ptr); + + realization::Formulation_Manager manager = realization::Formulation_Manager(stream); + + this->add_feature("cat-52"); + this->add_feature("cat-67"); + manager.read(this->fabric, catchment_output); + + ASSERT_EQ(manager.get_size(), 2); + + std::map> calculated_results; + + double dt = 3600.0; + + for (std::pair> formulation : manager) { + if (calculated_results.count(formulation.first) == 0) { + calculated_results.emplace(formulation.first, std::map()); + } + + double calculation; + + for (long t = 0; t < 4; t++) { + calculation = formulation.second->get_response(t, dt); + + calculated_results.at(formulation.first).emplace(t, calculation); + } + } +} + +/** + * Testing config the same as EX 3 (like in basic_run_3) but with `store_input_variable_metadata` true for global and + * one catchment formulation config (but not the other). + */ +TEST_F(Formulation_Manager_Test, basic_run_10) { + std::stringstream stream; + stream << fix_paths(EXAMPLE_10); + + std::ostream* raw_pointer = &std::cout; + std::shared_ptr s_ptr(raw_pointer, [](void*) {}); + utils::StreamHandler catchment_output(s_ptr); + + realization::Formulation_Manager manager = realization::Formulation_Manager(stream); + + this->add_feature("cat-67"); + manager.read(this->fabric, catchment_output); + + ASSERT_EQ(manager.get_size(), 1); + ASSERT_TRUE(manager.contains("cat-67")); + + std::vector expected_results = {571.4, 570.6, 569.0}; + + std::vector actual_results(expected_results.size()); + + for (int i = 0; i < expected_results.size(); i++) { + actual_results[i] = manager.get_formulation("cat-67")->get_response(i, 3600); + } + + for (int i = 0; i < actual_results.size(); i++) { + double actual = actual_results[i]; + // This is an error margin of the largest of 0.1% of actual value, or 1 mm + // TODO: this may not be precise enough long-term + double error_margin = std::max(actual * 0.001, 0.001); + double expected = expected_results[i]; + double diff = actual > expected ? actual - expected : expected - actual; + ASSERT_LE(diff, error_margin); + } +} + TEST_F(Formulation_Manager_Test, read_extra) { std::stringstream stream; stream << fix_paths(EXAMPLE_3); @@ -1060,7 +1312,7 @@ TEST_F(Formulation_Manager_Test, read_extra) { realization::Formulation_Manager manager = realization::Formulation_Manager(realization_config); ASSERT_TRUE(manager.is_empty()); - + this->add_feature("cat-67"); manager.read(simulation_time_config, this->fabric, catchment_output); diff --git a/test/realizations/catchments/Bmi_C_Formulation_Test.cpp b/test/realizations/catchments/Bmi_C_Formulation_Test.cpp index e532f201e9..6c7ebd1cf2 100644 --- a/test/realizations/catchments/Bmi_C_Formulation_Test.cpp +++ b/test/realizations/catchments/Bmi_C_Formulation_Test.cpp @@ -65,6 +65,18 @@ class Bmi_C_Formulation_Test : public ::testing::Test { return formulation.get_model_type_name(); } + static bool get_friend_store_input_variable_metadata(Bmi_C_Formulation& formulation) { + return formulation.store_input_variable_metadata; + } + + static std::set& get_friend_known_bmi_input_vars(Bmi_C_Formulation& formulation) { + return formulation.known_bmi_input_vars; + } + + static std::vector* get_friend_bmi_input_var_details(Bmi_C_Formulation& formulation) { + return formulation.bmi_input_var_details.get(); + } + static double get_friend_var_value_as_double(Bmi_C_Formulation& formulation, const std::string& var_name) { return formulation.get_var_value_as_double(0, var_name); } @@ -110,6 +122,7 @@ class Bmi_C_Formulation_Test : public ::testing::Test { std::vector config_json; std::vector catchment_ids; std::vector model_type_name; + std::vector stores_input_variable_metadata; std::vector forcing_file; std::vector lib_file; std::vector init_config; @@ -126,7 +139,7 @@ class Bmi_C_Formulation_Test : public ::testing::Test { void Bmi_C_Formulation_Test::SetUp() { testing::Test::SetUp(); -#define EX_COUNT 2 +#define EX_COUNT 3 forcing_dir_opts = {"./data/forcing/", "../data/forcing/", "../../data/forcing/"}; bmi_init_cfg_dir_opts = { @@ -147,6 +160,7 @@ void Bmi_C_Formulation_Test::SetUp() { lib_file = std::vector(EX_COUNT); init_config = std::vector(EX_COUNT); main_output_variable = std::vector(EX_COUNT); + stores_input_variable_metadata = std::vector(EX_COUNT); registration_functions = std::vector(EX_COUNT); uses_forcing_file = std::vector(EX_COUNT); // tries_mass_balance = std::vector(EX_COUNT); @@ -161,6 +175,7 @@ void Bmi_C_Formulation_Test::SetUp() { lib_file[0] = find_file(lib_dir_opts, BMI_TEST_C_LOCAL_LIB_NAME); init_config[0] = find_file(bmi_init_cfg_dir_opts, "test_bmi_c_config_0.txt"); main_output_variable[0] = "OUTPUT_VAR_1"; + stores_input_variable_metadata[0] = false; registration_functions[0] = "register_bmi"; uses_forcing_file[0] = false; // tries_mass_balance[0] = true; @@ -171,10 +186,22 @@ void Bmi_C_Formulation_Test::SetUp() { lib_file[1] = find_file(lib_dir_opts, BMI_TEST_C_LOCAL_LIB_NAME); init_config[1] = find_file(bmi_init_cfg_dir_opts, "test_bmi_c_config_1.txt"); main_output_variable[1] = "OUTPUT_VAR_1"; + stores_input_variable_metadata[1] = false; registration_functions[1] = "register_bmi"; uses_forcing_file[1] = false; // tries_mass_balance[1] = false; + catchment_ids[2] = "cat-27"; + model_type_name[2] = "test_bmi_c"; + forcing_file[2] = find_file(forcing_dir_opts, "cat-27_2015-12-01 00_00_00_2015-12-30 23_00_00.csv"); + lib_file[2] = find_file(lib_dir_opts, BMI_TEST_C_LOCAL_LIB_NAME); + init_config[2] = find_file(bmi_init_cfg_dir_opts, "test_bmi_c_config_1.txt"); + main_output_variable[2] = "OUTPUT_VAR_1"; + stores_input_variable_metadata[2] = true; + registration_functions[2] = "register_bmi"; + uses_forcing_file[2] = false; + // tries_mass_balance[1] = false; + std::string variables_with_rain_rate = " \"output_variables\": [\"OUTPUT_VAR_2\",\n" " \"OUTPUT_VAR_1\"],\n"; @@ -183,6 +210,14 @@ void Bmi_C_Formulation_Test::SetUp() { std::shared_ptr params = std::make_shared( forcing_params(forcing_file[i], "legacy", "2015-12-01 00:00:00", "2015-12-30 23:00:00")); std::string variables_line = (i == 1) ? variables_with_rain_rate : ""; + + // Add this substring with this key for even indices or if the value for it is true + std::string store_metadata_substr = ""; + if (i % 2 == 0 || stores_input_variable_metadata[i]) { + store_metadata_substr = "\"" BMI_REALIZATION_CFG_PARAM_OPT__STORE_INPUT_VAR_METADATA "\": " ; + store_metadata_substr += stores_input_variable_metadata[i] ? "true," : "false,"; + } + forcing_params_examples[i] = params; config_json[i] = "{" " \"global\": {}," @@ -193,6 +228,10 @@ void Bmi_C_Formulation_Test::SetUp() { " \"library_file\": \"" + lib_file[i] + "\"," " \"init_config\": \"" + init_config[i] + "\"," " \"main_output_variable\": \"" + main_output_variable[i] + "\"," + + // Add this predetermined substring from above (which could be empty) + + store_metadata_substr + + " \"" + BMI_REALIZATION_CFG_PARAM_OPT__OUTPUT_PRECISION + "\": 6, " " \"" + BMI_REALIZATION_CFG_PARAM_OPT__VAR_STD_NAMES + "\": { " " \"INPUT_VAR_2\": \"" + AORC_FIELD_NAME_TEMP_2M_AG + "\"," @@ -236,6 +275,16 @@ TEST_F(Bmi_C_Formulation_Test, Initialize_0_a) { ASSERT_EQ(get_friend_bmi_main_output_var(formulation), main_output_variable[ex_index]); } +/** Test example 0 (which should be explicitly configured) has `store_input_variable_metadata` as `false`. */ +TEST_F(Bmi_C_Formulation_Test, Initialize_0_b) { + int ex_index = 0; + + Bmi_C_Formulation formulation(catchment_ids[ex_index], std::make_shared(*forcing_params_examples[ex_index]), utils::StreamHandler()); + formulation.create_formulation(config_prop_ptree[ex_index]); + + ASSERT_FALSE(get_friend_store_input_variable_metadata(formulation)); +} + /** Test to make sure we can initialize multiple model instances with dynamic loading. */ TEST_F(Bmi_C_Formulation_Test, Initialize_1_a) { Bmi_C_Formulation form_1(catchment_ids[0], std::make_shared(*forcing_params_examples[0]), utils::StreamHandler()); @@ -251,6 +300,38 @@ TEST_F(Bmi_C_Formulation_Test, Initialize_1_a) { ASSERT_EQ(header_2, "OUTPUT_VAR_2,OUTPUT_VAR_1"); } +/** Test example 1 (which should not be explicitly configured) has `store_input_variable_metadata` as `false`. */ +TEST_F(Bmi_C_Formulation_Test, Initialize_1_b) { + int ex_index = 1; + + Bmi_C_Formulation formulation(catchment_ids[ex_index], std::make_shared(*forcing_params_examples[ex_index]), utils::StreamHandler()); + formulation.create_formulation(config_prop_ptree[ex_index]); + + ASSERT_FALSE(get_friend_store_input_variable_metadata(formulation)); +} + +/** Simple test to make sure the model initializes. */ +TEST_F(Bmi_C_Formulation_Test, Initialize_2_a) { + int ex_index = 2; + + Bmi_C_Formulation formulation(catchment_ids[ex_index], std::make_shared(*forcing_params_examples[ex_index]), utils::StreamHandler()); + formulation.create_formulation(config_prop_ptree[ex_index]); + + ASSERT_EQ(get_friend_model_type_name(formulation), model_type_name[ex_index]); + ASSERT_EQ(get_friend_bmi_init_config(formulation), init_config[ex_index]); + ASSERT_EQ(get_friend_bmi_main_output_var(formulation), main_output_variable[ex_index]); +} + +/** Test example 2 has `store_input_variable_metadata` as `true`. */ +TEST_F(Bmi_C_Formulation_Test, Initialize_2_b) { + int ex_index = 2; + + Bmi_C_Formulation formulation(catchment_ids[ex_index], std::make_shared(*forcing_params_examples[ex_index]), utils::StreamHandler()); + formulation.create_formulation(config_prop_ptree[ex_index]); + + ASSERT_TRUE(get_friend_store_input_variable_metadata(formulation)); +} + /** Simple test of get response. */ TEST_F(Bmi_C_Formulation_Test, GetResponse_0_a) { int ex_index = 0; @@ -296,6 +377,22 @@ TEST_F(Bmi_C_Formulation_Test, GetResponse_0_b) { ASSERT_EQ(expected, response); } +/** Test of get response of example 0 (store metadata is `false`) to make sure no metadata stored. */ +TEST_F(Bmi_C_Formulation_Test, GetResponse_0_c) { + int ex_index = 0; + + Bmi_C_Formulation formulation(catchment_ids[ex_index], std::make_shared(*forcing_params_examples[ex_index]), utils::StreamHandler()); + formulation.create_formulation(config_prop_ptree[ex_index]); + + double response; + for (int i = 0; i < 39; i++) { + response = formulation.get_response(i, 3600); + } + + ASSERT_EQ(get_friend_known_bmi_input_vars(formulation).size(), 0); + ASSERT_EQ(get_friend_bmi_input_var_details(formulation), nullptr); +} + /** Test to make sure we can execute multiple model instances with dynamic loading. */ TEST_F(Bmi_C_Formulation_Test, GetResponse_1_a) { Bmi_C_Formulation form_1(catchment_ids[0], std::make_shared(*forcing_params_examples[0]), utils::StreamHandler()); @@ -313,6 +410,149 @@ TEST_F(Bmi_C_Formulation_Test, GetResponse_1_a) { } } +/** Test of get response for example 2 (using stored metadata) after several iterations. */ +TEST_F(Bmi_C_Formulation_Test, GetResponse_2_b) { + int ex_index = 2; + + Bmi_C_Formulation formulation(catchment_ids[ex_index], std::make_shared(*forcing_params_examples[ex_index]), utils::StreamHandler()); + formulation.create_formulation(config_prop_ptree[ex_index]); + + double response; + for (int i = 0; i < 39; i++) { + response = formulation.get_response(i, 3600); + } + double expected = 2.7809780039160068e-08; + ASSERT_EQ(expected, response); +} + + +/** Test of get response of example 2 (store metadata is `true`) to make sure metadata stored. */ +TEST_F(Bmi_C_Formulation_Test, GetResponse_2_c) { + int ex_index = 2; + + Bmi_C_Formulation formulation(catchment_ids[ex_index], std::make_shared(*forcing_params_examples[ex_index]), utils::StreamHandler()); + formulation.create_formulation(config_prop_ptree[ex_index]); + + double response; + for (int i = 0; i < 39; i++) { + response = formulation.get_response(i, 3600); + } + + std::vector* instance_metadata = get_friend_bmi_input_var_details(formulation); + std::set global_metadata = get_friend_known_bmi_input_vars(formulation); + + ASSERT_NE(instance_metadata, nullptr); + ASSERT_EQ(instance_metadata->size(), 2); + ASSERT_EQ(global_metadata.size(), 2); +} + +/** Test of `bmi_input_var_details` is populated correctly for example 2. */ +TEST_F(Bmi_C_Formulation_Test, bmi_input_var_details_2_a) { + int ex_index = 2; + + Bmi_C_Formulation formulation(catchment_ids[ex_index], std::make_shared(*forcing_params_examples[ex_index]), utils::StreamHandler()); + formulation.create_formulation(config_prop_ptree[ex_index]); + + double response; + for (int i = 0; i < 39; i++) { + response = formulation.get_response(i, 3600); + } + + std::vector* input_metadata = get_friend_bmi_input_var_details(formulation); + + const Bmi_Var_Details* var_metadata = input_metadata->at(0); + ASSERT_EQ(var_metadata->get_name(), "INPUT_VAR_1"); + ASSERT_EQ(var_metadata->get_mapped_alias(), AORC_FIELD_NAME_PRECIP_RATE); + ASSERT_EQ(var_metadata->get_units(), "m"); + ASSERT_EQ(var_metadata->get_item_size(), 8); + ASSERT_EQ(var_metadata->get_num_items(), 1); + + var_metadata = input_metadata->at(1); + ASSERT_EQ(var_metadata->get_name(), "INPUT_VAR_2"); + ASSERT_EQ(var_metadata->get_mapped_alias(), AORC_FIELD_NAME_TEMP_2M_AG); + ASSERT_EQ(var_metadata->get_units(), "Pa"); + ASSERT_EQ(var_metadata->get_item_size(), 8); + ASSERT_EQ(var_metadata->get_num_items(), 1); +} + +/** Test of `bmi_input_var_details` values come from same static object in example 2. */ +TEST_F(Bmi_C_Formulation_Test, bmi_input_var_details_2_b) { + int ex_index = 2; + + Bmi_C_Formulation formulation_1(catchment_ids[ex_index], std::make_shared(*forcing_params_examples[ex_index]), utils::StreamHandler()); + formulation_1.create_formulation(config_prop_ptree[ex_index]); + + Bmi_C_Formulation formulation_2(catchment_ids[ex_index], std::make_shared(*forcing_params_examples[ex_index]), utils::StreamHandler()); + formulation_2.create_formulation(config_prop_ptree[ex_index]); + + // Advance only one to start with + formulation_1.get_response(0, 3600); + + std::vector* instance_metadata_1 = get_friend_bmi_input_var_details(formulation_1); + std::vector* instance_metadata_2 = get_friend_bmi_input_var_details(formulation_2); + + // So, only the first of the instances has actually advanced and populated metadata objects + ASSERT_NE(instance_metadata_1, nullptr); + ASSERT_EQ(instance_metadata_2, nullptr); + ASSERT_EQ(instance_metadata_1->size(), 2); + + // But the static member should be populated + std::set global_metadata_1 = get_friend_known_bmi_input_vars(formulation_1); + std::set global_metadata_2 = get_friend_known_bmi_input_vars(formulation_2); + ASSERT_EQ(global_metadata_1.size(), 2); + ASSERT_EQ(global_metadata_1.size(), global_metadata_2.size()); + + // Now advance formulation 2, and see that it has two var metadata objects + formulation_2.get_response(0, 3600); + instance_metadata_2 = get_friend_bmi_input_var_details(formulation_2); + ASSERT_NE(instance_metadata_2, nullptr); + ASSERT_EQ(instance_metadata_2->size(), 2); + + // But the static metadata collections won't have grown ... + ASSERT_EQ(global_metadata_1.size(), 2); + ASSERT_EQ(global_metadata_1.size(), global_metadata_2.size()); + + // And the instance metadata pointers across the two instances (with different collection objects) will be the same + ASSERT_NE(&instance_metadata_1, &instance_metadata_2); + for (size_t i = 0; i < instance_metadata_1->size(); i++) { + const Bmi_Var_Details* inst_1_obj_ptr = instance_metadata_1->at(i); + const Bmi_Var_Details* inst_2_obj_ptr = instance_metadata_2->at(i); + ASSERT_EQ(inst_1_obj_ptr, inst_2_obj_ptr); + } +} + +/** Test of `bmi_input_var_details` values are same objects for two objects with same config based on example 2. */ +TEST_F(Bmi_C_Formulation_Test, bmi_input_var_details_2_c) { + int ex_index = 2; + + Bmi_C_Formulation formulation_1(catchment_ids[ex_index], std::make_shared(*forcing_params_examples[ex_index]), utils::StreamHandler()); + formulation_1.create_formulation(config_prop_ptree[ex_index]); + + Bmi_C_Formulation formulation_2(catchment_ids[ex_index], std::make_shared(*forcing_params_examples[ex_index]), utils::StreamHandler()); + formulation_2.create_formulation(config_prop_ptree[ex_index]); + + for (int i = 0; i < 10; i++) { + formulation_1.get_response(i, 3600); + formulation_2.get_response(i, 3600); + } + + std::vector* instance_metadata_1 = get_friend_bmi_input_var_details(formulation_1); + std::vector* instance_metadata_2 = get_friend_bmi_input_var_details(formulation_2); + + std::set global_metadata_1 = get_friend_known_bmi_input_vars(formulation_1); + std::set global_metadata_2 = get_friend_known_bmi_input_vars(formulation_2); + + + ASSERT_EQ(instance_metadata_1->size(), 2); + ASSERT_EQ(instance_metadata_1->size(), instance_metadata_2->size()); + ASSERT_EQ(global_metadata_1.size(), 2); + ASSERT_EQ(global_metadata_1.size(), global_metadata_2.size()); + + for (size_t i = 0; i < instance_metadata_1->size(); i++) { + ASSERT_EQ(instance_metadata_1->at(i), instance_metadata_2->at(i)); + } +} + /** Simple test of output. */ TEST_F(Bmi_C_Formulation_Test, GetOutputLineForTimestep_0_a) { int ex_index = 0; @@ -356,6 +596,21 @@ TEST_F(Bmi_C_Formulation_Test, GetOutputLineForTimestep_1_b) { EXPECT_THAT(output, MatchesRegex("580.799988,0.000001")); } +/** Simple test of output, picking time step when there was non-zero rain rate. */ +TEST_F(Bmi_C_Formulation_Test, GetOutputLineForTimestep_2_b) { + int ex_index = 2; + + Bmi_C_Formulation formulation(catchment_ids[ex_index], std::make_shared(*forcing_params_examples[ex_index]), utils::StreamHandler()); + formulation.create_formulation(config_prop_ptree[ex_index]); + + int i = 0; + while (i < 542) + formulation.get_response(i++, 3600); + formulation.get_response(i, 3600); + std::string output = formulation.get_output_line_for_timestep(i, ","); + EXPECT_THAT(output, MatchesRegex("0.000001,580.799988")); +} + TEST_F(Bmi_C_Formulation_Test, determine_model_time_offset_0_a) { int ex_index = 0; From 5835c1e99faa40e8cdf664ce470af8311dd036ab Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Thu, 19 Mar 2026 11:54:28 -0400 Subject: [PATCH 09/28] Fix whitespace for better style. --- src/realizations/catchment/Bmi_Module_Formulation.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/realizations/catchment/Bmi_Module_Formulation.cpp b/src/realizations/catchment/Bmi_Module_Formulation.cpp index aadbef312b..6837c0d39e 100644 --- a/src/realizations/catchment/Bmi_Module_Formulation.cpp +++ b/src/realizations/catchment/Bmi_Module_Formulation.cpp @@ -755,7 +755,7 @@ namespace realization { if (num_items != 1) { //more than a single value needed for var_name std::vector values = provider->get_values( - CatchmentAggrDataSelector(get_catchment_id(), mapped_alias, src_data_start, t_delta,units) + CatchmentAggrDataSelector(get_catchment_id(), mapped_alias, src_data_start, t_delta, units) ); //need to marshal data types to the receiver as well //this could be done a little more elegantly if the provider interface were @@ -778,7 +778,7 @@ namespace realization { else { try { //scalar value - double value = provider->get_value(CatchmentAggrDataSelector(this->get_catchment_id(),mapped_alias, src_data_start, t_delta, units)); + double value = provider->get_value(CatchmentAggrDataSelector(this->get_catchment_id(), mapped_alias, src_data_start, t_delta, units)); value_ptr = get_value_as_type(cpp_type, value); } catch (UnitsHelper::unit_conversion_exception &uce) { bool new_error = UnitsHelper::record_unit_conversion_fault(uce, "Bmi_Module_Formulation::perform_set", mapped_alias); From 565e6a05d9c05eb014eee1882afa77f38fcabf95 Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Thu, 19 Mar 2026 12:34:35 -0400 Subject: [PATCH 10/28] Avoid searching map twice. Updating logic in Bmi_Module_Formulation::get_provider_for_input_var to avoid searching a map twice back-to-back for the same key. --- .../catchment/Bmi_Module_Formulation.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/realizations/catchment/Bmi_Module_Formulation.cpp b/src/realizations/catchment/Bmi_Module_Formulation.cpp index 6837c0d39e..873f64e0b5 100644 --- a/src/realizations/catchment/Bmi_Module_Formulation.cpp +++ b/src/realizations/catchment/Bmi_Module_Formulation.cpp @@ -714,11 +714,16 @@ namespace realization { } std::shared_ptr& Bmi_Module_Formulation::get_provider_for_input_var(const std::string& var_name, const std::string& mapped_alias) { - if (input_forcing_providers.find(mapped_alias) != input_forcing_providers.end()) - return input_forcing_providers[mapped_alias]; + auto alias_iter = input_forcing_providers.find(mapped_alias); + if (alias_iter != input_forcing_providers.end()) + return alias_iter->second; - if (mapped_alias != var_name && input_forcing_providers.find(var_name) != input_forcing_providers.end()) - return input_forcing_providers[var_name]; + if (mapped_alias == var_name) + return forcing; + + auto name_iter = input_forcing_providers.find(var_name); + if (name_iter != input_forcing_providers.end()) + return name_iter->second; return forcing; } From 2e1df82250aa45a019f1af6cc32e5b03524f978a Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Thu, 19 Mar 2026 12:49:26 -0400 Subject: [PATCH 11/28] Fix typo with extra ; --- include/realizations/catchment/Bmi_Module_Formulation.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/realizations/catchment/Bmi_Module_Formulation.hpp b/include/realizations/catchment/Bmi_Module_Formulation.hpp index 3e5e0dfaa8..2867000ef8 100644 --- a/include/realizations/catchment/Bmi_Module_Formulation.hpp +++ b/include/realizations/catchment/Bmi_Module_Formulation.hpp @@ -72,7 +72,7 @@ namespace realization { /** String for the C++ type corresponding to this variable's type. */ const std::string cpp_type; /** String for variable's units. */ - const std::string units;; + const std::string units; /** The size of individual items for this variable. */ int item_size; /** The number of items for this variable. */ From 48d575210e33f176163743448ff5d9b4f377382f Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Thu, 19 Mar 2026 15:13:32 -0400 Subject: [PATCH 12/28] Avoid stdlib issue with const vector template var. --- include/realizations/catchment/Bmi_Module_Formulation.hpp | 4 ++-- src/realizations/catchment/Bmi_Module_Formulation.cpp | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/realizations/catchment/Bmi_Module_Formulation.hpp b/include/realizations/catchment/Bmi_Module_Formulation.hpp index 2867000ef8..500674dd4b 100644 --- a/include/realizations/catchment/Bmi_Module_Formulation.hpp +++ b/include/realizations/catchment/Bmi_Module_Formulation.hpp @@ -536,7 +536,7 @@ namespace realization { * * These should be pointers to @ref Bmi_Var_Details instances in @ref known_bmi_input_vars. */ - std::unique_ptr> bmi_input_var_details; + std::unique_ptr> bmi_input_var_details; /** * Vector of data providers for BMI input vars, with the provider at an index corresponding to the var in @@ -545,7 +545,7 @@ namespace realization { * As with @ref bmi_input_var_details, these should be populated during the first call to * @ref do_bmi_sets_from_stored_metadata via a nested call to @ref initialize_bmi_input_var_metadata. */ - std::unique_ptr>> bmi_input_providers; + std::unique_ptr>> bmi_input_providers; models::bmi::protocols::NgenBmiProtocols bmi_protocols; /** diff --git a/src/realizations/catchment/Bmi_Module_Formulation.cpp b/src/realizations/catchment/Bmi_Module_Formulation.cpp index 873f64e0b5..f345173d7d 100644 --- a/src/realizations/catchment/Bmi_Module_Formulation.cpp +++ b/src/realizations/catchment/Bmi_Module_Formulation.cpp @@ -732,8 +732,8 @@ namespace realization { if (bmi_input_var_details != nullptr) { throw std::runtime_error("Cannot re-initialize module formulation bmi_input_var_details member"); } - bmi_input_var_details = std::make_unique>(); - bmi_input_providers = std::make_unique>>(); + bmi_input_var_details = std::make_unique>(); + bmi_input_providers = std::make_unique>>(); for (std::string & var_name : get_bmi_model()->GetInputVarNames()) { int item_size = get_bmi_model()->GetVarItemsize(var_name); std::string mapped_alias = get_config_mapped_variable_name(var_name); @@ -748,7 +748,7 @@ namespace realization { get_bmi_model()->get_analogous_cxx_type(get_bmi_model()->GetVarType(var_name), item_size), get_bmi_model()->GetVarUnits(var_name))); - bmi_input_var_details->push_back(&(*(iter_and_result.first))); + bmi_input_var_details->push_back(const_cast(&(*(iter_and_result.first)))); bmi_input_providers->push_back(get_provider_for_input_var(var_name, mapped_alias)); } } From 3e546e9aa1913746c460edae8bf324a2182b06f8 Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Thu, 19 Mar 2026 15:14:04 -0400 Subject: [PATCH 13/28] Fix tests for prior stdlib vector const change. --- .../catchments/Bmi_C_Formulation_Test.cpp | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/realizations/catchments/Bmi_C_Formulation_Test.cpp b/test/realizations/catchments/Bmi_C_Formulation_Test.cpp index 6c7ebd1cf2..1a397bd250 100644 --- a/test/realizations/catchments/Bmi_C_Formulation_Test.cpp +++ b/test/realizations/catchments/Bmi_C_Formulation_Test.cpp @@ -73,7 +73,7 @@ class Bmi_C_Formulation_Test : public ::testing::Test { return formulation.known_bmi_input_vars; } - static std::vector* get_friend_bmi_input_var_details(Bmi_C_Formulation& formulation) { + static std::vector* get_friend_bmi_input_var_details(Bmi_C_Formulation& formulation) { return formulation.bmi_input_var_details.get(); } @@ -438,7 +438,7 @@ TEST_F(Bmi_C_Formulation_Test, GetResponse_2_c) { response = formulation.get_response(i, 3600); } - std::vector* instance_metadata = get_friend_bmi_input_var_details(formulation); + std::vector* instance_metadata = get_friend_bmi_input_var_details(formulation); std::set global_metadata = get_friend_known_bmi_input_vars(formulation); ASSERT_NE(instance_metadata, nullptr); @@ -458,9 +458,9 @@ TEST_F(Bmi_C_Formulation_Test, bmi_input_var_details_2_a) { response = formulation.get_response(i, 3600); } - std::vector* input_metadata = get_friend_bmi_input_var_details(formulation); + std::vector* input_metadata = get_friend_bmi_input_var_details(formulation); - const Bmi_Var_Details* var_metadata = input_metadata->at(0); + Bmi_Var_Details* var_metadata = input_metadata->at(0); ASSERT_EQ(var_metadata->get_name(), "INPUT_VAR_1"); ASSERT_EQ(var_metadata->get_mapped_alias(), AORC_FIELD_NAME_PRECIP_RATE); ASSERT_EQ(var_metadata->get_units(), "m"); @@ -488,8 +488,8 @@ TEST_F(Bmi_C_Formulation_Test, bmi_input_var_details_2_b) { // Advance only one to start with formulation_1.get_response(0, 3600); - std::vector* instance_metadata_1 = get_friend_bmi_input_var_details(formulation_1); - std::vector* instance_metadata_2 = get_friend_bmi_input_var_details(formulation_2); + std::vector* instance_metadata_1 = get_friend_bmi_input_var_details(formulation_1); + std::vector* instance_metadata_2 = get_friend_bmi_input_var_details(formulation_2); // So, only the first of the instances has actually advanced and populated metadata objects ASSERT_NE(instance_metadata_1, nullptr); @@ -515,8 +515,8 @@ TEST_F(Bmi_C_Formulation_Test, bmi_input_var_details_2_b) { // And the instance metadata pointers across the two instances (with different collection objects) will be the same ASSERT_NE(&instance_metadata_1, &instance_metadata_2); for (size_t i = 0; i < instance_metadata_1->size(); i++) { - const Bmi_Var_Details* inst_1_obj_ptr = instance_metadata_1->at(i); - const Bmi_Var_Details* inst_2_obj_ptr = instance_metadata_2->at(i); + Bmi_Var_Details* inst_1_obj_ptr = instance_metadata_1->at(i); + Bmi_Var_Details* inst_2_obj_ptr = instance_metadata_2->at(i); ASSERT_EQ(inst_1_obj_ptr, inst_2_obj_ptr); } } @@ -536,8 +536,8 @@ TEST_F(Bmi_C_Formulation_Test, bmi_input_var_details_2_c) { formulation_2.get_response(i, 3600); } - std::vector* instance_metadata_1 = get_friend_bmi_input_var_details(formulation_1); - std::vector* instance_metadata_2 = get_friend_bmi_input_var_details(formulation_2); + std::vector* instance_metadata_1 = get_friend_bmi_input_var_details(formulation_1); + std::vector* instance_metadata_2 = get_friend_bmi_input_var_details(formulation_2); std::set global_metadata_1 = get_friend_known_bmi_input_vars(formulation_1); std::set global_metadata_2 = get_friend_known_bmi_input_vars(formulation_2); From 3cd29e1e27177804827c8e822f0d002cb8d8ee23 Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Mon, 30 Mar 2026 16:00:56 -0400 Subject: [PATCH 14/28] Refactor Bmi_Module_Formulation::perform_set. Updating with more concise params list that rolls several things into a Bmi_Var_Details object, and then updating usage to match. --- .../catchment/Bmi_Module_Formulation.hpp | 5 +- .../catchment/Bmi_Module_Formulation.cpp | 73 +++++++++---------- 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/include/realizations/catchment/Bmi_Module_Formulation.hpp b/include/realizations/catchment/Bmi_Module_Formulation.hpp index 500674dd4b..7a82869ebe 100644 --- a/include/realizations/catchment/Bmi_Module_Formulation.hpp +++ b/include/realizations/catchment/Bmi_Module_Formulation.hpp @@ -663,7 +663,10 @@ namespace realization { */ void initialize_bmi_input_var_metadata(); - void perform_set(const time_t &src_data_start, const time_step_t &t_delta, const int num_items, data_access::GenericDataProvider *provider, const std::string &mapped_alias, const std::string &var_name, const std::string &cpp_type, const std::string &units) const; + void perform_set(const time_t &src_data_start, + const time_step_t &t_delta, + const std::shared_ptr& provider, + const Bmi_Var_Details *var_details) const; }; /* template diff --git a/src/realizations/catchment/Bmi_Module_Formulation.cpp b/src/realizations/catchment/Bmi_Module_Formulation.cpp index f345173d7d..b0b94af5ad 100644 --- a/src/realizations/catchment/Bmi_Module_Formulation.cpp +++ b/src/realizations/catchment/Bmi_Module_Formulation.cpp @@ -682,16 +682,7 @@ namespace realization { } for (size_t i = 0; i < bmi_input_var_details->size(); i++) { - perform_set( - src_data_start, - t_delta, - bmi_input_var_details->at(i)->get_num_items(), - bmi_input_providers->at(i).get(), - bmi_input_var_details->at(i)->get_mapped_alias(), - bmi_input_var_details->at(i)->get_name(), - bmi_input_var_details->at(i)->get_cpp_type(), - bmi_input_var_details->at(i)->get_units() - ); + perform_set(src_data_start, t_delta, bmi_input_providers->at(i), bmi_input_var_details->at(i)); } } @@ -700,16 +691,17 @@ namespace realization { int item_size = get_bmi_model()->GetVarItemsize(var_name); std::string mapped_alias = get_config_mapped_variable_name(var_name); - perform_set( - src_data_start, - t_delta, - get_bmi_model()->GetVarNbytes(var_name) / item_size, - get_provider_for_input_var(var_name, mapped_alias).get(), - mapped_alias, + // Create a local Bmi_Var_Details object for this variable + Bmi_Var_Details var_details( var_name, + mapped_alias, + item_size, + get_bmi_model()->GetVarNbytes(var_name) / item_size, get_bmi_model()->get_analogous_cxx_type(get_bmi_model()->GetVarType(var_name), item_size), get_bmi_model()->GetVarUnits(var_name) - ); + ); + + perform_set(src_data_start, t_delta, get_provider_for_input_var(var_name, mapped_alias), &var_details); } } @@ -753,54 +745,61 @@ namespace realization { } } - void Bmi_Module_Formulation::perform_set(const time_t &src_data_start, const time_step_t& t_delta, - const int num_items, data_access::GenericDataProvider* provider, const std::string& mapped_alias, - const std::string& var_name, const std::string& cpp_type, const std::string& units) const { + void Bmi_Module_Formulation::perform_set(const time_t& src_data_start, + const time_step_t& t_delta, + const std::shared_ptr& provider, + const Bmi_Var_Details* var_details) const { std::shared_ptr value_ptr; - if (num_items != 1) { + if (var_details->get_num_items() != 1) { //more than a single value needed for var_name std::vector values = provider->get_values( - CatchmentAggrDataSelector(get_catchment_id(), mapped_alias, src_data_start, t_delta, units) + CatchmentAggrDataSelector(get_catchment_id(), var_details->get_mapped_alias(), src_data_start, + t_delta, var_details->get_units()) ); //need to marshal data types to the receiver as well //this could be done a little more elegantly if the provider interface were //"type aware", but for now, this will do (but requires yet another copy) - if(values.size() == 1){ + if (values.size() == 1) { //FIXME this isn't generic broadcasting, but works for scalar implementations #ifndef NGEN_QUIET - std::cerr << "WARN: broadcasting variable '" << var_name << "' from scalar to expected array\n"; + std::cerr << "WARN: broadcasting variable '" << var_details->get_name() << + "' from scalar to expected array\n"; #endif - values.resize(num_items, values[0]); + values.resize(var_details->get_num_items(), values[0]); } - else if (values.size() != num_items) { - throw std::runtime_error("Mismatch in item count for variable '" + var_name + "': model expects " - + std::to_string(num_items) + ", provider returned " + else if (values.size() != var_details->get_num_items()) { + throw std::runtime_error( + "Mismatch in item count for variable '" + var_details->get_name() + "': model expects " + + std::to_string(var_details->get_num_items()) + ", provider returned " + std::to_string(values.size()) + " items\n"); } - value_ptr = get_values_as_type(cpp_type, values.begin(), values.end() ); - + value_ptr = get_values_as_type(var_details->get_cpp_type(), values.begin(), values.end()); } else { try { //scalar value - double value = provider->get_value(CatchmentAggrDataSelector(this->get_catchment_id(), mapped_alias, src_data_start, t_delta, units)); - value_ptr = get_value_as_type(cpp_type, value); + double value = provider->get_value(CatchmentAggrDataSelector( + this->get_catchment_id(), var_details->get_mapped_alias(), src_data_start, t_delta, + var_details->get_units())); + value_ptr = get_value_as_type(var_details->get_cpp_type(), value); } catch (UnitsHelper::unit_conversion_exception &uce) { - bool new_error = UnitsHelper::record_unit_conversion_fault(uce, "Bmi_Module_Formulation::perform_set", mapped_alias); + bool new_error = UnitsHelper::record_unit_conversion_fault( + uce, "Bmi_Module_Formulation::perform_set", var_details->get_mapped_alias()); if (new_error) { std::stringstream ss; ss << "Unit conversion failure:" << " requester {'" << get_bmi_model()->get_model_name() << "' catchment '" << get_catchment_id() - << "' variable '" << var_name << "'" << " (alias '" << mapped_alias << "')" - << " units '" << units << "'}" + << "' variable '" << var_details->get_name() << "'" + << " (alias '" << var_details->get_mapped_alias() << "')" + << " units '" << var_details->get_units() << "'}" << " provider {'" << uce.provider_model_name << "' source variable '" << uce.provider_var_name << "'" << " raw value " << uce.unconverted_values[0] << "}" << " message \"" << uce.what() << "\"\n"; logging::warning(ss.str().c_str()); ss.str(""); } - value_ptr = get_value_as_type(cpp_type, uce.unconverted_values[0]); + value_ptr = get_value_as_type(var_details->get_cpp_type(), uce.unconverted_values[0]); } } - get_bmi_model()->SetValue(var_name, value_ptr.get()); + get_bmi_model()->SetValue(var_details->get_name(), value_ptr.get()); } } From 0dc1e8bf9e8cb0b2d0482ab13c5805c0f757ce3b Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Mon, 30 Mar 2026 16:07:42 -0400 Subject: [PATCH 15/28] Docstring for Bmi_Module_Formulation::perform_set. --- .../realizations/catchment/Bmi_Module_Formulation.hpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/include/realizations/catchment/Bmi_Module_Formulation.hpp b/include/realizations/catchment/Bmi_Module_Formulation.hpp index 7a82869ebe..ee6967af0b 100644 --- a/include/realizations/catchment/Bmi_Module_Formulation.hpp +++ b/include/realizations/catchment/Bmi_Module_Formulation.hpp @@ -663,6 +663,16 @@ namespace realization { */ void initialize_bmi_input_var_metadata(); + /** + * Do the action of retrieving data and setting values for an input variable ahead of advancing the model. + * + * @param src_data_start The start time (in seconds) to use when retrieving data from the appropriate provider + * to use for setting the model's variables. + * @param t_delta The size of the time step over which the formulation is going to update the model, which might + * be different than the model's internal time step. + * @param provider The data provider from which to source the data to use to set the variable. + * @param var_details Variable details struct containing the remainder of information required (e.g., var name). + */ void perform_set(const time_t &src_data_start, const time_step_t &t_delta, const std::shared_ptr& provider, From 55bc1f49ed8710f5de8e4111f3886a36339fc52f Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Mon, 30 Mar 2026 16:13:46 -0400 Subject: [PATCH 16/28] Rename to cache_input_variable_metadata. Rename store_input_variable_metadata to cache_input_variable_metadata to clarify things are saved in mem not stored on disk; updating macro name and documentation to also reflect and better indicate this. --- doc/BMI_MODELS.md | 6 +++--- .../realizations/catchment/Bmi_Formulation.hpp | 2 +- .../catchment/Bmi_Module_Formulation.hpp | 6 +++--- .../catchment/Bmi_Module_Formulation.cpp | 8 ++++---- test/realizations/Formulation_Manager_Test.cpp | 10 +++++----- .../catchments/Bmi_C_Formulation_Test.cpp | 18 +++++++++--------- 6 files changed, 25 insertions(+), 25 deletions(-) diff --git a/doc/BMI_MODELS.md b/doc/BMI_MODELS.md index ca94e33067..7a7f31aec0 100644 --- a/doc/BMI_MODELS.md +++ b/doc/BMI_MODELS.md @@ -183,16 +183,16 @@ There are some special BMI formulation config parameters which are required in c * `fixed_time_step` * boolean value to indicate whether this model has a fixed time step size * implied to be `true` by default -* `store_input_variable_metadata` +* `cache_input_variable_metadata` * boolean value, `false` by default - * indicates whether BMI input variable metadata should be stored on first look-up and reused, thus optimizing execution + * indicates whether BMI input variable metadata should be saved in memory on first look-up and reused, thus optimizing execution * BMI input variables have values set at each time step * certain metadata is required when performing those set operations: * name and mapped alias (if set) * units * memory size and type of individual items (e.g., `int`, `float`, `double`, etc.) * whether the variable is an array, and if so, the number of individual items - * for BMI modules that will not change applicable metadata throughout a simulation, this can be stored and reused + * for BMI modules that will not change applicable metadata throughout a simulation, this can be saved in memory and reused * the BMI specification does not expressly guarantee metadata values will not change, so configuration must consider the particular BMI module in use * also, these optimizations do result in additional memory usage * while relatively small, can scale significantly for larger simulations, so this may not be usable for situations when memory is constrained diff --git a/include/realizations/catchment/Bmi_Formulation.hpp b/include/realizations/catchment/Bmi_Formulation.hpp index 23e14e71fd..32169ef169 100644 --- a/include/realizations/catchment/Bmi_Formulation.hpp +++ b/include/realizations/catchment/Bmi_Formulation.hpp @@ -33,7 +33,7 @@ #define BMI_REALIZATION_CFG_PARAM_OPT__CPP_DESTROY_FUNC "destroy_function" #define BMI_REALIZATION_CFG_PARAM_OPT__CPP_CREATE_FUNC_DEFAULT "bmi_model_create" #define BMI_REALIZATION_CFG_PARAM_OPT__CPP_DESTROY_FUNC_DEFAULT "bmi_model_destroy" -#define BMI_REALIZATION_CFG_PARAM_OPT__STORE_INPUT_VAR_METADATA "store_input_variable_metadata" +#define BMI_REALIZATION_CFG_PARAM_OPT__CACHE_INPUT_VAR_METADATA "cache_input_variable_metadata" /* *************** See also the Forcing.h file for several CSDMS Standard Names definitions *************** */ diff --git a/include/realizations/catchment/Bmi_Module_Formulation.hpp b/include/realizations/catchment/Bmi_Module_Formulation.hpp index ee6967af0b..cee30bee2a 100644 --- a/include/realizations/catchment/Bmi_Module_Formulation.hpp +++ b/include/realizations/catchment/Bmi_Module_Formulation.hpp @@ -461,7 +461,7 @@ namespace realization { /** * Set BMI input variable values for the model appropriately prior to calling its `BMI `update()``. * - * Depending on the value of @ref store_input_variable_metadata (`false` by default, but which can be controlled + * Depending on the value of @ref cache_input_variable_metadata (`false` by default, but which can be controlled * using @ref set_store_input_var_metadata), this will defer most of its execution to a call either to * @ref do_bmi_sets_from_stored_metadata or @ref do_bmi_sets_with_full_refetch. * @@ -474,7 +474,7 @@ namespace realization { /** * Set member variable indicating whether @ref set_model_inputs_prior_to_update should store and reuse metadata. * - * Set the @ref store_input_variable_metadata member variable, which indicates whether + * Set the @ref cache_input_variable_metadata member variable, which indicates whether * @ref set_model_inputs_prior_to_update should store and reuse metadata, as opposed to refreshing such data * each time @ref set_model_inputs_prior_to_update is called. * @@ -586,7 +586,7 @@ namespace realization { }; /** Whether @ref set_model_inputs_prior_to_update should store and reuse metadata. */ - bool store_input_variable_metadata = false; + bool cache_input_variable_metadata = false; /** * Set BMI input variables before `BMI update, using saved metadata rather than re-fetching or re-calculating. diff --git a/src/realizations/catchment/Bmi_Module_Formulation.cpp b/src/realizations/catchment/Bmi_Module_Formulation.cpp index b0b94af5ad..b6479c8cfb 100644 --- a/src/realizations/catchment/Bmi_Module_Formulation.cpp +++ b/src/realizations/catchment/Bmi_Module_Formulation.cpp @@ -348,9 +348,9 @@ namespace realization { properties.at(BMI_REALIZATION_CFG_PARAM_OPT__FIXED_TIME_STEP).as_boolean()); } - if (properties.find(BMI_REALIZATION_CFG_PARAM_OPT__STORE_INPUT_VAR_METADATA) != properties.end()) { + if (properties.find(BMI_REALIZATION_CFG_PARAM_OPT__CACHE_INPUT_VAR_METADATA) != properties.end()) { set_store_input_var_metadata( - properties.at(BMI_REALIZATION_CFG_PARAM_OPT__STORE_INPUT_VAR_METADATA).as_boolean()); + properties.at(BMI_REALIZATION_CFG_PARAM_OPT__CACHE_INPUT_VAR_METADATA).as_boolean()); } auto std_names_it = properties.find(BMI_REALIZATION_CFG_PARAM_OPT__VAR_STD_NAMES); @@ -664,7 +664,7 @@ namespace realization { void Bmi_Module_Formulation::set_model_inputs_prior_to_update(const double &model_time, time_step_t t_delta) { time_t forcing_start = convert_model_time(model_time) + get_bmi_model_start_time_forcing_offset_s(); - if (store_input_variable_metadata) { + if (cache_input_variable_metadata) { do_bmi_sets_from_stored_metadata(forcing_start, t_delta); } else { @@ -673,7 +673,7 @@ namespace realization { } void Bmi_Module_Formulation::set_store_input_var_metadata(bool store_input_var_metadata) { - store_input_variable_metadata = store_input_var_metadata; + cache_input_variable_metadata = store_input_var_metadata; } void Bmi_Module_Formulation::do_bmi_sets_from_stored_metadata(const time_t &src_data_start, const time_step_t &t_delta) { diff --git a/test/realizations/Formulation_Manager_Test.cpp b/test/realizations/Formulation_Manager_Test.cpp index 8c01cd11ba..400cae3061 100644 --- a/test/realizations/Formulation_Manager_Test.cpp +++ b/test/realizations/Formulation_Manager_Test.cpp @@ -892,7 +892,7 @@ const std::string EXAMPLE_9 = "{ " "\"library_file\": \"{{EXTERN_LIB_DIR_PATH}}" BMI_TEST_CPP_LIB_NAME "\"," "\"init_config\": \"{{BMI_C_INIT_DIR_PATH}}/test_bmi_c_config_0.txt\"," "\"main_output_variable\": \"OUTPUT_VAR_2\"," - "\"store_input_variable_metadata\": true," + "\"cache_input_variable_metadata\": true," "\"" BMI_REALIZATION_CFG_PARAM_OPT__VAR_STD_NAMES "\": { " "\"INPUT_VAR_2\": \"" AORC_FIELD_NAME_TEMP_2M_AG "\"," "\"INPUT_VAR_1\": \"" AORC_FIELD_NAME_PRECIP_RATE "\"" @@ -979,7 +979,7 @@ const std::string EXAMPLE_10 = "{ " "\"library_file\": \"{{EXTERN_LIB_DIR_PATH}}" BMI_TEST_CPP_LIB_NAME "\"," "\"init_config\": \"{{BMI_C_INIT_DIR_PATH}}/test_bmi_c_config_0.txt\"," "\"main_output_variable\": \"OUTPUT_VAR_2\"," - "\"store_input_variable_metadata\": true," + "\"cache_input_variable_metadata\": true," "\"" BMI_REALIZATION_CFG_PARAM_OPT__VAR_STD_NAMES "\": { " "\"INPUT_VAR_2\": \"" AORC_FIELD_NAME_TEMP_2M_AG "\"," "\"INPUT_VAR_1\": \"" AORC_FIELD_NAME_PRECIP_RATE "\"" @@ -1011,7 +1011,7 @@ const std::string EXAMPLE_10 = "{ " "\"library_file\": \"{{EXTERN_LIB_DIR_PATH}}" BMI_TEST_CPP_LIB_NAME "\"," "\"init_config\": \"{{BMI_C_INIT_DIR_PATH}}/test_bmi_c_config_0.txt\"," "\"main_output_variable\": \"OUTPUT_VAR_2\"," - "\"store_input_variable_metadata\": true," + "\"cache_input_variable_metadata\": true," "\"" BMI_REALIZATION_CFG_PARAM_OPT__VAR_STD_NAMES "\": { " "\"INPUT_VAR_2\": \"" AORC_FIELD_NAME_TEMP_2M_AG "\"," "\"INPUT_VAR_1\": \"" AORC_FIELD_NAME_PRECIP_RATE "\"" @@ -1214,7 +1214,7 @@ TEST_F(Formulation_Manager_Test, basic_run_3) { } /** - * Testing config the same as EX 1 (like in basic_run_1) but with `store_input_variable_metadata` true for global + * Testing config the same as EX 1 (like in basic_run_1) but with `cache_input_variable_metadata` true for global * formulation config (which is all formulations). */ TEST_F(Formulation_Manager_Test, basic_run_9) { @@ -1253,7 +1253,7 @@ TEST_F(Formulation_Manager_Test, basic_run_9) { } /** - * Testing config the same as EX 3 (like in basic_run_3) but with `store_input_variable_metadata` true for global and + * Testing config the same as EX 3 (like in basic_run_3) but with `cache_input_variable_metadata` true for global and * one catchment formulation config (but not the other). */ TEST_F(Formulation_Manager_Test, basic_run_10) { diff --git a/test/realizations/catchments/Bmi_C_Formulation_Test.cpp b/test/realizations/catchments/Bmi_C_Formulation_Test.cpp index 1a397bd250..4811b3b999 100644 --- a/test/realizations/catchments/Bmi_C_Formulation_Test.cpp +++ b/test/realizations/catchments/Bmi_C_Formulation_Test.cpp @@ -65,8 +65,8 @@ class Bmi_C_Formulation_Test : public ::testing::Test { return formulation.get_model_type_name(); } - static bool get_friend_store_input_variable_metadata(Bmi_C_Formulation& formulation) { - return formulation.store_input_variable_metadata; + static bool get_friend_cache_input_variable_metadata(Bmi_C_Formulation& formulation) { + return formulation.cache_input_variable_metadata; } static std::set& get_friend_known_bmi_input_vars(Bmi_C_Formulation& formulation) { @@ -214,7 +214,7 @@ void Bmi_C_Formulation_Test::SetUp() { // Add this substring with this key for even indices or if the value for it is true std::string store_metadata_substr = ""; if (i % 2 == 0 || stores_input_variable_metadata[i]) { - store_metadata_substr = "\"" BMI_REALIZATION_CFG_PARAM_OPT__STORE_INPUT_VAR_METADATA "\": " ; + store_metadata_substr = "\"" BMI_REALIZATION_CFG_PARAM_OPT__CACHE_INPUT_VAR_METADATA "\": " ; store_metadata_substr += stores_input_variable_metadata[i] ? "true," : "false,"; } @@ -275,14 +275,14 @@ TEST_F(Bmi_C_Formulation_Test, Initialize_0_a) { ASSERT_EQ(get_friend_bmi_main_output_var(formulation), main_output_variable[ex_index]); } -/** Test example 0 (which should be explicitly configured) has `store_input_variable_metadata` as `false`. */ +/** Test example 0 (which should be explicitly configured) has `cache_input_variable_metadata` as `false`. */ TEST_F(Bmi_C_Formulation_Test, Initialize_0_b) { int ex_index = 0; Bmi_C_Formulation formulation(catchment_ids[ex_index], std::make_shared(*forcing_params_examples[ex_index]), utils::StreamHandler()); formulation.create_formulation(config_prop_ptree[ex_index]); - ASSERT_FALSE(get_friend_store_input_variable_metadata(formulation)); + ASSERT_FALSE(get_friend_cache_input_variable_metadata(formulation)); } /** Test to make sure we can initialize multiple model instances with dynamic loading. */ @@ -300,14 +300,14 @@ TEST_F(Bmi_C_Formulation_Test, Initialize_1_a) { ASSERT_EQ(header_2, "OUTPUT_VAR_2,OUTPUT_VAR_1"); } -/** Test example 1 (which should not be explicitly configured) has `store_input_variable_metadata` as `false`. */ +/** Test example 1 (which should not be explicitly configured) has `cache_input_variable_metadata` as `false`. */ TEST_F(Bmi_C_Formulation_Test, Initialize_1_b) { int ex_index = 1; Bmi_C_Formulation formulation(catchment_ids[ex_index], std::make_shared(*forcing_params_examples[ex_index]), utils::StreamHandler()); formulation.create_formulation(config_prop_ptree[ex_index]); - ASSERT_FALSE(get_friend_store_input_variable_metadata(formulation)); + ASSERT_FALSE(get_friend_cache_input_variable_metadata(formulation)); } /** Simple test to make sure the model initializes. */ @@ -322,14 +322,14 @@ TEST_F(Bmi_C_Formulation_Test, Initialize_2_a) { ASSERT_EQ(get_friend_bmi_main_output_var(formulation), main_output_variable[ex_index]); } -/** Test example 2 has `store_input_variable_metadata` as `true`. */ +/** Test example 2 has `cache_input_variable_metadata` as `true`. */ TEST_F(Bmi_C_Formulation_Test, Initialize_2_b) { int ex_index = 2; Bmi_C_Formulation formulation(catchment_ids[ex_index], std::make_shared(*forcing_params_examples[ex_index]), utils::StreamHandler()); formulation.create_formulation(config_prop_ptree[ex_index]); - ASSERT_TRUE(get_friend_store_input_variable_metadata(formulation)); + ASSERT_TRUE(get_friend_cache_input_variable_metadata(formulation)); } /** Simple test of get response. */ From ca6dfbf5693fe74da348c3022eca1d75e7876563 Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Wed, 1 Apr 2026 14:05:59 -0400 Subject: [PATCH 17/28] More renaming cache_input_variable_metadata. Renaming related params, functions, and similarly (but not identically) named variables in the same way. --- .../catchment/Bmi_Module_Formulation.hpp | 6 +++--- .../catchment/Bmi_Module_Formulation.cpp | 6 +++--- .../catchments/Bmi_C_Formulation_Test.cpp | 20 +++++++++---------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/include/realizations/catchment/Bmi_Module_Formulation.hpp b/include/realizations/catchment/Bmi_Module_Formulation.hpp index cee30bee2a..bd007a9f6b 100644 --- a/include/realizations/catchment/Bmi_Module_Formulation.hpp +++ b/include/realizations/catchment/Bmi_Module_Formulation.hpp @@ -462,7 +462,7 @@ namespace realization { * Set BMI input variable values for the model appropriately prior to calling its `BMI `update()``. * * Depending on the value of @ref cache_input_variable_metadata (`false` by default, but which can be controlled - * using @ref set_store_input_var_metadata), this will defer most of its execution to a call either to + * using @ref set_cache_input_var_metadata), this will defer most of its execution to a call either to * @ref do_bmi_sets_from_stored_metadata or @ref do_bmi_sets_with_full_refetch. * * @param model_time The model's time prior to the update, in its internal units and representation. @@ -478,9 +478,9 @@ namespace realization { * @ref set_model_inputs_prior_to_update should store and reuse metadata, as opposed to refreshing such data * each time @ref set_model_inputs_prior_to_update is called. * - * @param store_input_var_metadata Whether @ref set_model_inputs_prior_to_update should store and reuse metadata + * @param cache_input_var_metadata Whether @ref set_model_inputs_prior_to_update should store and reuse metadata */ - void set_store_input_var_metadata(bool store_input_var_metadata); + void set_cache_input_var_metadata(bool cache_input_var_metadata); /** The delta of the last model update execution (typically, this is time step size). */ time_step_t last_model_response_delta = 0; diff --git a/src/realizations/catchment/Bmi_Module_Formulation.cpp b/src/realizations/catchment/Bmi_Module_Formulation.cpp index b6479c8cfb..8c42f48b23 100644 --- a/src/realizations/catchment/Bmi_Module_Formulation.cpp +++ b/src/realizations/catchment/Bmi_Module_Formulation.cpp @@ -349,7 +349,7 @@ namespace realization { } if (properties.find(BMI_REALIZATION_CFG_PARAM_OPT__CACHE_INPUT_VAR_METADATA) != properties.end()) { - set_store_input_var_metadata( + set_cache_input_var_metadata( properties.at(BMI_REALIZATION_CFG_PARAM_OPT__CACHE_INPUT_VAR_METADATA).as_boolean()); } @@ -672,8 +672,8 @@ namespace realization { } } - void Bmi_Module_Formulation::set_store_input_var_metadata(bool store_input_var_metadata) { - cache_input_variable_metadata = store_input_var_metadata; + void Bmi_Module_Formulation::set_cache_input_var_metadata(bool cache_input_var_metadata) { + cache_input_variable_metadata = cache_input_var_metadata; } void Bmi_Module_Formulation::do_bmi_sets_from_stored_metadata(const time_t &src_data_start, const time_step_t &t_delta) { diff --git a/test/realizations/catchments/Bmi_C_Formulation_Test.cpp b/test/realizations/catchments/Bmi_C_Formulation_Test.cpp index 4811b3b999..d6a6d90fa1 100644 --- a/test/realizations/catchments/Bmi_C_Formulation_Test.cpp +++ b/test/realizations/catchments/Bmi_C_Formulation_Test.cpp @@ -122,7 +122,7 @@ class Bmi_C_Formulation_Test : public ::testing::Test { std::vector config_json; std::vector catchment_ids; std::vector model_type_name; - std::vector stores_input_variable_metadata; + std::vector caches_input_variable_metadata; std::vector forcing_file; std::vector lib_file; std::vector init_config; @@ -160,7 +160,7 @@ void Bmi_C_Formulation_Test::SetUp() { lib_file = std::vector(EX_COUNT); init_config = std::vector(EX_COUNT); main_output_variable = std::vector(EX_COUNT); - stores_input_variable_metadata = std::vector(EX_COUNT); + caches_input_variable_metadata = std::vector(EX_COUNT); registration_functions = std::vector(EX_COUNT); uses_forcing_file = std::vector(EX_COUNT); // tries_mass_balance = std::vector(EX_COUNT); @@ -175,7 +175,7 @@ void Bmi_C_Formulation_Test::SetUp() { lib_file[0] = find_file(lib_dir_opts, BMI_TEST_C_LOCAL_LIB_NAME); init_config[0] = find_file(bmi_init_cfg_dir_opts, "test_bmi_c_config_0.txt"); main_output_variable[0] = "OUTPUT_VAR_1"; - stores_input_variable_metadata[0] = false; + caches_input_variable_metadata[0] = false; registration_functions[0] = "register_bmi"; uses_forcing_file[0] = false; // tries_mass_balance[0] = true; @@ -186,7 +186,7 @@ void Bmi_C_Formulation_Test::SetUp() { lib_file[1] = find_file(lib_dir_opts, BMI_TEST_C_LOCAL_LIB_NAME); init_config[1] = find_file(bmi_init_cfg_dir_opts, "test_bmi_c_config_1.txt"); main_output_variable[1] = "OUTPUT_VAR_1"; - stores_input_variable_metadata[1] = false; + caches_input_variable_metadata[1] = false; registration_functions[1] = "register_bmi"; uses_forcing_file[1] = false; // tries_mass_balance[1] = false; @@ -197,7 +197,7 @@ void Bmi_C_Formulation_Test::SetUp() { lib_file[2] = find_file(lib_dir_opts, BMI_TEST_C_LOCAL_LIB_NAME); init_config[2] = find_file(bmi_init_cfg_dir_opts, "test_bmi_c_config_1.txt"); main_output_variable[2] = "OUTPUT_VAR_1"; - stores_input_variable_metadata[2] = true; + caches_input_variable_metadata[2] = true; registration_functions[2] = "register_bmi"; uses_forcing_file[2] = false; // tries_mass_balance[1] = false; @@ -212,10 +212,10 @@ void Bmi_C_Formulation_Test::SetUp() { std::string variables_line = (i == 1) ? variables_with_rain_rate : ""; // Add this substring with this key for even indices or if the value for it is true - std::string store_metadata_substr = ""; - if (i % 2 == 0 || stores_input_variable_metadata[i]) { - store_metadata_substr = "\"" BMI_REALIZATION_CFG_PARAM_OPT__CACHE_INPUT_VAR_METADATA "\": " ; - store_metadata_substr += stores_input_variable_metadata[i] ? "true," : "false,"; + std::string cache_metadata_substr = ""; + if (i % 2 == 0 || caches_input_variable_metadata[i]) { + cache_metadata_substr = "\"" BMI_REALIZATION_CFG_PARAM_OPT__CACHE_INPUT_VAR_METADATA "\": " ; + cache_metadata_substr += caches_input_variable_metadata[i] ? "true," : "false,"; } forcing_params_examples[i] = params; @@ -230,7 +230,7 @@ void Bmi_C_Formulation_Test::SetUp() { " \"main_output_variable\": \"" + main_output_variable[i] + "\"," // Add this predetermined substring from above (which could be empty) - + store_metadata_substr + + + cache_metadata_substr + " \"" + BMI_REALIZATION_CFG_PARAM_OPT__OUTPUT_PRECISION + "\": 6, " " \"" + BMI_REALIZATION_CFG_PARAM_OPT__VAR_STD_NAMES + "\": { " From d0dffa4071642ae7467b5999fa5357fef6a7711e Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Wed, 22 Apr 2026 10:16:24 -0400 Subject: [PATCH 18/28] Fix comment for Formulation_Manager_Test, basic_run_9. --- test/realizations/Formulation_Manager_Test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/realizations/Formulation_Manager_Test.cpp b/test/realizations/Formulation_Manager_Test.cpp index 400cae3061..0399c3a24e 100644 --- a/test/realizations/Formulation_Manager_Test.cpp +++ b/test/realizations/Formulation_Manager_Test.cpp @@ -1215,7 +1215,7 @@ TEST_F(Formulation_Manager_Test, basic_run_3) { /** * Testing config the same as EX 1 (like in basic_run_1) but with `cache_input_variable_metadata` true for global - * formulation config (which is all formulations). + * formulation config (which is not all formulations for that configuration, as there are two independently specified). */ TEST_F(Formulation_Manager_Test, basic_run_9) { std::stringstream stream; From 485d93d36f6251fb138c3a15234f10b26cda10fc Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Wed, 22 Apr 2026 10:21:21 -0400 Subject: [PATCH 19/28] Fix docstring grammar. --- include/realizations/catchment/Bmi_Module_Formulation.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/realizations/catchment/Bmi_Module_Formulation.hpp b/include/realizations/catchment/Bmi_Module_Formulation.hpp index bd007a9f6b..ebcf4a9d96 100644 --- a/include/realizations/catchment/Bmi_Module_Formulation.hpp +++ b/include/realizations/catchment/Bmi_Module_Formulation.hpp @@ -22,7 +22,7 @@ class Bmi_Cpp_Multi_Array_Test; namespace realization { - /** Type to hold some certain details about a BMI module variable that the framework will need used repeatedly. */ + /** Type to hold some certain details about a BMI module variable that the framework will need to use repeatedly. */ class Bmi_Var_Details { public: From e5fd4d76fad89e121ef44a875f7e227ebca97092 Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Wed, 22 Apr 2026 10:53:46 -0400 Subject: [PATCH 20/28] Refactor Bmi_Module_Formulation.hpp. Simplify copy constructor and improve formatting of < overload. --- include/realizations/catchment/Bmi_Module_Formulation.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/realizations/catchment/Bmi_Module_Formulation.hpp b/include/realizations/catchment/Bmi_Module_Formulation.hpp index ebcf4a9d96..a2142cbe3a 100644 --- a/include/realizations/catchment/Bmi_Module_Formulation.hpp +++ b/include/realizations/catchment/Bmi_Module_Formulation.hpp @@ -32,12 +32,12 @@ namespace realization { Bmi_Var_Details(const std::string& name, const std::string& alias, const int item_size, const int num_items, const std::string& cpp_type, const std::string& units) : name(name), mapped_alias(alias), cpp_type(cpp_type), units(units), item_size(item_size), num_items(num_items) { } - Bmi_Var_Details(const Bmi_Var_Details& source) : Bmi_Var_Details(source.name, source.mapped_alias, source.item_size, source.num_items, source.cpp_type, source.units) { } + Bmi_Var_Details(const Bmi_Var_Details& source) = default; friend bool operator<(const Bmi_Var_Details& lhs, const Bmi_Var_Details& rhs) { - return std::tie(lhs.name, lhs.mapped_alias, lhs.cpp_type, lhs.units, lhs.item_size, - lhs.num_items) < std::tie(rhs.name, rhs.mapped_alias, rhs.cpp_type, rhs.units, - rhs.item_size, rhs.num_items); + return std::tie(lhs.name, lhs.mapped_alias, lhs.cpp_type, lhs.units, lhs.item_size, lhs.num_items) + < + std::tie(rhs.name, rhs.mapped_alias, rhs.cpp_type, rhs.units, rhs.item_size, rhs.num_items); } const std::string& get_name() const { From aa5623ddeb321bae6d1ab3b3085342f3a3ab7c84 Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Wed, 22 Apr 2026 10:54:44 -0400 Subject: [PATCH 21/28] Fix more grammar in comments. --- include/realizations/catchment/Bmi_Module_Formulation.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/realizations/catchment/Bmi_Module_Formulation.hpp b/include/realizations/catchment/Bmi_Module_Formulation.hpp index a2142cbe3a..3270498dfe 100644 --- a/include/realizations/catchment/Bmi_Module_Formulation.hpp +++ b/include/realizations/catchment/Bmi_Module_Formulation.hpp @@ -657,7 +657,7 @@ namespace realization { /** * Initialize the metadata as needed for @ref do_bmi_sets_from_stored_metadata. * - * This will populate the @ref bmi_input_var_details member. As metadata is gather into @ref Bmi_Var_Details + * This will populate the @ref bmi_input_var_details member. As metadata is gathered into @ref Bmi_Var_Details * objects, these will be inserted into to the @ref known_bmi_input_vars static member. Pointers to the values * in that set are then added to @ref bmi_input_var_details. */ From f5d14068e3b1933cd7e82c33b87b81aa7e712b0b Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Tue, 7 Jul 2026 13:57:59 -0400 Subject: [PATCH 22/28] Fix and optimize formulation manager tests. Fixed new tests associated with the optimization changes, and moved the boilerplate present (now, after some rebasing) in prior tests but missing from new ones (related to those tests now breaking) into a reusable function. --- .../realizations/Formulation_Manager_Test.cpp | 157 ++++++------------ 1 file changed, 49 insertions(+), 108 deletions(-) diff --git a/test/realizations/Formulation_Manager_Test.cpp b/test/realizations/Formulation_Manager_Test.cpp index 0399c3a24e..c1c8a2ba08 100644 --- a/test/realizations/Formulation_Manager_Test.cpp +++ b/test/realizations/Formulation_Manager_Test.cpp @@ -172,6 +172,32 @@ class Formulation_Manager_Test : public ::testing::Test { return json; } + /** + * Parse a realization config from a stream and build its simulation time parameters. + * + * Reads the JSON in @p stream into @p realization_config (which can then be used to + * construct a @c Formulation_Manager) and derives the simulation time parameters from + * the config's required "time" section. + * + * @param stream Stream holding the (path-fixed) realization config JSON. + * @param realization_config Property tree populated with the parsed config (output parameter). + * @return The simulation time parameters parsed from the config's "time" section. + * @throws std::runtime_error If the config has no "time" section. + */ + simulation_time_params get_time_from_load_realization_config(std::stringstream& stream, + boost::property_tree::ptree& realization_config) + { + boost::property_tree::json_parser::read_json(stream, realization_config); + + boost::optional possible_simulation_time = + realization_config.get_child_optional("time"); + if (!possible_simulation_time) { + throw std::runtime_error("ERROR: No simulation time period defined."); + } + + return realization::config::Time(*possible_simulation_time).make_params(); + } + geojson::GeoJSON fabric = std::make_shared(); }; @@ -1062,14 +1088,7 @@ TEST_F(Formulation_Manager_Test, basic_reading_1) { stream << fix_paths(EXAMPLE_1); boost::property_tree::ptree realization_config; - boost::property_tree::json_parser::read_json(stream, realization_config); - - auto possible_simulation_time = realization_config.get_child_optional("time"); - if (!possible_simulation_time) { - throw std::runtime_error("ERROR: No simulation time period defined."); - } - - auto simulation_time_config = realization::config::Time(*possible_simulation_time).make_params(); + simulation_time_params simulation_time_config = get_time_from_load_realization_config(stream, realization_config); std::ostream* raw_pointer = &std::cout; std::shared_ptr s_ptr(raw_pointer, [](void*) {}); @@ -1095,14 +1114,7 @@ TEST_F(Formulation_Manager_Test, basic_reading_2) { stream << fix_paths(EXAMPLE_2); boost::property_tree::ptree realization_config; - boost::property_tree::json_parser::read_json(stream, realization_config); - - auto possible_simulation_time = realization_config.get_child_optional("time"); - if (!possible_simulation_time) { - throw std::runtime_error("ERROR: No simulation time period defined."); - } - - auto simulation_time_config = realization::config::Time(*possible_simulation_time).make_params(); + simulation_time_params simulation_time_config = get_time_from_load_realization_config(stream, realization_config); std::ostream* raw_pointer = &std::cout; std::shared_ptr s_ptr(raw_pointer, [](void*) {}); @@ -1128,14 +1140,7 @@ TEST_F(Formulation_Manager_Test, basic_run_1) { stream << fix_paths(EXAMPLE_1); boost::property_tree::ptree realization_config; - boost::property_tree::json_parser::read_json(stream, realization_config); - - auto possible_simulation_time = realization_config.get_child_optional("time"); - if (!possible_simulation_time) { - throw std::runtime_error("ERROR: No simulation time period defined."); - } - - auto simulation_time_config = realization::config::Time(*possible_simulation_time).make_params(); + simulation_time_params simulation_time_config = get_time_from_load_realization_config(stream, realization_config); std::ostream* raw_pointer = &std::cout; std::shared_ptr s_ptr(raw_pointer, [](void*) {}); @@ -1173,14 +1178,7 @@ TEST_F(Formulation_Manager_Test, basic_run_3) { stream << fix_paths(EXAMPLE_3); boost::property_tree::ptree realization_config; - boost::property_tree::json_parser::read_json(stream, realization_config); - - auto possible_simulation_time = realization_config.get_child_optional("time"); - if (!possible_simulation_time) { - throw std::runtime_error("ERROR: No simulation time period defined."); - } - - auto simulation_time_config = realization::config::Time(*possible_simulation_time).make_params(); + simulation_time_params simulation_time_config = get_time_from_load_realization_config(stream, realization_config); std::ostream* raw_pointer = &std::cout; std::shared_ptr s_ptr(raw_pointer, [](void*) {}); @@ -1221,15 +1219,18 @@ TEST_F(Formulation_Manager_Test, basic_run_9) { std::stringstream stream; stream << fix_paths(EXAMPLE_9); + boost::property_tree::ptree realization_config; + simulation_time_params simulation_time_config = get_time_from_load_realization_config(stream, realization_config); + std::ostream* raw_pointer = &std::cout; std::shared_ptr s_ptr(raw_pointer, [](void*) {}); utils::StreamHandler catchment_output(s_ptr); - realization::Formulation_Manager manager = realization::Formulation_Manager(stream); + realization::Formulation_Manager manager = realization::Formulation_Manager(realization_config); this->add_feature("cat-52"); this->add_feature("cat-67"); - manager.read(this->fabric, catchment_output); + manager.read(simulation_time_config, this->fabric, catchment_output); ASSERT_EQ(manager.get_size(), 2); @@ -1260,14 +1261,17 @@ TEST_F(Formulation_Manager_Test, basic_run_10) { std::stringstream stream; stream << fix_paths(EXAMPLE_10); + boost::property_tree::ptree realization_config; + simulation_time_params simulation_time_config = get_time_from_load_realization_config(stream, realization_config); + std::ostream* raw_pointer = &std::cout; std::shared_ptr s_ptr(raw_pointer, [](void*) {}); utils::StreamHandler catchment_output(s_ptr); - realization::Formulation_Manager manager = realization::Formulation_Manager(stream); + realization::Formulation_Manager manager = realization::Formulation_Manager(realization_config); this->add_feature("cat-67"); - manager.read(this->fabric, catchment_output); + manager.read(simulation_time_config, this->fabric, catchment_output); ASSERT_EQ(manager.get_size(), 1); ASSERT_TRUE(manager.contains("cat-67")); @@ -1296,14 +1300,7 @@ TEST_F(Formulation_Manager_Test, read_extra) { stream << fix_paths(EXAMPLE_3); boost::property_tree::ptree realization_config; - boost::property_tree::json_parser::read_json(stream, realization_config); - - auto possible_simulation_time = realization_config.get_child_optional("time"); - if (!possible_simulation_time) { - throw std::runtime_error("ERROR: No simulation time period defined."); - } - - auto simulation_time_config = realization::config::Time(*possible_simulation_time).make_params(); + simulation_time_params simulation_time_config = get_time_from_load_realization_config(stream, realization_config); std::ostream* raw_pointer = &std::cout; std::shared_ptr s_ptr(raw_pointer, [](void*) {}); @@ -1325,14 +1322,7 @@ TEST_F(Formulation_Manager_Test, init_config_pattern_match_global) { stream << fix_paths(EXAMPLE_7); boost::property_tree::ptree realization_config; - boost::property_tree::json_parser::read_json(stream, realization_config); - - auto possible_simulation_time = realization_config.get_child_optional("time"); - if (!possible_simulation_time) { - throw std::runtime_error("ERROR: No simulation time period defined."); - } - - auto simulation_time_config = realization::config::Time(*possible_simulation_time).make_params(); + simulation_time_params simulation_time_config = get_time_from_load_realization_config(stream, realization_config); std::ostream* raw_pointer = &std::cout; std::shared_ptr s_ptr(raw_pointer, [](void*) {}); @@ -1354,14 +1344,7 @@ TEST_F(Formulation_Manager_Test, init_config_pattern_match_specific) { stream << fix_paths(EXAMPLE_8); boost::property_tree::ptree realization_config; - boost::property_tree::json_parser::read_json(stream, realization_config); - - auto possible_simulation_time = realization_config.get_child_optional("time"); - if (!possible_simulation_time) { - throw std::runtime_error("ERROR: No simulation time period defined."); - } - - auto simulation_time_config = realization::config::Time(*possible_simulation_time).make_params(); + simulation_time_params simulation_time_config = get_time_from_load_realization_config(stream, realization_config); std::ostream* raw_pointer = &std::cout; std::shared_ptr s_ptr(raw_pointer, [](void*) {}); @@ -1383,14 +1366,7 @@ TEST_F(Formulation_Manager_Test, forcing_provider_specification) { stream << fix_paths(EXAMPLE_4); boost::property_tree::ptree realization_config; - boost::property_tree::json_parser::read_json(stream, realization_config); - - auto possible_simulation_time = realization_config.get_child_optional("time"); - if (!possible_simulation_time) { - throw std::runtime_error("ERROR: No simulation time period defined."); - } - - auto simulation_time_config = realization::config::Time(*possible_simulation_time).make_params(); + simulation_time_params simulation_time_config = get_time_from_load_realization_config(stream, realization_config); std::ostream* raw_pointer = &std::cout; std::shared_ptr s_ptr(raw_pointer, [](void*) {}); @@ -1480,14 +1456,7 @@ TEST_F(Formulation_Manager_Test, read_external_attributes) { }; boost::property_tree::ptree realization_config_a; - boost::property_tree::json_parser::read_json(stream_a, realization_config_a); - - auto possible_simulation_time_a = realization_config_a.get_child_optional("time"); - if (!possible_simulation_time_a) { - throw std::runtime_error("ERROR: No simulation time period defined."); - } - - auto simulation_time_config_a = realization::config::Time(*possible_simulation_time_a).make_params(); + simulation_time_params simulation_time_config_a = get_time_from_load_realization_config(stream_a, realization_config_a); auto manager = realization::Formulation_Manager(realization_config_a); @@ -1518,14 +1487,7 @@ TEST_F(Formulation_Manager_Test, read_external_attributes) { this->fabric->remove_feature_by_id("cat-27115"); boost::property_tree::ptree realization_config_b; - boost::property_tree::json_parser::read_json(stream_b, realization_config_b); - - auto possible_simulation_time_b = realization_config_b.get_child_optional("time"); - if (!possible_simulation_time_b) { - throw std::runtime_error("ERROR: No simulation time period defined."); - } - - auto simulation_time_config_b = realization::config::Time(*possible_simulation_time_b).make_params(); + simulation_time_params simulation_time_config_b = get_time_from_load_realization_config(stream_b, realization_config_b); manager = realization::Formulation_Manager(realization_config_b); @@ -1557,14 +1519,7 @@ TEST_F(Formulation_Manager_Test, test_is_disable_catchment_output_1_a) { stream << fix_paths(EXAMPLE_1); boost::property_tree::ptree realization_config; - boost::property_tree::json_parser::read_json(stream, realization_config); - - auto possible_simulation_time = realization_config.get_child_optional("time"); - if (!possible_simulation_time) { - throw std::runtime_error("ERROR: No simulation time period defined."); - } - - auto simulation_time_config = realization::config::Time(*possible_simulation_time).make_params(); + simulation_time_params simulation_time_config = get_time_from_load_realization_config(stream, realization_config); std::ostream* raw_pointer = &std::cout; std::shared_ptr s_ptr(raw_pointer, [](void*) {}); @@ -1588,14 +1543,7 @@ TEST_F(Formulation_Manager_Test, test_is_disable_catchment_output_2_a) { stream << fix_paths(EXAMPLE_2); boost::property_tree::ptree realization_config; - boost::property_tree::json_parser::read_json(stream, realization_config); - - auto possible_simulation_time = realization_config.get_child_optional("time"); - if (!possible_simulation_time) { - throw std::runtime_error("ERROR: No simulation time period defined."); - } - - auto simulation_time_config = realization::config::Time(*possible_simulation_time).make_params(); + simulation_time_params simulation_time_config = get_time_from_load_realization_config(stream, realization_config); std::ostream* raw_pointer = &std::cout; std::shared_ptr s_ptr(raw_pointer, [](void*) {}); @@ -1619,14 +1567,7 @@ TEST_F(Formulation_Manager_Test, test_is_disable_catchment_output_6_a) { stream << fix_paths(EXAMPLE_6); boost::property_tree::ptree realization_config; - boost::property_tree::json_parser::read_json(stream, realization_config); - - auto possible_simulation_time = realization_config.get_child_optional("time"); - if (!possible_simulation_time) { - throw std::runtime_error("ERROR: No simulation time period defined."); - } - - auto simulation_time_config = realization::config::Time(*possible_simulation_time).make_params(); + simulation_time_params simulation_time_config = get_time_from_load_realization_config(stream, realization_config); std::ostream* raw_pointer = &std::cout; std::shared_ptr s_ptr(raw_pointer, [](void*) {}); From c6c996d666bc3bd7a3e9eb1ceb0ff0462ef81529 Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Tue, 7 Jul 2026 15:21:45 -0400 Subject: [PATCH 23/28] Optimize Bmi_Var_Details constructor. Use by-val and moves rather than const ref to reduce copies, per suggestion from @hellite500. Co-authored-by: Nels --- include/realizations/catchment/Bmi_Module_Formulation.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/realizations/catchment/Bmi_Module_Formulation.hpp b/include/realizations/catchment/Bmi_Module_Formulation.hpp index 3270498dfe..55c8f72f79 100644 --- a/include/realizations/catchment/Bmi_Module_Formulation.hpp +++ b/include/realizations/catchment/Bmi_Module_Formulation.hpp @@ -29,8 +29,8 @@ namespace realization { //Bmi_Var_Details() : Bmi_Var_Details("", "", nullptr, -1, -1, "", "") { } - Bmi_Var_Details(const std::string& name, const std::string& alias, const int item_size, const int num_items, const std::string& cpp_type, const std::string& units) - : name(name), mapped_alias(alias), cpp_type(cpp_type), units(units), item_size(item_size), num_items(num_items) { } + Bmi_Var_Details(std::string name, std::string alias, const int item_size, const int num_items, std::string cpp_type, std::string units) + : name(std::move(name)), mapped_alias(std::move(alias)), cpp_type(std::move(cpp_type)), units(std::move(units)), item_size(item_size), num_items(num_items) { } Bmi_Var_Details(const Bmi_Var_Details& source) = default; From 251051e9e5d554a286986532d43e9d8036cf2a96 Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Tue, 7 Jul 2026 15:23:25 -0400 Subject: [PATCH 24/28] Add test for consistency when caching metadata. Add test that caching input variable metadata yields responses consistent with default execution, where metadata is re-fetched each time. --- .../catchments/Bmi_C_Formulation_Test.cpp | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/test/realizations/catchments/Bmi_C_Formulation_Test.cpp b/test/realizations/catchments/Bmi_C_Formulation_Test.cpp index d6a6d90fa1..b3bec2f817 100644 --- a/test/realizations/catchments/Bmi_C_Formulation_Test.cpp +++ b/test/realizations/catchments/Bmi_C_Formulation_Test.cpp @@ -69,6 +69,10 @@ class Bmi_C_Formulation_Test : public ::testing::Test { return formulation.cache_input_variable_metadata; } + static void set_friend_cache_input_variable_metadata(Bmi_C_Formulation& formulation, bool value) { + formulation.set_cache_input_var_metadata(value); + } + static std::set& get_friend_known_bmi_input_vars(Bmi_C_Formulation& formulation) { return formulation.known_bmi_input_vars; } @@ -553,6 +557,35 @@ TEST_F(Bmi_C_Formulation_Test, bmi_input_var_details_2_c) { } } +/** + * Test that caching input variable metadata yields responses identical to the default refetch path. + * + * Two formulations are built from the same example 2 config, differing only in whether + * `cache_input_variable_metadata` is enabled. This guards against behavioral divergence between the two + * execution paths of `set_model_inputs_prior_to_update` (i.e., `do_bmi_sets_from_stored_metadata` versus + * `do_bmi_sets_with_full_refetch`), including any unintended side effect from how `Bmi_Var_Details` + * instances are constructed in either path. + */ +TEST_F(Bmi_C_Formulation_Test, cache_matches_refetch_2_a) { + int ex_index = 2; + + // Cached formulation directly from the example 2 config (which enables caching). + Bmi_C_Formulation cached(catchment_ids[ex_index], std::make_shared(*forcing_params_examples[ex_index]), utils::StreamHandler()); + cached.create_formulation(config_prop_ptree[ex_index]); + ASSERT_TRUE(get_friend_cache_input_variable_metadata(cached)); + + // Same config, but with caching disabled so the full-refetch path is exercised instead. The flag is + // overridden before the first response, since metadata is initialized lazily on that first call. + Bmi_C_Formulation refetch(catchment_ids[ex_index], std::make_shared(*forcing_params_examples[ex_index]), utils::StreamHandler()); + refetch.create_formulation(config_prop_ptree[ex_index]); + set_friend_cache_input_variable_metadata(refetch, false); + ASSERT_FALSE(get_friend_cache_input_variable_metadata(refetch)); + + for (int i = 0; i < 39; i++) { + ASSERT_EQ(cached.get_response(i, 3600), refetch.get_response(i, 3600)); + } +} + /** Simple test of output. */ TEST_F(Bmi_C_Formulation_Test, GetOutputLineForTimestep_0_a) { int ex_index = 0; From c29a87aef8815466637917233c7a4575b5190560 Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Wed, 8 Jul 2026 11:03:51 -0400 Subject: [PATCH 25/28] Add public getter BMI metadata cache flag. --- .../realizations/catchment/Bmi_Module_Formulation.hpp | 9 +++++++++ src/realizations/catchment/Bmi_Module_Formulation.cpp | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/include/realizations/catchment/Bmi_Module_Formulation.hpp b/include/realizations/catchment/Bmi_Module_Formulation.hpp index 55c8f72f79..8b7eca5965 100644 --- a/include/realizations/catchment/Bmi_Module_Formulation.hpp +++ b/include/realizations/catchment/Bmi_Module_Formulation.hpp @@ -307,6 +307,15 @@ namespace realization { bool is_bmi_input_variable(const std::string &var_name) const override; bool is_bmi_output_variable(const std::string &var_name) const override; + /** + * Test whether @ref set_model_inputs_prior_to_update caches and reuses input variable metadata. + * + * See @ref cache_input_variable_metadata and @ref set_cache_input_var_metadata for details. + * + * @return Whether input variable metadata is cached and reused across time steps, rather than re-fetched. + */ + bool is_input_variable_metadata_cached() const; + /** * Get whether a property's per-time-step values are each an aggregate sum over the entire time step. * diff --git a/src/realizations/catchment/Bmi_Module_Formulation.cpp b/src/realizations/catchment/Bmi_Module_Formulation.cpp index 8c42f48b23..8834dbb4bf 100644 --- a/src/realizations/catchment/Bmi_Module_Formulation.cpp +++ b/src/realizations/catchment/Bmi_Module_Formulation.cpp @@ -601,6 +601,10 @@ namespace realization { return model_initialized; } + bool Bmi_Module_Formulation::is_input_variable_metadata_cached() const { + return cache_input_variable_metadata; + } + void Bmi_Module_Formulation::set_allow_model_exceed_end_time(bool allow_exceed_end) { allow_model_exceed_end_time = allow_exceed_end; } From 407b7c72f03df28df0b1894014553d14750c5bb3 Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Wed, 8 Jul 2026 11:06:18 -0400 Subject: [PATCH 26/28] Add more tests for BMI metadata cache flag. More tests to make sure Formulation_Manager behaves properly in various situations and gets formulations with appropriate cache flag setting for config. --- .../realizations/Formulation_Manager_Test.cpp | 176 ++++++++++++++++++ 1 file changed, 176 insertions(+) diff --git a/test/realizations/Formulation_Manager_Test.cpp b/test/realizations/Formulation_Manager_Test.cpp index c1c8a2ba08..42f4b892f5 100644 --- a/test/realizations/Formulation_Manager_Test.cpp +++ b/test/realizations/Formulation_Manager_Test.cpp @@ -1082,6 +1082,43 @@ const std::string EXAMPLE_10 = "{ " "} " "}"; +// Like EXAMPLE_9, but with no catchment-specific formulations, so added features fall through to the global +// formulation and thus inherit its `cache_input_variable_metadata` value of `true`. +const std::string EXAMPLE_11 = "{ " + "\"global\": { " + "\"formulations\": [ " + "{" + "\"name\":\"bmi_c++\"," + "\"params\": {" + "\"model_type_name\": \"test_bmi_cpp\"," + "\"library_file\": \"{{EXTERN_LIB_DIR_PATH}}" BMI_TEST_CPP_LIB_NAME "\"," + "\"init_config\": \"{{BMI_C_INIT_DIR_PATH}}/test_bmi_c_config_0.txt\"," + "\"main_output_variable\": \"OUTPUT_VAR_2\"," + "\"cache_input_variable_metadata\": true," + "\"" BMI_REALIZATION_CFG_PARAM_OPT__VAR_STD_NAMES "\": { " + "\"INPUT_VAR_2\": \"" AORC_FIELD_NAME_TEMP_2M_AG "\"," + "\"INPUT_VAR_1\": \"" AORC_FIELD_NAME_PRECIP_RATE "\"" + "}," + "\"create_function\": \"bmi_model_create\"," + "\"destroy_function\": \"bmi_model_destroy\"," + "\"uses_forcing_file\": false" + "} " + "} " + "], " + "\"forcing\": { " + "\"file_pattern\": \".*{{id}}.*.csv\", " + "\"path\": \"./data/forcing/\", " + "\"provider\": \"CsvPerFeature\" " + "} " + "}, " + "\"time\": { " + "\"start_time\": \"2015-12-01 00:00:00\", " + "\"end_time\": \"2015-12-30 23:00:00\", " + "\"output_interval\": 3600 " + "}, " + "\"disable_catchment_output\": true " +"}"; + TEST_F(Formulation_Manager_Test, basic_reading_1) { std::stringstream stream; @@ -1295,6 +1332,145 @@ TEST_F(Formulation_Manager_Test, basic_run_10) { } } +/** + * Verify BMI input variable metadata caching is disabled by default when nothing configures it (EX 1). + * + * EXAMPLE_1 sets `cache_input_variable_metadata` nowhere (neither globally nor for either catchment's own + * formulation), so both catchments should resolve to the default of `false`. + */ +TEST_F(Formulation_Manager_Test, cache_bmi_var_metadata_1) { + std::stringstream stream; + stream << fix_paths(EXAMPLE_1); + + boost::property_tree::ptree realization_config; + simulation_time_params simulation_time_config = get_time_from_load_realization_config(stream, realization_config); + + std::ostream* raw_pointer = &std::cout; + std::shared_ptr s_ptr(raw_pointer, [](void*) {}); + utils::StreamHandler catchment_output(s_ptr); + + realization::Formulation_Manager manager = realization::Formulation_Manager(realization_config); + + this->add_feature("cat-52"); + this->add_feature("cat-67"); + manager.read(simulation_time_config, this->fabric, catchment_output); + + ASSERT_EQ(manager.get_size(), 2); + + for (const std::pair& expected : {std::make_pair(std::string("cat-52"), false), + std::make_pair(std::string("cat-67"), false)}) { + std::shared_ptr mod = + std::dynamic_pointer_cast(manager.get_formulation(expected.first)); + ASSERT_NE(mod, nullptr) << expected.first << " should be a BMI module formulation"; + ASSERT_EQ(mod->is_input_variable_metadata_cached(), expected.second) << expected.first; + } +} + +/** + * Verify a global `cache_input_variable_metadata` of `true` is not inherited by catchments with their own + * formulations (EX 9). + * + * EXAMPLE_1 config but with `cache_input_variable_metadata` `true` for the global formulation only. Because both + * catchments independently specify their own formulations (which omit the option), neither inherits the global + * value, so both should resolve to the default of `false`. + */ +TEST_F(Formulation_Manager_Test, cache_bmi_var_metadata_9) { + std::stringstream stream; + stream << fix_paths(EXAMPLE_9); + + boost::property_tree::ptree realization_config; + simulation_time_params simulation_time_config = get_time_from_load_realization_config(stream, realization_config); + + std::ostream* raw_pointer = &std::cout; + std::shared_ptr s_ptr(raw_pointer, [](void*) {}); + utils::StreamHandler catchment_output(s_ptr); + + realization::Formulation_Manager manager = realization::Formulation_Manager(realization_config); + + this->add_feature("cat-52"); + this->add_feature("cat-67"); + manager.read(simulation_time_config, this->fabric, catchment_output); + + ASSERT_EQ(manager.get_size(), 2); + + for (const std::pair& expected : {std::make_pair(std::string("cat-52"), false), + std::make_pair(std::string("cat-67"), false)}) { + std::shared_ptr mod = + std::dynamic_pointer_cast(manager.get_formulation(expected.first)); + ASSERT_NE(mod, nullptr) << expected.first << " should be a BMI module formulation"; + ASSERT_EQ(mod->is_input_variable_metadata_cached(), expected.second) << expected.first; + } +} + +/** + * Verify `cache_input_variable_metadata` resolves independently per catchment formulation (EX 10). + * + * EXAMPLE_10 enables the option globally and for cat-52's own formulation, but not for cat-67's own formulation. + * So cat-52 should resolve to `true` and cat-67 to `false`, demonstrating the option is honored per catchment. + */ +TEST_F(Formulation_Manager_Test, cache_bmi_var_metadata_10) { + std::stringstream stream; + stream << fix_paths(EXAMPLE_10); + + boost::property_tree::ptree realization_config; + simulation_time_params simulation_time_config = get_time_from_load_realization_config(stream, realization_config); + + std::ostream* raw_pointer = &std::cout; + std::shared_ptr s_ptr(raw_pointer, [](void*) {}); + utils::StreamHandler catchment_output(s_ptr); + + realization::Formulation_Manager manager = realization::Formulation_Manager(realization_config); + + this->add_feature("cat-52"); + this->add_feature("cat-67"); + manager.read(simulation_time_config, this->fabric, catchment_output); + + ASSERT_EQ(manager.get_size(), 2); + + for (const std::pair& expected : {std::make_pair(std::string("cat-52"), true), + std::make_pair(std::string("cat-67"), false)}) { + std::shared_ptr mod = + std::dynamic_pointer_cast(manager.get_formulation(expected.first)); + ASSERT_NE(mod, nullptr) << expected.first << " should be a BMI module formulation"; + ASSERT_EQ(mod->is_input_variable_metadata_cached(), expected.second) << expected.first; + } +} + +/** + * Verify a global `cache_input_variable_metadata` of `true` is inherited by catchments without their own + * formulations (EX 11). + * + * EXAMPLE_11 enables the option only on the global formulation and specifies no catchment formulations. Both + * added features therefore fall through to the global formulation and should inherit its value of `true`. + */ +TEST_F(Formulation_Manager_Test, cache_bmi_var_metadata_11) { + std::stringstream stream; + stream << fix_paths(EXAMPLE_11); + + boost::property_tree::ptree realization_config; + simulation_time_params simulation_time_config = get_time_from_load_realization_config(stream, realization_config); + + std::ostream* raw_pointer = &std::cout; + std::shared_ptr s_ptr(raw_pointer, [](void*) {}); + utils::StreamHandler catchment_output(s_ptr); + + realization::Formulation_Manager manager = realization::Formulation_Manager(realization_config); + + this->add_feature("cat-52"); + this->add_feature("cat-67"); + manager.read(simulation_time_config, this->fabric, catchment_output); + + ASSERT_EQ(manager.get_size(), 2); + + for (const std::pair& expected : {std::make_pair(std::string("cat-52"), true), + std::make_pair(std::string("cat-67"), true)}) { + std::shared_ptr mod = + std::dynamic_pointer_cast(manager.get_formulation(expected.first)); + ASSERT_NE(mod, nullptr) << expected.first << " should be a BMI module formulation"; + ASSERT_EQ(mod->is_input_variable_metadata_cached(), expected.second) << expected.first; + } +} + TEST_F(Formulation_Manager_Test, read_extra) { std::stringstream stream; stream << fix_paths(EXAMPLE_3); From 216518208775ec137a01ef49d922b67c31f4c347 Mon Sep 17 00:00:00 2001 From: Robert Bartel Date: Tue, 14 Jul 2026 15:34:33 -0400 Subject: [PATCH 27/28] Clarify in docs option for nested modules. --- doc/BMI_MODELS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/BMI_MODELS.md b/doc/BMI_MODELS.md index 7a7f31aec0..dcf5e1efd4 100644 --- a/doc/BMI_MODELS.md +++ b/doc/BMI_MODELS.md @@ -196,6 +196,7 @@ There are some special BMI formulation config parameters which are required in c * the BMI specification does not expressly guarantee metadata values will not change, so configuration must consider the particular BMI module in use * also, these optimizations do result in additional memory usage * while relatively small, can scale significantly for larger simulations, so this may not be usable for situations when memory is constrained + * for a multi-BMI formulation, this option is configured on the individual nested modules (i.e., within each nested module's `params`) rather than at the top level of the multi-BMI formulation, so it can be enabled or disabled per nested module ## BMI Models Written in C From 7f7fd4ab3d515dee98b7eab9dfaabe4b07acc93a Mon Sep 17 00:00:00 2001 From: Josh Cunningham Date: Fri, 7 Aug 2026 14:30:05 -0500 Subject: [PATCH 28/28] Implement variable size caching in iso_c_bmi.f90 Added caching for variable sizes to optimize lookups and reduce model queries. Introduced var_size_t type and modified functions to utilize the cache. --- extern/iso_c_fortran_bmi/src/iso_c_bmi.f90 | 137 +++++++++++++++++---- 1 file changed, 110 insertions(+), 27 deletions(-) diff --git a/extern/iso_c_fortran_bmi/src/iso_c_bmi.f90 b/extern/iso_c_fortran_bmi/src/iso_c_bmi.f90 index bff7f28463..f958041a5c 100644 --- a/extern/iso_c_fortran_bmi/src/iso_c_bmi.f90 +++ b/extern/iso_c_fortran_bmi/src/iso_c_bmi.f90 @@ -14,8 +14,21 @@ module iso_c_bmif_2_0 use, intrinsic :: iso_c_binding, only: c_ptr, c_loc, c_f_pointer, c_char, c_null_char, c_int, c_double, c_float, c_null_ptr implicit none + ! Cached itemsize/nbytes for one variable, populated once after initialize() so + ! get_value_*/set_value_* don't need to re-query the model every timestep. + ! name_len is the true (trimmed) length of name, computed once at cache-build + ! time, so lookups can bound their character comparison instead of scanning + ! the full BMI_MAX_VAR_NAME-sized buffer via trim()/compare_string every call. + type var_size_t + character(len=BMI_MAX_VAR_NAME) :: name = '' + integer :: name_len = 0 + integer :: nbytes = 0 + integer :: item_size = 0 + end type var_size_t + type box class(bmi), pointer :: ptr => null() + type(var_size_t), allocatable :: size_cache(:) end type contains @@ -54,6 +67,89 @@ pure function f_to_c_string(f_string) result(c_string) c_string(n+1) = c_null_char !make sure to add null terminator end function f_to_c_string + ! Query the model once for every input/output variable's itemsize and nbytes and + ! stash them in bmi_box%size_cache. Called right after a successful initialize(). + ! Best-effort: if any step fails, size_cache is left unallocated and callers fall + ! back to live model queries via lookup_var_size. + subroutine populate_size_cache(bmi_box) + type(box), intent(inout) :: bmi_box + character(len=BMI_MAX_VAR_NAME), pointer :: in_names(:), out_names(:) + integer :: bmi_status, n_in, n_out, i + + bmi_status = bmi_box%ptr%get_input_item_count(n_in) + if (bmi_status .ne. BMI_SUCCESS) return + bmi_status = bmi_box%ptr%get_output_item_count(n_out) + if (bmi_status .ne. BMI_SUCCESS) return + if (n_in + n_out == 0) return + + if (n_in > 0) then + bmi_status = bmi_box%ptr%get_input_var_names(in_names) + if (bmi_status .ne. BMI_SUCCESS) return + end if + if (n_out > 0) then + bmi_status = bmi_box%ptr%get_output_var_names(out_names) + if (bmi_status .ne. BMI_SUCCESS) return + end if + + allocate(bmi_box%size_cache(n_in + n_out)) + + do i = 1, n_in + bmi_box%size_cache(i)%name = trim(in_names(i)) + bmi_box%size_cache(i)%name_len = len_trim(in_names(i)) + bmi_status = bmi_box%ptr%get_var_nbytes(trim(in_names(i)), bmi_box%size_cache(i)%nbytes) + bmi_status = bmi_box%ptr%get_var_itemsize(trim(in_names(i)), bmi_box%size_cache(i)%item_size) + end do + + do i = 1, n_out + bmi_box%size_cache(n_in + i)%name = trim(out_names(i)) + bmi_box%size_cache(n_in + i)%name_len = len_trim(out_names(i)) + bmi_status = bmi_box%ptr%get_var_nbytes(trim(out_names(i)), bmi_box%size_cache(n_in + i)%nbytes) + bmi_status = bmi_box%ptr%get_var_itemsize(trim(out_names(i)), bmi_box%size_cache(n_in + i)%item_size) + end do + end subroutine populate_size_cache + + ! Look up the cached (nbytes, item_size) pair for `name`. Falls back to a live + ! model query if the cache wasn't populated or doesn't contain this variable + ! (e.g. a model with a dynamic grid that changes size after initialize()). + ! + ! Compares names by hand, bounded by the cached name_len, instead of + ! trim()/`==` on the BMI_MAX_VAR_NAME-sized buffer: trim() has to scan + ! backwards from position BMI_MAX_VAR_NAME to find the last non-blank + ! character on every call, which dominated runtime once this ran every + ! timestep for every variable. + function lookup_var_size(bmi_box, name, num_bytes, item_size) result(bmi_status) + type(box), intent(in) :: bmi_box + character(len=*), intent(in) :: name + integer, intent(out) :: num_bytes, item_size + integer :: bmi_status + integer :: i, j, n + logical :: is_match + + if (allocated(bmi_box%size_cache)) then + n = len(name) + do i = 1, size(bmi_box%size_cache) + if (bmi_box%size_cache(i)%name_len /= n) cycle + is_match = .true. + do j = 1, n + if (bmi_box%size_cache(i)%name(j:j) /= name(j:j)) then + is_match = .false. + exit + end if + end do + if (is_match) then + num_bytes = bmi_box%size_cache(i)%nbytes + item_size = bmi_box%size_cache(i)%item_size + bmi_status = BMI_SUCCESS + return + end if + end do + end if + + bmi_status = bmi_box%ptr%get_var_nbytes(name, num_bytes) + if (bmi_status .ne. BMI_SUCCESS) return + bmi_status = bmi_box%ptr%get_var_itemsize(name, item_size) + end function lookup_var_size + ! Perform startup tasks for the model. function initialize(this, config_file) result(bmi_status) bind(C, name="initialize") type(c_ptr) :: this @@ -69,6 +165,9 @@ function initialize(this, config_file) result(bmi_status) bind(C, name="initiali f_file = c_to_f_string(config_file) bmi_status = bmi_box%ptr%initialize(f_file) deallocate(f_file) + if (bmi_status == BMI_SUCCESS) then + call populate_size_cache(bmi_box) + end if end function initialize ! Advance the model one time step. @@ -394,9 +493,7 @@ function get_value_int(this, name, dest) result(bmi_status) bind(C, name="get_va f_str = c_to_f_string(name) ! Use variable metadata to determine the size of the array required to ! hold the variable. - bmi_status = bmi_box%ptr%get_var_nbytes(f_str, num_bytes) - if( bmi_status .ne. BMI_SUCCESS ) return - bmi_status = bmi_box%ptr%get_var_itemsize(f_str, item_size) + bmi_status = lookup_var_size(bmi_box, f_str, num_bytes, item_size) if( bmi_status .ne. BMI_SUCCESS ) return if( item_size .eq. 0 ) then ! cannot get a value no size, fail @@ -426,9 +523,7 @@ function get_value_float(this, name, dest) result(bmi_status) bind(C, name="get_ f_str = c_to_f_string(name) ! Use variable metadata to determine the size of the array required to ! hold the variable. - bmi_status = bmi_box%ptr%get_var_nbytes(f_str, num_bytes) - if( bmi_status .ne. BMI_SUCCESS ) return - bmi_status = bmi_box%ptr%get_var_itemsize(f_str, item_size) + bmi_status = lookup_var_size(bmi_box, f_str, num_bytes, item_size) if( bmi_status .ne. BMI_SUCCESS ) return if( item_size .eq. 0 ) then ! cannot get a value no size, fail @@ -458,17 +553,15 @@ function get_value_double(this, name, dest) result(bmi_status) bind(C, name="get f_str = c_to_f_string(name) ! Use variable metadata to determine the size of the array required to ! hold the variable. - bmi_status = bmi_box%ptr%get_var_nbytes(f_str, num_bytes) - if( bmi_status .ne. BMI_SUCCESS ) return - bmi_status = bmi_box%ptr%get_var_itemsize(f_str, item_size) + bmi_status = lookup_var_size(bmi_box, f_str, num_bytes, item_size) if( bmi_status .ne. BMI_SUCCESS ) return - num_items = num_bytes/item_size if( item_size .eq. 0 ) then ! cannot get a value no size, fail ! also prevents divide by 0 below bmi_status = BMI_FAILURE return endif + num_items = num_bytes/item_size bmi_status = bmi_box%ptr%get_value_double(f_str, dest(:num_items)) deallocate(f_str) end function get_value_double @@ -556,15 +649,13 @@ function set_value_int(this, name, src) result(bmi_status) bind(C, name="set_val f_str = c_to_f_string(name) ! Use variable metadata to determine the size of the array required to ! hold the variable. - bmi_status = bmi_box%ptr%get_var_nbytes(f_str, num_bytes) - if( bmi_status .ne. BMI_SUCCESS ) return - bmi_status = bmi_box%ptr%get_var_itemsize(f_str, item_size) + bmi_status = lookup_var_size(bmi_box, f_str, num_bytes, item_size) if( bmi_status .ne. BMI_SUCCESS ) return if( item_size .eq. 0 ) then ! we can attempt to set a value of 0 size ! but we need to avoid divide by 0 num_items = 0 - else + else num_items = num_bytes/item_size endif !write(0,*) "set_value_int, grid_size: ", num_items @@ -590,15 +681,13 @@ function set_value_float(this, name, src) result(bmi_status) bind(C, name="set_v f_str = c_to_f_string(name) ! Use variable metadata to determine the size of the array required to ! hold the variable. - bmi_status = bmi_box%ptr%get_var_nbytes(f_str, num_bytes) - if( bmi_status .ne. BMI_SUCCESS ) return - bmi_status = bmi_box%ptr%get_var_itemsize(f_str, item_size) + bmi_status = lookup_var_size(bmi_box, f_str, num_bytes, item_size) if( bmi_status .ne. BMI_SUCCESS ) return if( item_size .eq. 0 ) then ! we can attempt to set a value of 0 size ! but we need to avoid divide by 0 num_items = 0 - else + else num_items = num_bytes/item_size endif bmi_status = bmi_box%ptr%set_value_float(f_str, src(:num_items)) @@ -623,23 +712,17 @@ function set_value_double(this, name, src) result(bmi_status) bind(C, name="set_ f_str = c_to_f_string(name) ! Use variable metadata to determine the size of the array required to ! hold the variable. - bmi_status = bmi_box%ptr%get_var_nbytes(f_str, num_bytes) - if( bmi_status .ne. BMI_SUCCESS ) then - ! TODO make this write unit configurable??? - write(0,*) "Failed to get var nbytes: ", f_str - return - end if - bmi_status = bmi_box%ptr%get_var_itemsize(f_str, item_size) + bmi_status = lookup_var_size(bmi_box, f_str, num_bytes, item_size) if( bmi_status .ne. BMI_SUCCESS ) then ! TODO make this write unit configurable??? - write(0,*) "Failed to get var itemsize: ", f_str + write(0,*) "Failed to get var nbytes/itemsize: ", f_str return end if if( item_size .eq. 0 ) then ! we can attempt to set a value of 0 size ! but we need to avoid divide by 0 num_items = 0 - else + else num_items = num_bytes/item_size endif bmi_status = bmi_box%ptr%set_value_double(f_str, src(:num_items))