Merge pull request #5295 from NREL/5233_ValidateOSW

Fix #5233 - Validate OSW measures before running
NREL · Nov 11, 2024 · 8cccf80 · 8cccf80
2 parents ae56871 + 4aafb5a
commit 8cccf80
Show file tree

Hide file tree

Showing 13 changed files with 329 additions and 4 deletions.
diff --git a/resources/workflow/invalid_measures/missing_a_measure.osw b/resources/workflow/invalid_measures/missing_a_measure.osw
@@ -0,0 +1,19 @@
+{
+	"weather_file": "../../Examples/compact_osw/files/srrl_2013_amy.epw",
+	"seed_file": "../example_model.osm",
+	"measure_paths": ["../measures/"],
+	"steps": [
+		{
+			"measure_dir_name": "FakeModelMeasure",
+			"arguments": {}
+		},
+		{
+			"measure_dir_name": "NON_EXISTING_MEASURE_THIS_SHOULD_BE_CAUGHT",
+			"arguments": {}
+		},
+		{
+			"measure_dir_name": "FakeReport",
+			"arguments": {}
+		}
+	]
+}
diff --git a/resources/workflow/invalid_measures/unloadable_measure.osw b/resources/workflow/invalid_measures/unloadable_measure.osw
@@ -0,0 +1,19 @@
+{
+	"weather_file": "../../Examples/compact_osw/files/srrl_2013_amy.epw",
+	"seed_file": "../example_model.osm",
+	"measure_paths": ["../measures/"],
+	"steps": [
+		{
+			"measure_dir_name": "FakeModelMeasure",
+			"arguments": {}
+		},
+		{
+			"measure_dir_name": "UnloadableMeasure",
+			"arguments": {}
+		},
+		{
+			"measure_dir_name": "FakeReport",
+			"arguments": {}
+		}
+	]
+}
diff --git a/resources/workflow/invalid_measures/wrong_measure_type_order.osw b/resources/workflow/invalid_measures/wrong_measure_type_order.osw
@@ -0,0 +1,15 @@
+{
+	"weather_file": "../../Examples/compact_osw/files/srrl_2013_amy.epw",
+	"seed_file": "../example_model.osm",
+	"measure_paths": ["../measures/"],
+	"steps": [
+		{
+			"measure_dir_name": "FakeReport",
+			"arguments": {}
+		},
+		{
+			"measure_dir_name": "FakeModelMeasure",
+			"arguments": {}
+		}
+	]
+}
diff --git a/resources/workflow/measures/FakeModelMeasure/measure.rb b/resources/workflow/measures/FakeModelMeasure/measure.rb
@@ -0,0 +1,42 @@
+class FakeModelMeasure < OpenStudio::Measure::ModelMeasure
+  # human readable name
+  def name
+    # Measure name should be the title case of the class name.
+    return 'A dumb ModelMeasure'
+  end
+
+  # human readable description
+  def description
+    return 'Does nothing'
+  end
+
+  # human readable description of modeling approach
+  def modeler_description
+    return 'Just for testing'
+  end
+
+  # define the arguments that the user will input
+  def arguments(model)
+    args = OpenStudio::Measure::OSArgumentVector.new
+
+    return args
+  end
+
+  # define what happens when the measure is run
+  def run(model, runner, user_arguments)
+    super(model, runner, user_arguments)  # Do **NOT** remove this line
+
+    # use the built-in error checking
+    if !runner.validateUserArguments(arguments(model), user_arguments)
+      return false
+    end
+
+    # report final condition of model
+    runner.registerFinalCondition("The FakeModelMeasure run.")
+
+    return true
+  end
+end
+
+# register the measure to be used by the application
+FakeModelMeasure.new.registerWithApplication
diff --git a/resources/workflow/measures/FakeModelMeasure/measure.xml b/resources/workflow/measures/FakeModelMeasure/measure.xml
@@ -0,0 +1,44 @@
+<?xml version="1.0"?>
+<measure>
+  <schema_version>3.1</schema_version>
+  <name>fake_model_measure</name>
+  <uid>677b8fd3-2627-4516-b090-f6e47dc99fea</uid>
+  <version_id>162465e5-b6f6-419f-ab5f-c2fc4bd549e0</version_id>
+  <version_modified>2024-11-07T11:55:10Z</version_modified>
+  <xml_checksum>82D8F881</xml_checksum>
+  <class_name>FakeModelMeasure</class_name>
+  <display_name>A dumb ModelMeasure</display_name>
+  <description>Does nothing</description>
+  <modeler_description>Just for testing</modeler_description>
+  <arguments />
+  <outputs />
+  <provenances />
+  <tags>
+    <tag>Envelope.Form</tag>
+  </tags>
+  <attributes>
+    <attribute>
+      <name>Measure Type</name>
+      <value>ModelMeasure</value>
+      <datatype>string</datatype>
+    </attribute>
+    <attribute>
+      <name>Measure Language</name>
+      <value>Ruby</value>
+      <datatype>string</datatype>
+    </attribute>
+  </attributes>
+  <files>
+    <file>
+      <version>
+        <software_program>OpenStudio</software_program>
+        <identifier>3.9.0</identifier>
+        <min_compatible>3.9.0</min_compatible>
+      </version>
+      <filename>measure.rb</filename>
+      <filetype>rb</filetype>
+      <usage_type>script</usage_type>
+      <checksum>DDA977B0</checksum>
+    </file>
+  </files>
+</measure>
diff --git a/resources/workflow/measures/UnloadableMeasure/README.md b/resources/workflow/measures/UnloadableMeasure/README.md
@@ -0,0 +1 @@
+This doesnt have a xml
diff --git a/resources/workflow/measures/UnloadableMeasure/measure.rb b/resources/workflow/measures/UnloadableMeasure/measure.rb
diff --git a/src/cli/CMakeLists.txt b/src/cli/CMakeLists.txt
@@ -270,6 +270,63 @@ if(BUILD_TESTING)
     PASS_REGULAR_EXPRESSION "HI FROM ERB PYTHON PLUGIN[\r\n\t ]*HI FROM JINJA PYTHON PLUGIN"
   )
 
