From c0cf707b3d132de1f95609c16d48604c3dc6f230 Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Wed, 20 Sep 2023 11:18:10 -0700 Subject: [PATCH 01/11] Replace written-out default copy/move constructors with compiler-generated --- include/bmi/Bmi_Adapter.hpp | 42 ++----------------------------------- 1 file changed, 2 insertions(+), 40 deletions(-) diff --git a/include/bmi/Bmi_Adapter.hpp b/include/bmi/Bmi_Adapter.hpp index 640ef34d5f..ff5ab4bb58 100644 --- a/include/bmi/Bmi_Adapter.hpp +++ b/include/bmi/Bmi_Adapter.hpp @@ -40,46 +40,8 @@ namespace models { } } - /** - * Copy constructor. - * - * @param adapter Base adapter instance to copy. - */ - Bmi_Adapter(Bmi_Adapter &adapter) - : allow_model_exceed_end_time(adapter.allow_model_exceed_end_time), - bmi_init_config(adapter.bmi_init_config), - bmi_model(adapter.bmi_model), - bmi_model_has_fixed_time_step(adapter.bmi_model_has_fixed_time_step), - bmi_model_time_convert_factor(adapter.bmi_model_time_convert_factor), - bmi_model_time_step_size(adapter.bmi_model_time_step_size), - bmi_model_uses_forcing_file(adapter.bmi_model_uses_forcing_file), - forcing_file_path(adapter.forcing_file_path), - init_exception_msg(adapter.init_exception_msg), input_var_names(adapter.input_var_names), - model_initialized(adapter.model_initialized), model_name(adapter.model_name), - output(adapter.output), - output_var_names(adapter.output_var_names) {} - - /** - * Move constructor. - * - * @param adapter Base adapter instance to copy. - */ - Bmi_Adapter(Bmi_Adapter &&adapter) - : allow_model_exceed_end_time(std::move(adapter.allow_model_exceed_end_time)), - bmi_init_config(std::move(adapter.bmi_init_config)), - bmi_model(std::move(adapter.bmi_model)), - bmi_model_has_fixed_time_step(std::move(adapter.bmi_model_has_fixed_time_step)), - bmi_model_time_convert_factor(std::move(adapter.bmi_model_time_convert_factor)), - bmi_model_time_step_size(std::move(adapter.bmi_model_time_step_size)), - bmi_model_uses_forcing_file(std::move(adapter.bmi_model_uses_forcing_file)), - forcing_file_path(std::move(adapter.forcing_file_path)), - init_exception_msg(std::move(adapter.init_exception_msg)), - input_var_names(std::move(adapter.input_var_names)), - model_initialized(std::move(adapter.model_initialized)), - model_name(std::move(adapter.model_name)), - output(std::move(adapter.output)), - output_var_names(std::move(adapter.output_var_names)) {} - + Bmi_Adapter(Bmi_Adapter &) = default; + Bmi_Adapter(Bmi_Adapter &&) = default; /** * Determine backing model's time units and return an appropriate conversion factor. From b92feb49f0deec705b25fb614bc8775b8045c587 Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Wed, 20 Sep 2023 11:18:42 -0700 Subject: [PATCH 02/11] Bugfix: correct mis-typed copy constructor argument --- include/bmi/Bmi_Adapter.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/bmi/Bmi_Adapter.hpp b/include/bmi/Bmi_Adapter.hpp index ff5ab4bb58..ba12322443 100644 --- a/include/bmi/Bmi_Adapter.hpp +++ b/include/bmi/Bmi_Adapter.hpp @@ -40,7 +40,7 @@ namespace models { } } - Bmi_Adapter(Bmi_Adapter &) = default; + Bmi_Adapter(Bmi_Adapter const&) = default; Bmi_Adapter(Bmi_Adapter &&) = default; /** From 210cb06e8979c207e799629d9e3d142e4cbcb306 Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Wed, 20 Sep 2023 11:34:48 -0700 Subject: [PATCH 03/11] Refactor: Move bmi_model pointer down to obviate template usage --- include/bmi/AbstractCLibBmiAdapter.hpp | 5 +++++ include/bmi/Bmi_Adapter.hpp | 2 -- include/bmi/Bmi_Py_Adapter.hpp | 5 +++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/include/bmi/AbstractCLibBmiAdapter.hpp b/include/bmi/AbstractCLibBmiAdapter.hpp index 0eea6ed911..fc620466e4 100644 --- a/include/bmi/AbstractCLibBmiAdapter.hpp +++ b/include/bmi/AbstractCLibBmiAdapter.hpp @@ -36,6 +36,7 @@ namespace models { AbstractCLibBmiAdapter(AbstractCLibBmiAdapter &&adapter) noexcept : Bmi_Adapter(std::move(adapter)), + bmi_model(std::move(adapter.bmi_model)), bmi_lib_file(std::move(adapter.bmi_lib_file)), bmi_registration_function(adapter.bmi_registration_function), dyn_lib_handle(adapter.dyn_lib_handle) @@ -205,6 +206,10 @@ namespace models { return dyn_lib_handle; } + protected: + /** Pointer to backing BMI model instance. */ + std::shared_ptr bmi_model = nullptr; + private: /** Path to the BMI shared library file, for dynamic linking. */ diff --git a/include/bmi/Bmi_Adapter.hpp b/include/bmi/Bmi_Adapter.hpp index ba12322443..0f06e67728 100644 --- a/include/bmi/Bmi_Adapter.hpp +++ b/include/bmi/Bmi_Adapter.hpp @@ -221,8 +221,6 @@ namespace models { bool allow_model_exceed_end_time = false; /** Path (as a string) to the BMI config file for initializing the backing model (empty if none). */ std::string bmi_init_config; - /** Pointer to backing BMI model instance. */ - std::shared_ptr bmi_model = nullptr; /** Whether this particular model has a time step size that cannot be changed internally or externally. */ bool bmi_model_has_fixed_time_step = true; /** Conversion factor for converting values for model time in model's unit type to equivalent in seconds. */ diff --git a/include/bmi/Bmi_Py_Adapter.hpp b/include/bmi/Bmi_Py_Adapter.hpp index 1cd91ab5ba..c25a8e41fe 100644 --- a/include/bmi/Bmi_Py_Adapter.hpp +++ b/include/bmi/Bmi_Py_Adapter.hpp @@ -767,6 +767,11 @@ namespace models { // For unit testing friend class ::Bmi_Py_Adapter_Test; + + protected: + /** Pointer to backing BMI model instance. */ + std::shared_ptr bmi_model = nullptr; + }; } From 47e046664c537d597185818939f6b62d4b4b34cc Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Wed, 20 Sep 2023 12:23:36 -0700 Subject: [PATCH 04/11] Refactor: de-template class Bmi_Adapter --- include/bmi/AbstractCLibBmiAdapter.hpp | 6 +++--- include/bmi/Bmi_Adapter.hpp | 1 - include/bmi/Bmi_Py_Adapter.hpp | 2 +- include/utilities/bmi_utilities.hpp | 4 ++-- src/bmi/Bmi_Py_Adapter.cpp | 2 +- 5 files changed, 7 insertions(+), 8 deletions(-) diff --git a/include/bmi/AbstractCLibBmiAdapter.hpp b/include/bmi/AbstractCLibBmiAdapter.hpp index fc620466e4..8a7c01cbca 100644 --- a/include/bmi/AbstractCLibBmiAdapter.hpp +++ b/include/bmi/AbstractCLibBmiAdapter.hpp @@ -10,7 +10,7 @@ namespace models { namespace bmi { template - class AbstractCLibBmiAdapter : public Bmi_Adapter { + class AbstractCLibBmiAdapter : public Bmi_Adapter { public: /** @@ -29,13 +29,13 @@ namespace models { AbstractCLibBmiAdapter(const std::string &type_name, std::string library_file_path, std::string bmi_init_config, std::string forcing_file_path, bool allow_exceed_end, bool has_fixed_time_step, std::string registration_func, utils::StreamHandler output) - : Bmi_Adapter(type_name, std::move(bmi_init_config), std::move(forcing_file_path), + : Bmi_Adapter(type_name, std::move(bmi_init_config), std::move(forcing_file_path), allow_exceed_end, has_fixed_time_step, output), bmi_lib_file(std::move(library_file_path)), bmi_registration_function(std::move(registration_func)) { } AbstractCLibBmiAdapter(AbstractCLibBmiAdapter &&adapter) noexcept : - Bmi_Adapter(std::move(adapter)), + Bmi_Adapter(std::move(adapter)), bmi_model(std::move(adapter.bmi_model)), bmi_lib_file(std::move(adapter.bmi_lib_file)), bmi_registration_function(adapter.bmi_registration_function), diff --git a/include/bmi/Bmi_Adapter.hpp b/include/bmi/Bmi_Adapter.hpp index 0f06e67728..babffbd151 100644 --- a/include/bmi/Bmi_Adapter.hpp +++ b/include/bmi/Bmi_Adapter.hpp @@ -16,7 +16,6 @@ namespace models { * Abstract adapter interface for C++ classes to interact with the essential aspects of external models that * implement the BMI spec but that are written in some other programming language. */ - template class Bmi_Adapter : public ::bmi::Bmi { public: diff --git a/include/bmi/Bmi_Py_Adapter.hpp b/include/bmi/Bmi_Py_Adapter.hpp index c25a8e41fe..b69eb9f2ea 100644 --- a/include/bmi/Bmi_Py_Adapter.hpp +++ b/include/bmi/Bmi_Py_Adapter.hpp @@ -30,7 +30,7 @@ namespace models { * An adapter class to serve as a C++ interface to the aspects of external models written in the Python * language that implement the BMI. */ - class Bmi_Py_Adapter : public Bmi_Adapter { + class Bmi_Py_Adapter : public Bmi_Adapter { public: diff --git a/include/utilities/bmi_utilities.hpp b/include/utilities/bmi_utilities.hpp index 300061c06e..1261316f4a 100644 --- a/include/utilities/bmi_utilities.hpp +++ b/include/utilities/bmi_utilities.hpp @@ -37,8 +37,8 @@ namespace models { * @param name Bmi variable name to query the model for * @return std::vector Copy of data from the BMI model for variable @param name */ - template - std::vector GetValue(Bmi_Adapter& model, const std::string& name) { + template + std::vector GetValue(Bmi_Adapter& model, const std::string& name) { //TODO make model const ref int total_mem = model.GetVarNbytes(name); int item_size = model.GetVarItemsize(name); diff --git a/src/bmi/Bmi_Py_Adapter.cpp b/src/bmi/Bmi_Py_Adapter.cpp index 1947b4f407..2ad4fb0013 100644 --- a/src/bmi/Bmi_Py_Adapter.cpp +++ b/src/bmi/Bmi_Py_Adapter.cpp @@ -16,7 +16,7 @@ Bmi_Py_Adapter::Bmi_Py_Adapter(const std::string &type_name, std::string bmi_ini Bmi_Py_Adapter::Bmi_Py_Adapter(const std::string &type_name, std::string bmi_init_config, const std::string &bmi_python_type, std::string forcing_file_path, bool allow_exceed_end, bool has_fixed_time_step, utils::StreamHandler output) - : Bmi_Adapter(type_name + " (BMI Py)", std::move(bmi_init_config), + : Bmi_Adapter(type_name + " (BMI Py)", std::move(bmi_init_config), std::move(forcing_file_path), allow_exceed_end, has_fixed_time_step, output), bmi_type_py_full_name(bmi_python_type), From ee072f35bb1e0f0f65cff23b42119c0bfc57a177 Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Thu, 21 Sep 2023 10:57:07 -0700 Subject: [PATCH 05/11] Delete copy constructor that should never be called --- include/bmi/Bmi_Adapter.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/bmi/Bmi_Adapter.hpp b/include/bmi/Bmi_Adapter.hpp index babffbd151..54c3f1b50b 100644 --- a/include/bmi/Bmi_Adapter.hpp +++ b/include/bmi/Bmi_Adapter.hpp @@ -39,7 +39,7 @@ namespace models { } } - Bmi_Adapter(Bmi_Adapter const&) = default; + Bmi_Adapter(Bmi_Adapter const&) = delete; Bmi_Adapter(Bmi_Adapter &&) = default; /** From 8707e5c9bdd1706563782d9d180a32ac59785135 Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Thu, 21 Sep 2023 13:27:48 -0700 Subject: [PATCH 06/11] Enhance exception message --- include/bmi/AbstractCLibBmiAdapter.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/bmi/AbstractCLibBmiAdapter.hpp b/include/bmi/AbstractCLibBmiAdapter.hpp index 8a7c01cbca..e6c5ab88f3 100644 --- a/include/bmi/AbstractCLibBmiAdapter.hpp +++ b/include/bmi/AbstractCLibBmiAdapter.hpp @@ -165,7 +165,7 @@ namespace models { */ inline void *dynamic_load_symbol(const std::string &symbol_name, bool is_null_valid) { if (dyn_lib_handle == nullptr) { - throw std::runtime_error("Cannot load symbol " + symbol_name + " without handle to shared library"); + throw std::runtime_error("Cannot load symbol '" + symbol_name + "' without handle to shared library (bmi_lib_file = '" + bmi_lib_file + "')"); } // Call first to ensure any previous error is cleared before trying to load the symbol dlerror(); From 7fdb3a7f768133d8af8ab41443c3efaabf18e91a Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Tue, 7 Nov 2023 16:21:05 -0800 Subject: [PATCH 07/11] Bmi_Cpp_Adapter: delete pointless static model_name variable --- include/bmi/Bmi_Cpp_Adapter.hpp | 6 ++---- src/bmi/Bmi_Cpp_Adapter.cpp | 4 +--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/include/bmi/Bmi_Cpp_Adapter.hpp b/include/bmi/Bmi_Cpp_Adapter.hpp index e805b2e39f..a9ed2c0178 100644 --- a/include/bmi/Bmi_Cpp_Adapter.hpp +++ b/include/bmi/Bmi_Cpp_Adapter.hpp @@ -312,8 +312,6 @@ namespace models { void UpdateUntil(double time) override; protected: - // TODO: look at setting this in some other way - static const std::string model_name; /** * Construct the backing BMI model object, then call its BMI-native ``Initialize()`` function. @@ -339,12 +337,12 @@ namespace models { if (get_dyn_lib_handle() == nullptr) { if (model_create_fname.empty()) { this->init_exception_msg = - "Can't init " + this->model_name + "; empty name given for module's create function."; + "Can't init BMI C++ model; empty name given for module's create function."; throw std::runtime_error(this->init_exception_msg); } if (model_destroy_fname.empty()){ this->init_exception_msg = - "Can't init " + this->model_name + "; empty name given for module's destroy function."; + "Can't init BMI C++ model; empty name given for module's destroy function."; throw std::runtime_error(this->init_exception_msg); } dynamic_library_load(); diff --git a/src/bmi/Bmi_Cpp_Adapter.cpp b/src/bmi/Bmi_Cpp_Adapter.cpp index fd80bd26d9..176ede37b3 100644 --- a/src/bmi/Bmi_Cpp_Adapter.cpp +++ b/src/bmi/Bmi_Cpp_Adapter.cpp @@ -6,8 +6,6 @@ using namespace models::bmi; -const std::string Bmi_Cpp_Adapter::model_name = "BMI C++ model"; - Bmi_Cpp_Adapter::Bmi_Cpp_Adapter(const std::string& type_name, std::string library_file_path, std::string forcing_file_path, bool allow_exceed_end, bool has_fixed_time_step, std::string creator_func, std::string destroyer_func, @@ -60,7 +58,7 @@ Bmi_Cpp_Adapter::Bmi_Cpp_Adapter(const std::string& type_name, std::string libra // original object closes the handle for its dynamically loaded lib) it make more sense to remove the copy constructor. // TODO: However, it may make sense to bring it back once it is possible to serialize and deserialize the model. /* -Bmi_Cpp_Adapter::Bmi_Cpp_Adapter(Bmi_Cpp_Adapter &adapter) : model_name(adapter.model_name), +Bmi_Cpp_Adapter::Bmi_Cpp_Adapter(Bmi_Cpp_Adapter &adapter) : allow_model_exceed_end_time(adapter.allow_model_exceed_end_time), bmi_init_config(adapter.bmi_init_config), bmi_lib_file(adapter.bmi_lib_file), From 9322eb6ccb5e9aad19a5159ab816244da7403525 Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Tue, 7 Nov 2023 17:34:09 -0800 Subject: [PATCH 08/11] AbstractCLibBmiAdapter: Move bmi_model member down to leaf classes, since their handling of lifecycle is disparate --- include/bmi/AbstractCLibBmiAdapter.hpp | 5 ----- include/bmi/Bmi_C_Adapter.hpp | 5 ++++- include/bmi/Bmi_Cpp_Adapter.hpp | 4 +++- include/bmi/Bmi_Fortran_Adapter.hpp | 5 +++++ src/bmi/Bmi_C_Adapter.cpp | 2 -- src/bmi/Bmi_Cpp_Adapter.cpp | 5 ----- 6 files changed, 12 insertions(+), 14 deletions(-) diff --git a/include/bmi/AbstractCLibBmiAdapter.hpp b/include/bmi/AbstractCLibBmiAdapter.hpp index e6c5ab88f3..1cb07f5c84 100644 --- a/include/bmi/AbstractCLibBmiAdapter.hpp +++ b/include/bmi/AbstractCLibBmiAdapter.hpp @@ -36,7 +36,6 @@ namespace models { AbstractCLibBmiAdapter(AbstractCLibBmiAdapter &&adapter) noexcept : Bmi_Adapter(std::move(adapter)), - bmi_model(std::move(adapter.bmi_model)), bmi_lib_file(std::move(adapter.bmi_lib_file)), bmi_registration_function(adapter.bmi_registration_function), dyn_lib_handle(adapter.dyn_lib_handle) @@ -206,10 +205,6 @@ namespace models { return dyn_lib_handle; } - protected: - /** Pointer to backing BMI model instance. */ - std::shared_ptr bmi_model = nullptr; - private: /** Path to the BMI shared library file, for dynamic linking. */ diff --git a/include/bmi/Bmi_C_Adapter.hpp b/include/bmi/Bmi_C_Adapter.hpp index f6f4d70e91..0eeb09fe8e 100755 --- a/include/bmi/Bmi_C_Adapter.hpp +++ b/include/bmi/Bmi_C_Adapter.hpp @@ -95,7 +95,7 @@ namespace models { //Bmi_C_Adapter(Bmi_C_Adapter &adapter); // Move constructor - Bmi_C_Adapter(Bmi_C_Adapter &&adapter) noexcept; + Bmi_C_Adapter(Bmi_C_Adapter &&adapter) noexcept = default; /** * Class destructor. @@ -643,6 +643,9 @@ namespace models { // For unit testing friend class ::Bmi_C_Adapter_Test; + /** Pointer to backing BMI model instance. */ + std::shared_ptr bmi_model = nullptr; + }; } diff --git a/include/bmi/Bmi_Cpp_Adapter.hpp b/include/bmi/Bmi_Cpp_Adapter.hpp index a9ed2c0178..b8c54fd8b2 100644 --- a/include/bmi/Bmi_Cpp_Adapter.hpp +++ b/include/bmi/Bmi_Cpp_Adapter.hpp @@ -105,7 +105,7 @@ namespace models { //Bmi_Cpp_Adapter(Bmi_Cpp_Adapter &adapter); // Move constructor - Bmi_Cpp_Adapter(Bmi_Cpp_Adapter &&adapter) noexcept; + Bmi_Cpp_Adapter(Bmi_Cpp_Adapter &&adapter) noexcept = default; /** * Class destructor. @@ -470,6 +470,8 @@ namespace models { // For unit testing friend class ::Bmi_Cpp_Adapter_Test; + /** Pointer to backing BMI model instance. */ + std::shared_ptr bmi_model = nullptr; }; } diff --git a/include/bmi/Bmi_Fortran_Adapter.hpp b/include/bmi/Bmi_Fortran_Adapter.hpp index 41a6fa6269..4818c8ade6 100644 --- a/include/bmi/Bmi_Fortran_Adapter.hpp +++ b/include/bmi/Bmi_Fortran_Adapter.hpp @@ -799,6 +799,11 @@ namespace models { } friend class ::Bmi_Fortran_Adapter_Test; + + private: + /** Pointer to backing BMI model instance. */ + std::shared_ptr bmi_model = nullptr; + }; } } diff --git a/src/bmi/Bmi_C_Adapter.cpp b/src/bmi/Bmi_C_Adapter.cpp index 9e283fcbde..78bc574c22 100755 --- a/src/bmi/Bmi_C_Adapter.cpp +++ b/src/bmi/Bmi_C_Adapter.cpp @@ -132,8 +132,6 @@ Bmi_C_Adapter::Bmi_C_Adapter(Bmi_C_Adapter &adapter) : model_name(adapter.model_ } */ -Bmi_C_Adapter::Bmi_C_Adapter(Bmi_C_Adapter &&adapter) noexcept : AbstractCLibBmiAdapter(std::move(adapter)) { } - std::string Bmi_C_Adapter::GetComponentName() { char component_name[BMI_MAX_COMPONENT_NAME]; if (bmi_model->get_component_name(bmi_model.get(), component_name) != BMI_SUCCESS) { diff --git a/src/bmi/Bmi_Cpp_Adapter.cpp b/src/bmi/Bmi_Cpp_Adapter.cpp index 176ede37b3..3baef252b2 100644 --- a/src/bmi/Bmi_Cpp_Adapter.cpp +++ b/src/bmi/Bmi_Cpp_Adapter.cpp @@ -83,11 +83,6 @@ Bmi_Cpp_Adapter::Bmi_Cpp_Adapter(Bmi_Cpp_Adapter &adapter) : } */ -Bmi_Cpp_Adapter::Bmi_Cpp_Adapter(Bmi_Cpp_Adapter &&adapter) noexcept : - AbstractCLibBmiAdapter(std::move(adapter)), - model_create_fname(std::move(adapter.model_create_fname)), - model_destroy_fname(std::move(adapter.model_destroy_fname)) { } - std::string Bmi_Cpp_Adapter::GetComponentName() { return bmi_model->GetComponentName(); } From 32824d6070ceda60cd7ef4e4ae2ca6b466ebb2b2 Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Tue, 7 Nov 2023 17:44:41 -0800 Subject: [PATCH 09/11] Bmi_*_Adapter: make them non-copyable and non-movable --- include/bmi/Bmi_C_Adapter.hpp | 5 ++--- include/bmi/Bmi_Cpp_Adapter.hpp | 4 ++-- include/bmi/Bmi_Fortran_Adapter.hpp | 3 +++ include/bmi/Bmi_Py_Adapter.hpp | 3 +++ src/realizations/catchment/Bmi_C_Formulation.cpp | 3 +-- src/realizations/catchment/Bmi_Cpp_Formulation.cpp | 3 +-- src/realizations/catchment/Bmi_Py_Formulation.cpp | 3 +-- 7 files changed, 13 insertions(+), 11 deletions(-) diff --git a/include/bmi/Bmi_C_Adapter.hpp b/include/bmi/Bmi_C_Adapter.hpp index 0eeb09fe8e..fc83f97a18 100755 --- a/include/bmi/Bmi_C_Adapter.hpp +++ b/include/bmi/Bmi_C_Adapter.hpp @@ -92,10 +92,9 @@ namespace models { // once original object closes the handle for its dynamically loaded lib) it make more sense to remove the // copy constructor. // TODO: However, it may make sense to bring it back once it's possible to serialize/deserialize the model. - //Bmi_C_Adapter(Bmi_C_Adapter &adapter); + Bmi_C_Adapter(Bmi_C_Adapter &adapter) = delete; - // Move constructor - Bmi_C_Adapter(Bmi_C_Adapter &&adapter) noexcept = default; + Bmi_C_Adapter(Bmi_C_Adapter &&adapter) noexcept = delete; /** * Class destructor. diff --git a/include/bmi/Bmi_Cpp_Adapter.hpp b/include/bmi/Bmi_Cpp_Adapter.hpp index b8c54fd8b2..f702ed500a 100644 --- a/include/bmi/Bmi_Cpp_Adapter.hpp +++ b/include/bmi/Bmi_Cpp_Adapter.hpp @@ -102,10 +102,10 @@ namespace models { // once original object closes the handle for its dynamically loaded lib) it make more sense to remove the // copy constructor. // TODO: However, it may make sense to bring it back once it's possible to serialize/deserialize the model. - //Bmi_Cpp_Adapter(Bmi_Cpp_Adapter &adapter); + Bmi_Cpp_Adapter(Bmi_Cpp_Adapter &adapter) = delete; // Move constructor - Bmi_Cpp_Adapter(Bmi_Cpp_Adapter &&adapter) noexcept = default; + Bmi_Cpp_Adapter(Bmi_Cpp_Adapter &&adapter) noexcept = delete; /** * Class destructor. diff --git a/include/bmi/Bmi_Fortran_Adapter.hpp b/include/bmi/Bmi_Fortran_Adapter.hpp index 4818c8ade6..bb8f6e2532 100644 --- a/include/bmi/Bmi_Fortran_Adapter.hpp +++ b/include/bmi/Bmi_Fortran_Adapter.hpp @@ -76,6 +76,9 @@ namespace models { } } + Bmi_Fortran_Adapter(Bmi_Fortran_Adapter const&) = delete; + Bmi_Fortran_Adapter(Bmi_Fortran_Adapter&&) = delete; + std::string GetComponentName() override; /** diff --git a/include/bmi/Bmi_Py_Adapter.hpp b/include/bmi/Bmi_Py_Adapter.hpp index b69eb9f2ea..377e8a35d2 100644 --- a/include/bmi/Bmi_Py_Adapter.hpp +++ b/include/bmi/Bmi_Py_Adapter.hpp @@ -41,6 +41,9 @@ namespace models { std::string forcing_file_path, bool allow_exceed_end, bool has_fixed_time_step, utils::StreamHandler output); + Bmi_Py_Adapter(Bmi_Py_Adapter const&) = delete; + Bmi_Py_Adapter(Bmi_Py_Adapter&&) = delete; + /** * Copy the given BMI variable's values from the backing numpy array to a C++ array. * diff --git a/src/realizations/catchment/Bmi_C_Formulation.cpp b/src/realizations/catchment/Bmi_C_Formulation.cpp index 5a5ec2a389..04a0106795 100644 --- a/src/realizations/catchment/Bmi_C_Formulation.cpp +++ b/src/realizations/catchment/Bmi_C_Formulation.cpp @@ -25,7 +25,6 @@ std::shared_ptr Bmi_C_Formulation::construct_model(const geojson: std::string reg_func = reg_func_itr == properties.end() ? BMI_C_DEFAULT_REGISTRATION_FUNC : reg_func_itr->second.as_string(); return std::make_shared( - Bmi_C_Adapter( get_model_type_name(), lib_file, get_bmi_init_config(), @@ -33,7 +32,7 @@ std::shared_ptr Bmi_C_Formulation::construct_model(const geojson: get_allow_model_exceed_end_time(), is_bmi_model_time_step_fixed(), reg_func, - output)); + output); } std::string Bmi_C_Formulation::get_output_header_line(std::string delimiter) { diff --git a/src/realizations/catchment/Bmi_Cpp_Formulation.cpp b/src/realizations/catchment/Bmi_Cpp_Formulation.cpp index 82e16a312b..24419b2c75 100644 --- a/src/realizations/catchment/Bmi_Cpp_Formulation.cpp +++ b/src/realizations/catchment/Bmi_Cpp_Formulation.cpp @@ -34,7 +34,6 @@ std::shared_ptr Bmi_Cpp_Formulation::construct_model(const geoj json_prop_itr == properties.end() ? BMI_REALIZATION_CFG_PARAM_OPT__CPP_DESTROY_FUNC_DEFAULT : json_prop_itr->second.as_string(); return std::make_shared( - Bmi_Cpp_Adapter( get_model_type_name(), lib_file, get_bmi_init_config(), @@ -43,7 +42,7 @@ std::shared_ptr Bmi_Cpp_Formulation::construct_model(const geoj is_bmi_model_time_step_fixed(), model_create_fname, model_destroy_fname, - output)); + output); } std::string Bmi_Cpp_Formulation::get_output_header_line(std::string delimiter) { diff --git a/src/realizations/catchment/Bmi_Py_Formulation.cpp b/src/realizations/catchment/Bmi_Py_Formulation.cpp index e52f2ffa17..cc69a80a32 100644 --- a/src/realizations/catchment/Bmi_Py_Formulation.cpp +++ b/src/realizations/catchment/Bmi_Py_Formulation.cpp @@ -23,13 +23,12 @@ shared_ptr Bmi_Py_Formulation::construct_model(const geojson::Pr std::string python_type_name = python_type_name_iter->second.as_string(); return std::make_shared( - Bmi_Py_Adapter( get_model_type_name(), get_bmi_init_config(), python_type_name, get_allow_model_exceed_end_time(), is_bmi_model_time_step_fixed(), - output)); + output); } time_t realization::Bmi_Py_Formulation::convert_model_time(const double &model_time) { From 6ac2685fb0e9e065e4338d413f25bba033766aa6 Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Wed, 8 Nov 2023 13:07:02 -0800 Subject: [PATCH 10/11] AbstractCLibBmiAdapter: De-template since the bmi_model member is moved down --- include/bmi/AbstractCLibBmiAdapter.hpp | 1 - include/bmi/Bmi_C_Adapter.hpp | 2 +- include/bmi/Bmi_Cpp_Adapter.hpp | 2 +- include/bmi/Bmi_Fortran_Adapter.hpp | 2 +- src/bmi/Bmi_C_Adapter.cpp | 2 +- src/bmi/Bmi_Cpp_Adapter.cpp | 2 +- 6 files changed, 5 insertions(+), 6 deletions(-) diff --git a/include/bmi/AbstractCLibBmiAdapter.hpp b/include/bmi/AbstractCLibBmiAdapter.hpp index 1cb07f5c84..6e7e23823f 100644 --- a/include/bmi/AbstractCLibBmiAdapter.hpp +++ b/include/bmi/AbstractCLibBmiAdapter.hpp @@ -9,7 +9,6 @@ namespace models { namespace bmi { - template class AbstractCLibBmiAdapter : public Bmi_Adapter { public: diff --git a/include/bmi/Bmi_C_Adapter.hpp b/include/bmi/Bmi_C_Adapter.hpp index fc83f97a18..d736cf5329 100755 --- a/include/bmi/Bmi_C_Adapter.hpp +++ b/include/bmi/Bmi_C_Adapter.hpp @@ -20,7 +20,7 @@ namespace models { * An adapter class to serve as a C++ interface to the essential aspects of external models written in the C * language that implement the BMI. */ - class Bmi_C_Adapter : public AbstractCLibBmiAdapter { + class Bmi_C_Adapter : public AbstractCLibBmiAdapter { public: diff --git a/include/bmi/Bmi_Cpp_Adapter.hpp b/include/bmi/Bmi_Cpp_Adapter.hpp index f702ed500a..b3571db78d 100644 --- a/include/bmi/Bmi_Cpp_Adapter.hpp +++ b/include/bmi/Bmi_Cpp_Adapter.hpp @@ -24,7 +24,7 @@ namespace models { * loaded dynamically from libraries. This is less important than e.g. @see Bmi_C_Adapter but still provides * some useful generalized functionality. */ - class Bmi_Cpp_Adapter : public AbstractCLibBmiAdapter { + class Bmi_Cpp_Adapter : public AbstractCLibBmiAdapter { public: diff --git a/include/bmi/Bmi_Fortran_Adapter.hpp b/include/bmi/Bmi_Fortran_Adapter.hpp index bb8f6e2532..00bef70f7f 100644 --- a/include/bmi/Bmi_Fortran_Adapter.hpp +++ b/include/bmi/Bmi_Fortran_Adapter.hpp @@ -21,7 +21,7 @@ namespace models { * An adapter class to serve as a C++ interface to the essential aspects of external models written in the * Fortran language that implement the BMI. */ - class Bmi_Fortran_Adapter : public AbstractCLibBmiAdapter { + class Bmi_Fortran_Adapter : public AbstractCLibBmiAdapter { public: diff --git a/src/bmi/Bmi_C_Adapter.cpp b/src/bmi/Bmi_C_Adapter.cpp index 78bc574c22..733fe834e1 100755 --- a/src/bmi/Bmi_C_Adapter.cpp +++ b/src/bmi/Bmi_C_Adapter.cpp @@ -69,7 +69,7 @@ Bmi_C_Adapter::Bmi_C_Adapter(const std::string &type_name, std::string library_f Bmi_C_Adapter::Bmi_C_Adapter(const std::string &type_name, std::string library_file_path, std::string bmi_init_config, std::string forcing_file_path, bool allow_exceed_end, bool has_fixed_time_step, std::string registration_func, utils::StreamHandler output, bool do_initialization) - : AbstractCLibBmiAdapter(type_name, library_file_path, std::move(bmi_init_config), std::move(forcing_file_path), allow_exceed_end, + : AbstractCLibBmiAdapter(type_name, library_file_path, std::move(bmi_init_config), std::move(forcing_file_path), allow_exceed_end, has_fixed_time_step, registration_func, output) { if (do_initialization) { diff --git a/src/bmi/Bmi_Cpp_Adapter.cpp b/src/bmi/Bmi_Cpp_Adapter.cpp index 3baef252b2..94a48707af 100644 --- a/src/bmi/Bmi_Cpp_Adapter.cpp +++ b/src/bmi/Bmi_Cpp_Adapter.cpp @@ -25,7 +25,7 @@ Bmi_Cpp_Adapter::Bmi_Cpp_Adapter(const std::string& type_name, std::string libra std::string forcing_file_path, bool allow_exceed_end, bool has_fixed_time_step, std::string creator_func, std::string destroyer_func, utils::StreamHandler output, bool do_initialization) - : AbstractCLibBmiAdapter(type_name, library_file_path, std::move(bmi_init_config), std::move(forcing_file_path), allow_exceed_end, + : AbstractCLibBmiAdapter(type_name, library_file_path, std::move(bmi_init_config), std::move(forcing_file_path), allow_exceed_end, has_fixed_time_step, creator_func, output), model_create_fname(std::move(creator_func)), model_destroy_fname(std::move(destroyer_func)) From 1e0ab7d8f0cf858a29c1241904fccc28782359c8 Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Wed, 8 Nov 2023 13:58:25 -0800 Subject: [PATCH 11/11] Convert bmi_model members from shared_ptr to unique_ptr This avoids giving the impression that there should ever actually be shared ownership - a model instance is always exclusively held by the single adapter instance that created it. Leave the C++ adapter alone for now due to the more complicated lifetime logic that it implements. Leave the Python adapted alone because the unit tests do silly things with reaching in and grabbing the underlying object. --- include/bmi/Bmi_C_Adapter.hpp | 4 ++-- include/bmi/Bmi_Fortran_Adapter.hpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/bmi/Bmi_C_Adapter.hpp b/include/bmi/Bmi_C_Adapter.hpp index d736cf5329..5dcabbf0ad 100755 --- a/include/bmi/Bmi_C_Adapter.hpp +++ b/include/bmi/Bmi_C_Adapter.hpp @@ -562,7 +562,7 @@ namespace models { inline void construct_and_init_backing_model_for_type() { if (model_initialized) return; - bmi_model = std::make_shared(C_Bmi()); + bmi_model = std::make_unique(C_Bmi()); execModuleRegistration(); int init_result = bmi_model->initialize(bmi_model.get(), bmi_init_config.c_str()); if (init_result != BMI_SUCCESS) { @@ -643,7 +643,7 @@ namespace models { friend class ::Bmi_C_Adapter_Test; /** Pointer to backing BMI model instance. */ - std::shared_ptr bmi_model = nullptr; + std::unique_ptr bmi_model = nullptr; }; diff --git a/include/bmi/Bmi_Fortran_Adapter.hpp b/include/bmi/Bmi_Fortran_Adapter.hpp index 00bef70f7f..c21a77c13d 100644 --- a/include/bmi/Bmi_Fortran_Adapter.hpp +++ b/include/bmi/Bmi_Fortran_Adapter.hpp @@ -498,7 +498,7 @@ namespace models { inline void construct_and_init_backing_model_for_fortran() { if (model_initialized) return; - bmi_model = std::make_shared(Bmi_Fortran_Handle_Wrapper()); + bmi_model = std::make_unique(Bmi_Fortran_Handle_Wrapper()); dynamic_library_load(); execModuleRegistration(); int init_result = initialize(&bmi_model->handle, bmi_init_config.c_str()); @@ -805,7 +805,7 @@ namespace models { private: /** Pointer to backing BMI model instance. */ - std::shared_ptr bmi_model = nullptr; + std::unique_ptr bmi_model = nullptr; }; }