Fix 384, prioritize use of BMI potential evapotranspiration

include/realizations/catchment/Bmi_Module_Formulation.hpp

@@ -372,40 +372,27 @@ namespace realization {
             }
             */
 
-            // Handle ET requests slightly differently
-            //TODO: This should really only happen if neither of these output_names are provided by the module...
-            // But this code should also probably be removed in the near future (see GH #297 second comment).
-            if (output_name == NGEN_STD_NAME_POTENTIAL_ET_FOR_TIME_STEP || output_name == CSDMS_STD_NAME_POTENTIAL_ET) {
-                return calc_et();
-            }
-
-            // If not ET, now get correct BMI variable name, which may be the output or something mapped to this output.
-            // TODO: move this to a dedicated function
+            // check if output is available from BMI
             std::string bmi_var_name;
-            std::vector<std::string> output_names = get_bmi_model()->GetOutputVarNames();
-            if (std::find(output_names.begin(), output_names.end(), output_name) != output_names.end()) {
-                bmi_var_name = output_name;
-            }
-            else {
-                for (auto & iter : bmi_var_names_map) {
-                    if (iter.second == output_name) {
-                        bmi_var_name = iter.first;
-                        break;
-                    }
-                }
-            }
+            get_bmi_var_name(output_name, bmi_var_name);
 
-            double value = get_var_value_as_double(bmi_var_name);
-
-            // Convert units
-            std::string native_units = get_bmi_model()->GetVarUnits(bmi_var_name);
-            try {
-                return UnitsHelper::get_converted_value(native_units, value, output_units);
-            }
-            catch (const std::runtime_error& e){
-                std::cerr<<"Unit conversion error: "<<std::endl<<e.what()<<std::endl<<"Returning unconverted value!"<<std::endl;
-                return value;
+            if( !bmi_var_name.empty() )
+            {
+                //Get forcing value from BMI variable
+                double value = get_var_value_as_double(bmi_var_name);
+
+                // Convert units
+                std::string native_units = get_bmi_model()->GetVarUnits(bmi_var_name);
+                try {
+                    return UnitsHelper::get_converted_value(native_units, value, output_units);
+                }
+                catch (const std::runtime_error& e){
+                    std::cerr<<"Unit conversion error: "<<std::endl<<e.what()<<std::endl<<"Returning unconverted value!"<<std::endl;
+                    return value;
+                }
             }
+            //Fall back to any internal providers as a last resort.
+            return check_internal_providers<double>(output_name);
         }
 
         bool is_bmi_input_variable(const std::string &var_name) override {
@@ -451,8 +438,50 @@ namespace realization {
             return get_bmi_model()->GetOutputVarNames();
         }
 
+        /**
+         * @brief Get correct BMI variable name, which may be the output or something mapped to this output.
+         * 
+         * @param name 
+         * @param bmi_var_name 
+         */
+        inline void get_bmi_var_name(const std::string &name, std::string &bmi_var_name)
+        {
+            std::vector<std::string> output_names = get_bmi_model()->GetOutputVarNames();
+            if (std::find(output_names.begin(), output_names.end(), name) != output_names.end()) {
+                bmi_var_name = name;
+            }
+            else {
+                for (auto & iter : bmi_var_names_map) {
+                    if (iter.second == name) {
+                        bmi_var_name = iter.first;
+                        break;
+                    }
+                }
+            }
+        }
+
     protected:
 
+        /**
+         * @brief Check for implementation of internal calculators/data for a given requsted output_name
+         * 
+         * @tparam T the type expected to be returned for the value of @p output_name
+         * @param output_name 
+         * @return T
+         * @throws std::runtime_error If no known value or function for @p output_name
+         */ 
+        template <typename T>
+        inline T check_internal_providers(std::string output_name){
+            // Only use the internal et_calc() if this formulation (or possibly multi-formulation)
+            // does not know how to supply potential et
+            if (output_name == NGEN_STD_NAME_POTENTIAL_ET_FOR_TIME_STEP || output_name == CSDMS_STD_NAME_POTENTIAL_ET) {
+                return calc_et();
+            }
+            //Note, when called via get_value, this is unlikely to throw since a pre-check on available names is done
+            //in that function.
+            throw runtime_error(get_formulation_type() + " received invalid output forcing name " + output_name);
+        }
+
         /**
          * Construct model and its shared pointer, potentially supplying input variable values from config.
          *

test/CMakeLists.txt

@@ -2,6 +2,7 @@ project(test)
 
 add_subdirectory(googletest)
 include_directories(${gtest_SOURCE_DIR}/include ${gtest_SOURCE_DIR})
+include_directories(${gmock_SOURCE_DIR}/include ${gmock_SOURCE_DIR})
 include_directories(${PROJ_ROOT_INCLUDE_DIR})
 
 if (NGEN_ACTIVATE_PYTHON)

test/realizations/catchments/Bmi_C_Formulation_Test.cpp

@@ -16,16 +16,17 @@
 #include "Bmi_Module_Formulation.hpp"
 #include "Bmi_C_Formulation.hpp"
 #include "gtest/gtest.h"
+#include "gmock/gmock.h"
 #include <iostream>
 #include <vector>
-#include <regex>
 #include <boost/property_tree/ptree.hpp>
 #include <boost/property_tree/json_parser.hpp>
 #include "FileChecker.h"
 #include "Formulation_Manager.hpp"
 #include "Forcing.h"
 #include <boost/date_time.hpp>
 
+using ::testing::MatchesRegex;
 using namespace realization;
 
 class Bmi_C_Formulation_Test : public ::testing::Test {
@@ -306,8 +307,7 @@ TEST_F(Bmi_C_Formulation_Test, GetOutputLineForTimestep_0_a) {
 
     formulation.get_response(0, 3600);
     std::string output = formulation.get_output_line_for_timestep(0, ",");
-    std::regex expected ("(-?)0.000000,(-?)0.000000");
-    ASSERT_TRUE(std::regex_match(output, expected));
+    EXPECT_THAT(output, MatchesRegex("-?0.000000,-?0.000000"));
 }
 
 /** Simple test of output with modified variables. */
@@ -323,8 +323,7 @@ TEST_F(Bmi_C_Formulation_Test, GetOutputLineForTimestep_1_a) {
     // OUTPUT_VAR_1 first.
     formulation.get_response(0, 3600);
     std::string output = formulation.get_output_line_for_timestep(0, ",");
-    std::regex expected ("(-?)0.000000,(-?)0.000000");
-    ASSERT_TRUE(std::regex_match(output, expected));
+    EXPECT_THAT(output, MatchesRegex("-?0.000000,-?0.000000"));
 }
 
 /** Simple test of output with modified variables, picking time step when there was non-zero rain rate. */
@@ -339,8 +338,7 @@ TEST_F(Bmi_C_Formulation_Test, GetOutputLineForTimestep_1_b) {
         formulation.get_response(i++, 3600);
     formulation.get_response(i, 3600);
     std::string output = formulation.get_output_line_for_timestep(i, ",");
-    std::regex expected ("(-?)0.000000,0.000001");
-    ASSERT_TRUE(std::regex_match(output, expected));
+    EXPECT_THAT(output, MatchesRegex("-?0.000000,0.000001"));
 }
 
 TEST_F(Bmi_C_Formulation_Test, determine_model_time_offset_0_a) {