diff --git a/include/bmi/AbstractCLibBmiAdapter.hpp b/include/bmi/AbstractCLibBmiAdapter.hpp index 0eea6ed911..6e7e23823f 100644 --- a/include/bmi/AbstractCLibBmiAdapter.hpp +++ b/include/bmi/AbstractCLibBmiAdapter.hpp @@ -9,8 +9,7 @@ namespace models { namespace bmi { - template - class AbstractCLibBmiAdapter : public Bmi_Adapter { + class AbstractCLibBmiAdapter : public Bmi_Adapter { public: /** @@ -29,13 +28,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_lib_file(std::move(adapter.bmi_lib_file)), bmi_registration_function(adapter.bmi_registration_function), dyn_lib_handle(adapter.dyn_lib_handle) @@ -164,7 +163,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(); diff --git a/include/bmi/Bmi_Adapter.hpp b/include/bmi/Bmi_Adapter.hpp index 640ef34d5f..54c3f1b50b 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: @@ -40,46 +39,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 const&) = delete; + Bmi_Adapter(Bmi_Adapter &&) = default; /** * Determine backing model's time units and return an appropriate conversion factor. @@ -259,8 +220,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_C_Adapter.hpp b/include/bmi/Bmi_C_Adapter.hpp index f6f4d70e91..5dcabbf0ad 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: @@ -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; + Bmi_C_Adapter(Bmi_C_Adapter &&adapter) noexcept = delete; /** * Class destructor. @@ -563,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,6 +642,9 @@ namespace models { // For unit testing friend class ::Bmi_C_Adapter_Test; + /** Pointer to backing BMI model instance. */ + std::unique_ptr bmi_model = nullptr; + }; } diff --git a/include/bmi/Bmi_Cpp_Adapter.hpp b/include/bmi/Bmi_Cpp_Adapter.hpp index e805b2e39f..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: @@ -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; + Bmi_Cpp_Adapter(Bmi_Cpp_Adapter &&adapter) noexcept = delete; /** * Class destructor. @@ -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(); @@ -472,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..c21a77c13d 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: @@ -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; /** @@ -495,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()); @@ -799,6 +802,11 @@ namespace models { } friend class ::Bmi_Fortran_Adapter_Test; + + private: + /** Pointer to backing BMI model instance. */ + std::unique_ptr bmi_model = nullptr; + }; } } diff --git a/include/bmi/Bmi_Py_Adapter.hpp b/include/bmi/Bmi_Py_Adapter.hpp index 1cd91ab5ba..377e8a35d2 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: @@ -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. * @@ -767,6 +770,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; + }; } 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_C_Adapter.cpp b/src/bmi/Bmi_C_Adapter.cpp index 9e283fcbde..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) { @@ -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 fd80bd26d9..94a48707af 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, @@ -27,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)) @@ -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), @@ -85,11 +83,6 @@ Bmi_Cpp_Adapter::Bmi_Cpp_Adapter(Bmi_Cpp_Adapter &adapter) : model_name(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(); } 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), 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) {