Skip to content

Unit Conversion Error (NGWPC-7604) #40

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

Closed
mkarim-rtx wants to merge 3 commits into from
Closed

Unit Conversion Error (NGWPC-7604) #40

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
9605b42
Normalize “none”→“1” and short-circuit equal/unspecified conversions …
mkarim-rtx Sep 17, 2025
c6c047a
fixing indentation
mkarim-rtx Sep 17, 2025
18d4867
unit conversion fix
mkarim-rtx Sep 18, 2025
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/realizations/catchment/Bmi_Multi_Formulation.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ namespace realization {
try {
auto const& nested_module = data_provider_iter->second;
long nested_module_time = nested_module->get_data_start_time() + ( this->get_model_current_time() - this->get_model_start_time() );
auto selector = CatchmentAggrDataSelector(this->get_catchment_id(),var_name,nested_module_time,this->record_duration(),"1");
auto selector = CatchmentAggrDataSelector(this->get_catchment_id(),var_name,nested_module_time,this->record_duration(),"");
//TODO: After merge PR#405, try re-adding support for index
return nested_module->get_value(selector);
}
Expand Down
57 changes: 47 additions & 10 deletions src/core/mediator/UnitsHelper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,34 +58,71 @@ std::shared_ptr<cv_converter> UnitsHelper::get_converter(const std::string& in_u

double UnitsHelper::get_converted_value(const std::string &in_units, const double &value, const std::string &out_units)
{
if(in_units == out_units){
return value; // Early-out optimization
auto is_noneish = [](const std::string& u)->bool {
return u.empty() || u == "none" || u == "unitless" || u == "dimensionless" || u == "-";
};

// Normalize input units: map none-ish → "1"
const std::string in_norm = is_noneish(in_units) ? std::string("1") : in_units;

// Normalize requested units:
// - none-ish → "1" if input is "1"; otherwise "" (unspecified → skip conversion)
std::string out_norm;
if (is_noneish(out_units)) {
out_norm = (in_norm == "1") ? std::string("1") : std::string("");
}
else {
out_norm = out_units;
}

// Early outs (no UDUNITS parsing or converter creation)
if (out_norm.empty() || in_norm == out_norm) {
return value;
}

std::call_once(unit_system_inited, init_unit_system);

auto converter = get_converter(in_units, out_units);
auto converter = get_converter(in_norm, out_norm);

double r = cv_convert_double(converter.get(), value);
return r;
}

double* UnitsHelper::convert_values(const std::string &in_units, double* in_values, const std::string &out_units, double* out_values, const size_t& count)
{
if(in_units == out_units){
// Early-out optimization
if(in_values == out_values){
auto is_noneish = [](const std::string& u)->bool {
return u.empty() || u == "none" || u == "unitless" || u == "dimensionless" || u == "-";
};

// Normalize input units: map none-ish → "1"
const std::string in_norm = is_noneish(in_units) ? std::string("1") : in_units;

// Normalize requested units:
// - none-ish → "1" if input is "1"; otherwise "" (unspecified → skip conversion)
std::string out_norm;
if (is_noneish(out_units)) {
out_norm = (in_norm == "1") ? std::string("1") : std::string("");
}
else {
out_norm = out_units;
}

// Early outs (no UDUNITS parsing or converter creation)
if (out_norm.empty() || in_norm == out_norm) {
// Pass-through
if (in_values == out_values) {
return in_values;
} else {
memcpy(out_values, in_values, sizeof(double)*count);
std::memcpy(out_values, in_values, sizeof(double)*count);
return out_values;
}
}

std::call_once(unit_system_inited, init_unit_system);
auto converter = get_converter(in_units, out_units);

auto converter = get_converter(in_norm, out_norm);

cv_convert_doubles(converter.get(), in_values, count, out_values);

return out_values;
}

132 changes: 101 additions & 31 deletions src/realizations/catchment/Bmi_Module_Formulation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,18 +176,35 @@ namespace realization {

// Convert units
std::string native_units = get_bmi_model()->GetVarUnits(bmi_var_name);

// minimal robustness: if no units requested, return native
if (output_units.empty()) {
return values;
}
auto norm = [](std::string u) {
std::transform(u.begin(), u.end(), u.begin(), [](unsigned char c){ return std::tolower(c); });
if (u.empty() || u == "none" || u == "-" || u == "unitless" || u == "dimensionless") return std::string("1");
return u;
};
std::string native_units_norm = norm(native_units);
std::string output_units_norm = norm(output_units);
if (native_units_norm == output_units_norm) {
return values;
}

try {
UnitsHelper::convert_values(native_units, values.data(), output_units, values.data(), values.size());
UnitsHelper::convert_values(native_units_norm, values.data(), output_units_norm, values.data(), values.size());
return values;
}
catch (const std::runtime_error& e) {
catch (const std::runtime_error& e) {
data_access::unit_conversion_exception uce(e.what());
uce.provider_model_name = get_bmi_model()->get_model_name();
uce.provider_model_name = get_bmi_model()->get_model_name(); // provider component name
uce.provider_bmi_var_name = bmi_var_name;
uce.provider_units = native_units;
uce.unconverted_values = std::move(values);
uce.provider_units = native_units; // include units for the logger
uce.unconverted_values = std::move(values);
throw uce;
}

}
//This is unlikely (impossible?) to throw since a pre-check on available names is done above. Assert instead?
throw std::runtime_error(get_formulation_type() + " received invalid output forcing name " + output_name);
Expand Down Expand Up @@ -225,24 +242,40 @@ namespace realization {

// Convert units
std::string native_units = get_bmi_model()->GetVarUnits(bmi_var_name);

// minimal robustness: if no units requested, return native
if (output_units.empty()) {
return value;
}
auto norm = [](std::string u) {
std::transform(u.begin(), u.end(), u.begin(), [](unsigned char c){ return std::tolower(c); });
if (u.empty() || u == "none" || u == "-" || u == "unitless" || u == "dimensionless") return std::string("1");
return u;
};
std::string native_units_norm = norm(native_units);
std::string output_units_norm = norm(output_units);
if (native_units_norm == output_units_norm) {
return value;
}

try {
return UnitsHelper::get_converted_value(native_units, value, output_units);
return UnitsHelper::get_converted_value(native_units_norm, value, output_units_norm);
}
catch (const std::runtime_error& e){
catch (const std::runtime_error& e) {
data_access::unit_conversion_exception uce(e.what());
uce.provider_model_name = get_bmi_model()->get_model_name();
uce.provider_model_name = get_bmi_model()->get_model_name(); // provider component name
uce.provider_bmi_var_name = bmi_var_name;
uce.provider_units = native_units;
uce.provider_units = native_units; // include units for the logger
uce.unconverted_values.push_back(value);
throw uce;
}
}

}

//This is unlikely (impossible?) to throw since a pre-check on available names is done above. Assert instead?
throw std::runtime_error(get_formulation_type() + " received invalid output forcing name " + output_name);
}


static bool is_var_name_in_collection(const std::vector<std::string> &all_names, const std::string &var_name) {
return std::count(all_names.begin(), all_names.end(), var_name) > 0;
}
Expand Down Expand Up @@ -692,34 +725,72 @@ namespace realization {
// 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);

// Normalize consumer units once per var; if dimensionless, avoid asking providers to convert to "1"
std::string consumer_units_raw = get_bmi_model()->GetVarUnits(var_name);
std::string units_for_selector = consumer_units_raw;
if (units_for_selector.empty() || units_for_selector == "none" || units_for_selector == "unitless" || units_for_selector == "dimensionless" || units_for_selector == "-") {
units_for_selector = "";
}

if (numItems != 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)));
//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::stringstream ss;
ss << "WARN: broadcasting variable '" << var_name << "' from scalar to expected array\n";;
LOG(ss.str(), LogLevel::SEVERE); ss.str("");
#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()) +
" items\n");
try {
auto values = provider->get_values(CatchmentAggrDataSelector(this->get_catchment_id(),var_map_alias, model_epoch_time, t_delta,
units_for_selector));
//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::stringstream ss;
ss << "WARN: broadcasting variable '" << var_name << "' from scalar to expected array\n";;
LOG(ss.str(), LogLevel::SEVERE); ss.str("");
#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()) +
" items\n");

}
value_ptr = get_values_as_type( type, values.begin(), values.end() );
} catch (data_access::unit_conversion_exception &uce) {
data_access::unit_error_log_key key{get_id(), var_map_alias, uce.provider_model_name, uce.provider_bmi_var_name, uce.what()};
auto ret = data_access::unit_errors_reported.insert(key);
bool new_error = ret.second;
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) << "'}"
<< " provider {'" << uce.provider_model_name << "' source variable '" << uce.provider_bmi_var_name << "'"
<< " raw values count " << uce.unconverted_values.size() << "}"
<< " message \"" << uce.what() << "\"";
LOG(ss.str(), LogLevel::WARNING); ss.str("");
}
std::vector<double> fallback(numItems, 0.0);
if (!uce.unconverted_values.empty()) {
if (uce.unconverted_values.size() == 1) {
std::fill(fallback.begin(), fallback.end(), uce.unconverted_values[0]);
} else if (uce.unconverted_values.size() == (size_t)numItems) {
fallback = uce.unconverted_values;
} else {
for (int i = 0; i < numItems; ++i) {
fallback[i] = uce.unconverted_values[i % uce.unconverted_values.size()];
}
}
}
value_ptr = get_values_as_type(type, fallback.begin(), fallback.end());
}
value_ptr = get_values_as_type( 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)));
units_for_selector));
value_ptr = get_value_as_type(type, value);
} catch (data_access::unit_conversion_exception &uce) {
data_access::unit_error_log_key key{get_id(), var_map_alias, uce.provider_model_name, uce.provider_bmi_var_name, uce.what()};
Expand All @@ -743,7 +814,6 @@ namespace realization {
}
}


void Bmi_Module_Formulation::append_model_inputs_to_stream(const double &model_init_time, time_step_t t_delta, std::stringstream &inputs) {
std::vector<std::string> 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();
Expand Down
Loading