Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bmi_Adapter cleanup, de-template #646

Merged
merged 11 commits into from
Nov 13, 2023
Merged
9 changes: 4 additions & 5 deletions include/bmi/AbstractCLibBmiAdapter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@
namespace models {
namespace bmi {

template <class C>
class AbstractCLibBmiAdapter : public Bmi_Adapter<C> {
class AbstractCLibBmiAdapter : public Bmi_Adapter {

public:
/**
Expand All @@ -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<C>(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<C>(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)
Expand Down Expand Up @@ -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();
Expand Down
45 changes: 2 additions & 43 deletions include/bmi/Bmi_Adapter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 T>
class Bmi_Adapter : public ::bmi::Bmi {
public:

Expand All @@ -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.
Expand Down Expand Up @@ -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<T> 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. */
Expand Down
12 changes: 7 additions & 5 deletions include/bmi/Bmi_C_Adapter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<C_Bmi> {
class Bmi_C_Adapter : public AbstractCLibBmiAdapter {

public:

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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>(C_Bmi());
bmi_model = std::make_unique<C_Bmi>(C_Bmi());
execModuleRegistration();
int init_result = bmi_model->initialize(bmi_model.get(), bmi_init_config.c_str());
if (init_result != BMI_SUCCESS) {
Expand Down Expand Up @@ -643,6 +642,9 @@ namespace models {
// For unit testing
friend class ::Bmi_C_Adapter_Test;

/** Pointer to backing BMI model instance. */
std::unique_ptr<C_Bmi> bmi_model = nullptr;

};

}
Expand Down
14 changes: 7 additions & 7 deletions include/bmi/Bmi_Cpp_Adapter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<Cpp_Bmi> {
class Bmi_Cpp_Adapter : public AbstractCLibBmiAdapter {

public:

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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();
Expand Down Expand Up @@ -472,6 +470,8 @@ namespace models {
// For unit testing
friend class ::Bmi_Cpp_Adapter_Test;

/** Pointer to backing BMI model instance. */
std::shared_ptr<Cpp_Bmi> bmi_model = nullptr;
};

}
Expand Down
12 changes: 10 additions & 2 deletions include/bmi/Bmi_Fortran_Adapter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<Bmi_Fortran_Handle_Wrapper> {
class Bmi_Fortran_Adapter : public AbstractCLibBmiAdapter {

public:

Expand Down Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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_Fortran_Handle_Wrapper());
bmi_model = std::make_unique<Bmi_Fortran_Handle_Wrapper>(Bmi_Fortran_Handle_Wrapper());
dynamic_library_load();
execModuleRegistration();
int init_result = initialize(&bmi_model->handle, bmi_init_config.c_str());
Expand Down Expand Up @@ -799,6 +802,11 @@ namespace models {
}

friend class ::Bmi_Fortran_Adapter_Test;

private:
/** Pointer to backing BMI model instance. */
std::unique_ptr<Bmi_Fortran_Handle_Wrapper> bmi_model = nullptr;

};
}
}
Expand Down
10 changes: 9 additions & 1 deletion include/bmi/Bmi_Py_Adapter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<py::object> {
class Bmi_Py_Adapter : public Bmi_Adapter {

public:

Expand All @@ -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.
*
Expand Down Expand Up @@ -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<py::object> bmi_model = nullptr;

};

}
Expand Down
4 changes: 2 additions & 2 deletions include/utilities/bmi_utilities.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ namespace models {
* @param name Bmi variable name to query the model for
* @return std::vector<T> Copy of data from the BMI model for variable @param name
*/
template <typename T, typename A>
std::vector<T> GetValue(Bmi_Adapter<A>& model, const std::string& name) {
template <typename T>
std::vector<T> 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);
Expand Down
4 changes: 1 addition & 3 deletions src/bmi/Bmi_C_Adapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<C_Bmi>(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) {
Expand Down Expand Up @@ -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<C_Bmi>(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) {
Expand Down
11 changes: 2 additions & 9 deletions src/bmi/Bmi_Cpp_Adapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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<Cpp_Bmi>(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))
Expand Down Expand Up @@ -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),
Expand All @@ -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<Cpp_Bmi>(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();
}
Expand Down
2 changes: 1 addition & 1 deletion src/bmi/Bmi_Py_Adapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<py::object>(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),
Expand Down
3 changes: 1 addition & 2 deletions src/realizations/catchment/Bmi_C_Formulation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,14 @@ std::shared_ptr<Bmi_C_Adapter> 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>(
Bmi_C_Adapter(
get_model_type_name(),
lib_file,
get_bmi_init_config(),
(is_bmi_using_forcing_file() ? get_forcing_file_path() : ""),
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) {
Expand Down
3 changes: 1 addition & 2 deletions src/realizations/catchment/Bmi_Cpp_Formulation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ std::shared_ptr<Bmi_Cpp_Adapter> 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>(
Bmi_Cpp_Adapter(
get_model_type_name(),
lib_file,
get_bmi_init_config(),
Expand All @@ -43,7 +42,7 @@ std::shared_ptr<Bmi_Cpp_Adapter> 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) {
Expand Down
Loading
Loading