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.

Sign up for GitHub

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

Refactor get_available_variable_names to return a span<string> rather than a concrete vector<string> #644

Merged
merged 3 commits into from
Sep 21, 2023
Merged

Refactor get_available_variable_names to return a span<string> rather than a concrete vector<string> #644

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7cabee5
Refactor get_available_variable_names to return a span<string> rather…
PhilMiller Sep 20, 2023
6c6bd67
Add missing 'override' annotation
PhilMiller Sep 21, 2023
f3cb2f0
NullForcingProvider: Drop now-unnecessary private empty vector
PhilMiller Sep 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/forcing/CsvPerFeatureForcingProvider.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ class CsvPerFeatureForcingProvider : public data_access::GenericDataProvider
return is_param_sum_over_time_step(name);
}

const std::vector<std::string> &get_available_variable_names() override {
boost::span<const std::string> get_available_variable_names() override {
return available_forcings;
}

Expand Down
3 changes: 2 additions & 1 deletion include/forcing/DataProvider.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include <string>
#include <vector>
#include <span.hpp>

namespace data_access
{
Expand Down Expand Up @@ -32,7 +33,7 @@ namespace data_access

/** Return the variables that are accessable by this data provider */

virtual const std::vector<std::string>& get_available_variable_names() = 0;
virtual boost::span<const std::string> get_available_variable_names() = 0;

/** Return the first valid time for which data from the request variable can be requested */

Expand Down
4 changes: 2 additions & 2 deletions include/forcing/DeferredWrappedProvider.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ namespace data_access {
* @return The names of the outputs for which this instance is (or will be) able to provide values.
*/

const std::vector<std::string> &get_available_variable_names() {
boost::span<const std::string> get_available_variable_names() override {
return providedOutputs;
}

Expand Down Expand Up @@ -137,7 +137,7 @@ namespace data_access {
}

// Confirm this will provide everything needed
const std::vector<std::string> &available = provider->get_available_variable_names();
const auto available = provider->get_available_variable_names();
for (const std::string &requiredName : providedOutputs) {
if (std::find(available.begin(), available.end(), requiredName) == available.end()) {
setMessage = "Given provider does not provide the required " + requiredName;
Expand Down
2 changes: 1 addition & 1 deletion include/forcing/NetCDFPerFeatureDataProvider.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ namespace data_access
~NetCDFPerFeatureDataProvider();

/** Return the variables that are accessable by this data provider */
const std::vector<std::string>& get_available_variable_names() override;
boost::span<const std::string> get_available_variable_names() override;

/** return a list of ids in the current file */
const std::vector<std::string>& get_ids() const;
Expand Down
9 changes: 2 additions & 7 deletions include/forcing/NullForcingProvider.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,9 @@ class NullForcingProvider : public data_access::GenericDataProvider
throw std::runtime_error("Got request for variable " + name + " but no such variable is provided by NullForcingProvider." + SOURCE_LOC);
}

const std::vector<std::string> &get_available_variable_names() override {
return available_forcings;
boost::span<const std::string> get_available_variable_names() override {
return {};
}

private:

std::vector<std::string> available_forcings;

};

#endif // NGEN_NULLFORCING_H
2 changes: 1 addition & 1 deletion include/forcing/OptionalWrappedDataProvider.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ namespace data_access {
std::map<std::string, int> defaultUsageWaits;

static bool isSuppliedByProvider(const std::string &outputName, GenericDataProvider *provider) {
const std::vector<std::string> &available = provider->get_available_variable_names();
auto available = provider->get_available_variable_names();
return find(available.begin(), available.end(), outputName) != available.end();
}

Expand Down
2 changes: 1 addition & 1 deletion include/forcing/WrappedDataProvider.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ namespace data_access {
* @return const std::vector<std::string>& the names of available data variables
*/

const std::vector<std::string> &get_available_variable_names() override {
boost::span<const std::string> get_available_variable_names() override {
return wrapped_provider->get_available_variable_names();
}

Expand Down
13 changes: 3 additions & 10 deletions include/realizations/catchment/Bmi_Module_Formulation.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ namespace realization {
* @return The collection of forcing output property names this instance can provide.
* @see ForcingProvider
*/
//const vector<std::string> &get_available_forcing_outputs() {
const std::vector<std::string> &get_available_variable_names() override {
boost::span<const std::string> get_available_variable_names() override {
if (is_model_initialized() && available_forcings.empty()) {
for (const std::string &output_var_name : get_bmi_model()->GetOutputVarNames()) {
available_forcings.push_back(output_var_name);
Expand All @@ -82,10 +81,6 @@ namespace realization {
return available_forcings;
}

//inline const vector<std::string> &get_available_variable_names() override {
// return get_available_forcing_outputs();
//}

/**
* Get the inclusive beginning of the period of time over which this instance can provide data for this forcing.
*
Expand Down Expand Up @@ -219,8 +214,7 @@ namespace realization {
std::string output_units = selector.get_output_units();

// First make sure this is an available output
//const std::vector<std::string> forcing_outputs = get_available_forcing_outputs();
const std::vector<std::string> forcing_outputs = get_available_variable_names();
auto forcing_outputs = get_available_variable_names();
if (std::find(forcing_outputs.begin(), forcing_outputs.end(), output_name) == forcing_outputs.end()) {
throw std::runtime_error(get_formulation_type() + " received invalid output forcing name " + output_name);
}
Expand Down Expand Up @@ -288,8 +282,7 @@ namespace realization {
std::string output_units = selector.get_output_units();

// First make sure this is an available output
//const std::vector<std::string> forcing_outputs = get_available_forcing_outputs();
const std::vector<std::string> forcing_outputs = get_available_variable_names();
auto forcing_outputs = get_available_variable_names();
if (std::find(forcing_outputs.begin(), forcing_outputs.end(), output_name) == forcing_outputs.end()) {
throw std::runtime_error(get_formulation_type() + " received invalid output forcing name " + output_name);
}
Expand Down
4 changes: 1 addition & 3 deletions include/realizations/catchment/Bmi_Multi_Formulation.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,7 @@ namespace realization {
* @return The collection of forcing output property names this instance can provide.
* @see ForcingProvider
*/
//const vector<std::string> &get_available_forcing_outputs();
//const vector<std::string> &get_available_variable_names() override { return get_available_forcing_outputs(); }
const std::vector<std::string> &get_available_variable_names() override;
boost::span <const std::string> get_available_variable_names() override;

/**
* Get the input variables of
Expand Down
2 changes: 1 addition & 1 deletion src/forcing/NetCDFPerFeatureDataProvider.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ NetCDFPerFeatureDataProvider::NetCDFPerFeatureDataProvider(std::string input_pat

NetCDFPerFeatureDataProvider::~NetCDFPerFeatureDataProvider() = default;

const std::vector<std::string>& NetCDFPerFeatureDataProvider::get_available_variable_names()
boost::span<const std::string> NetCDFPerFeatureDataProvider::get_available_variable_names()
{
return variable_names;
}
Expand Down
3 changes: 1 addition & 2 deletions src/realizations/catchment/Bmi_Multi_Formulation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,7 @@ const bool &Bmi_Multi_Formulation::get_allow_model_exceed_end_time() const {
* @return The collection of forcing output property names this instance can provide.
* @see ForcingProvider
*/
//const vector<std::string> &Bmi_Multi_Formulation::get_available_forcing_outputs() {
const std::vector<std::string> &Bmi_Multi_Formulation::get_available_variable_names() {
boost::span<const std::string> Bmi_Multi_Formulation::get_available_variable_names() {
if (is_model_initialized() && available_forcings.empty()) {
for (const nested_module_ptr &module: modules) {
for (const std::string &out_var_name: module->get_bmi_output_variables()) {
Expand Down
4 changes: 2 additions & 2 deletions test/forcing/CsvPerFeatureForcingProvider_Test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ TEST_F(CsvPerFeatureForcingProviderTest, TestForcingDataUnitConversion)
///Test AORC Forcing Object
TEST_F(CsvPerFeatureForcingProviderTest, TestGetAvailableForcingOutputs)
{
const std::vector<std::string>& afos = Forcing_Object->get_available_variable_names();
auto afos = Forcing_Object->get_available_variable_names();
EXPECT_EQ(afos.size(), 18);
EXPECT_TRUE(std::find(afos.begin(), afos.end(), "DLWRF_surface") != afos.end());
EXPECT_TRUE(std::find(afos.begin(), afos.end(), CSDMS_STD_NAME_SOLAR_LONGWAVE) != afos.end());
Expand Down Expand Up @@ -221,4 +221,4 @@ TEST_F(CsvPerFeatureForcingProviderTest, TestForcingUnitHeaderParsing)
}

}
}
}
2 changes: 1 addition & 1 deletion test/forcing/TrivialForcingProvider.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace data_access {
outputs.push_back(OUTPUT_NAME_1);
}

const std::vector<std::string>& get_available_variable_names() override {
boost::span<const std::string> get_available_variable_names() override {
return outputs;
}

Expand Down