+  # ======================== Workflows should fail ========================
+  add_test(NAME OpenStudioCLI.Run_Validate.MissingAMeasure
+    COMMAND $<TARGET_FILE:openstudio> run --show-stdout -w missing_a_measure.osw
+    WORKING_DIRECTORY "${PROJECT_BINARY_DIR}/resources/workflow/invalid_measures/"
+  )
+  set_tests_properties(OpenStudioCLI.Run_Validate.MissingAMeasure PROPERTIES
+    WILL_FAIL TRUE
+    RESOURCE_LOCK "invalid_measures"
+  )
+
+  add_test(NAME OpenStudioCLI.Run_Validate.UnloadableMeasure
+    COMMAND $<TARGET_FILE:openstudio> run --show-stdout -w unloadable_measure.osw
+    WORKING_DIRECTORY "${PROJECT_BINARY_DIR}/resources/workflow/invalid_measures/"
+  )
+  set_tests_properties(OpenStudioCLI.Run_Validate.UnloadableMeasure PROPERTIES
+    WILL_FAIL TRUE
+    RESOURCE_LOCK "invalid_measures"
+  )
+
+  add_test(NAME OpenStudioCLI.Run_Validate.WrongMeasureTypeOrder
+    COMMAND $<TARGET_FILE:openstudio> run --show-stdout -w wrong_measure_type_order.osw
+    WORKING_DIRECTORY "${PROJECT_BINARY_DIR}/resources/workflow/invalid_measures/"
+  )
+  set_tests_properties(OpenStudioCLI.Run_Validate.WrongMeasureTypeOrder PROPERTIES
+    WILL_FAIL TRUE
+    RESOURCE_LOCK "invalid_measures"
+  )
+
+  # Classic
+  add_test(NAME OpenStudioCLI.Classic.Run_Validate.MissingAMeasure
+    COMMAND $<TARGET_FILE:openstudio> classic run --show-stdout -w missing_a_measure.osw
+    WORKING_DIRECTORY "${PROJECT_BINARY_DIR}/resources/workflow/invalid_measures/"
+  )
+  set_tests_properties(OpenStudioCLI.Classic.Run_Validate.MissingAMeasure PROPERTIES
+    WILL_FAIL TRUE
+    RESOURCE_LOCK "invalid_measures"
+  )
+
+  add_test(NAME OpenStudioCLI.Classic.Run_Validate.UnloadableMeasure
+    COMMAND $<TARGET_FILE:openstudio> classic run --show-stdout -w unloadable_measure.osw
+    WORKING_DIRECTORY "${PROJECT_BINARY_DIR}/resources/workflow/invalid_measures/"
+  )
+  set_tests_properties(OpenStudioCLI.Classic.Run_Validate.UnloadableMeasure PROPERTIES
+    WILL_FAIL TRUE
+    RESOURCE_LOCK "invalid_measures"
+  )
+
+  add_test(NAME OpenStudioCLI.Classic.Run_Validate.WrongMeasureTypeOrder
+    COMMAND $<TARGET_FILE:openstudio> classic run --show-stdout -w wrong_measure_type_order.osw
+    WORKING_DIRECTORY "${PROJECT_BINARY_DIR}/resources/workflow/invalid_measures/"
+  )
+  set_tests_properties(OpenStudioCLI.Classic.Run_Validate.WrongMeasureTypeOrder PROPERTIES
+    WILL_FAIL TRUE
+    RESOURCE_LOCK "invalid_measures"
+  )
+  # ====================== End Workflows should fail ======================
+
   if (Pytest_AVAILABLE)
     add_test(NAME OpenStudioCLI.test_loglevel
       COMMAND ${Python_EXECUTABLE} -m pytest --verbose ${Pytest_XDIST_OPTS} --os-cli-path $<TARGET_FILE:openstudio> "${CMAKE_CURRENT_SOURCE_DIR}/test/test_loglevel.py"

diff --git a/src/utilities/filetypes/WorkflowJSON.cpp b/src/utilities/filetypes/WorkflowJSON.cpp
@@ -46,10 +46,12 @@ namespace detail {
     std::string formattedErrors;
     bool parsingSuccessful = Json::parseFromStream(rbuilder, ss, &m_value, &formattedErrors);
 
+    openstudio::path p;
+
     if (!parsingSuccessful) {
 
       // see if this is a path
-      openstudio::path p = toPath(s);
+      p = toPath(s);
       if (boost::filesystem::exists(p) && boost::filesystem::is_regular_file(p)) {
         // open file
         std::ifstream ifs(openstudio::toSystemFilename(p));
@@ -65,6 +67,10 @@ namespace detail {
 
     parseSteps();
     parseRunOptions();
+
+    if (!p.empty()) {
+      setOswPath(p, false);
+    }
   }
 
   WorkflowJSON_Impl::WorkflowJSON_Impl(const openstudio::path& p) {
@@ -851,6 +857,65 @@ namespace detail {
     }
   }
 
+  bool WorkflowJSON_Impl::validateMeasures() const {
+    // TODO: should we exit early, or return all problems found?
+
+    bool result = true;
+    MeasureType state = MeasureType::ModelMeasure;
+
+    for (size_t i = 0; const auto& step : m_steps) {
+      LOG(Debug, "Validating step " << i);
+      if (auto step_ = step.optionalCast<MeasureStep>()) {
+        // Not calling getBCLMeasure because I want to mimic workflow-gem and be as explicit as possible about what went wrong
+        const auto measureDirName = step_->measureDirName();
+        auto measurePath_ = findMeasure(measureDirName);
+        if (!measurePath_) {
+          LOG(Error, "Cannot find measure '" << measureDirName << "'");
+          result = false;
+          continue;
+        }
+        auto bclMeasure_ = BCLMeasure::load(*measurePath_);
+        if (!bclMeasure_) {
+          LOG(Error, "Cannot load measure '" << measureDirName << "' at '" << *measurePath_ << "'");
+          result = false;
+          continue;
+        }
+
+        // Ensure that measures are in order, i.e. no OS after E+, E+ or OS after Reporting
+        const auto measureType = bclMeasure_->measureType();
+
+        if (measureType == MeasureType::ModelMeasure) {
+          if (state == MeasureType::EnergyPlusMeasure) {
+            LOG(Error, "OpenStudio measure '" << measureDirName << "' called after transition to EnergyPlus.");
+            result = false;
+          }
+          if (state == MeasureType::ReportingMeasure) {
+            LOG(Error, "OpenStudio measure '" << measureDirName << "' called after Energyplus simulation.");
+            result = false;
+          }
+
+        } else if (measureType == MeasureType::EnergyPlusMeasure) {
+          if (state == MeasureType::ReportingMeasure) {
+            LOG(Error, "EnergyPlus measure '" << measureDirName << "' called after Energyplus simulation.");
+            result = false;
+          }
+          if (state == MeasureType::ModelMeasure) {
+            state = MeasureType::EnergyPlusMeasure;
+          }
+
+        } else if (measureType == MeasureType::ReportingMeasure) {
+          state = MeasureType::ReportingMeasure;
+
+        } else {
+          LOG(Error, "MeasureType " << measureType.valueName() << " of measure '" << measureDirName << "' is not supported");
+          result = false;
+        }
+      }
+      ++i;
+    }
+
+    return result;
+  }
 }  // namespace detail
 
 WorkflowJSON::WorkflowJSON() : m_impl(std::shared_ptr<detail::WorkflowJSON_Impl>(new detail::WorkflowJSON_Impl())) {}
@@ -1123,6 +1188,10 @@ void WorkflowJSON::resetRunOptions() {
   getImpl<detail::WorkflowJSON_Impl>()->resetRunOptions();
 }
 
+bool WorkflowJSON::validateMeasures() const {
+  return getImpl<detail::WorkflowJSON_Impl>()->validateMeasures();
+}
+
 std::ostream& operator<<(std::ostream& os, const WorkflowJSON& workflowJSON) {
   os << workflowJSON.string();
   return os;

diff --git a/src/utilities/filetypes/WorkflowJSON.hpp b/src/utilities/filetypes/WorkflowJSON.hpp
@@ -233,6 +233,9 @@ class UTILITIES_API WorkflowJSON
   /** Reset RunOptions for this workflow. */
   void resetRunOptions();
 
+  /** Checks that all measures in the Workflow can be found, and are in the correct order (ModelMeasure > EnergyPlusMeasure > ReportingMeasure) */
+  bool validateMeasures() const;
+
  protected:
   // get the impl
   template <typename T>

diff --git a/src/utilities/filetypes/WorkflowJSON_Impl.hpp b/src/utilities/filetypes/WorkflowJSON_Impl.hpp
@@ -153,6 +153,8 @@ namespace detail {
     // Emitted on any change
     Nano::Signal<void()> onChange;
 
+    bool validateMeasures() const;
+
    private:
     REGISTER_LOGGER("openstudio.WorkflowJSON");
 

diff --git a/src/utilities/filetypes/test/WorkflowJSON_GTest.cpp b/src/utilities/filetypes/test/WorkflowJSON_GTest.cpp
@@ -15,9 +15,11 @@
 
 #include "../../time/DateTime.hpp"
 
+#include "../../core/Filesystem.hpp"
 #include "../../core/Exception.hpp"
 #include "../../core/System.hpp"
 #include "../../core/Checksum.hpp"
+#include "../../core/StringStreamLogSink.hpp"
 
 #include <resources.hxx>
 
@@ -1436,3 +1438,48 @@ TEST(Filetypes, RunOptions_overrideValuesWith) {
   ASSERT_FALSE(ftOptions.excludeSpaceTranslation());
   ASSERT_TRUE(ftOptions.isExcludeSpaceTranslationDefaulted());
 }
+
+TEST(Filetypes, WorkflowJSON_ValidateMeasures_Ok) {
+  auto p = resourcesPath() / toPath("utilities/Filetypes/full.osw");
+  WorkflowJSON w(p);
+  EXPECT_TRUE(w.validateMeasures());
+}
+
+TEST(Filetypes, WorkflowJSON_ValidateMeasures_Missing) {
+  auto p = resourcesPath() / toPath("workflow/invalid_measures/missing_a_measure.osw");
+  ASSERT_TRUE(boost::filesystem::is_regular_file(p));
+  WorkflowJSON w(p);
+  StringStreamLogSink sink;
+  sink.setLogLevel(Error);
+  EXPECT_FALSE(w.validateMeasures());
+  ASSERT_EQ(1, sink.logMessages().size());
+  EXPECT_EQ("Cannot find measure 'NON_EXISTING_MEASURE_THIS_SHOULD_BE_CAUGHT'", sink.logMessages()[0].logMessage());
+}
+
+TEST(Filetypes, WorkflowJSON_ValidateMeasures_Unloadable) {
+  auto p = resourcesPath() / toPath("workflow/invalid_measures/unloadable_measure.osw");
+  ASSERT_TRUE(boost::filesystem::is_regular_file(p));
+  WorkflowJSON w(p);
+  StringStreamLogSink sink;
+  sink.setLogLevel(Error);
+  EXPECT_FALSE(w.validateMeasures());
+  auto logMessages = sink.logMessages();
+  ASSERT_EQ(3, logMessages.size());
+  EXPECT_EQ("utilities.bcl.BCLXML", logMessages.at(0).logChannel());
+  EXPECT_EQ("utilities.bcl.BCLMeasure", logMessages.at(1).logChannel());
+  EXPECT_EQ("openstudio.WorkflowJSON", logMessages.at(2).logChannel());
+  auto logMessage = sink.logMessages()[2].logMessage();
+  EXPECT_TRUE(logMessage.find("Cannot load measure 'UnloadableMeasure' at '") != std::string::npos) << logMessage;
+}
+
+TEST(Filetypes, WorkflowJSON_ValidateMeasures_WrongOrder) {
+  auto p = resourcesPath() / toPath("workflow/invalid_measures/wrong_measure_type_order.osw");
+  ASSERT_TRUE(boost::filesystem::is_regular_file(p));
+  WorkflowJSON w(p);
+  StringStreamLogSink sink;
+  sink.setLogLevel(Error);
+  EXPECT_FALSE(w.validateMeasures());
+  ASSERT_EQ(1, sink.logMessages().size());
+
+  EXPECT_EQ("OpenStudio measure 'FakeModelMeasure' called after Energyplus simulation.", sink.logMessages()[0].logMessage());
+}