From 6d2d0c6347a63dd6f98286425bd893dbb5d03519 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Thu, 9 Nov 2023 18:03:11 +0100 Subject: [PATCH 01/10] WIP For #5019 --- src/workflow/ApplyMeasure.cpp | 22 +++++++++++++++--- src/workflow/OSWorkflow.cpp | 31 ++++++++++++++++++++++++++ src/workflow/OSWorkflow.hpp | 8 +++++++ src/workflow/RunEnergyPlusMeasures.cpp | 1 + src/workflow/RunOpenStudioMeasures.cpp | 1 + src/workflow/RunReportingMeasures.cpp | 1 + 6 files changed, 61 insertions(+), 3 deletions(-) diff --git a/src/workflow/ApplyMeasure.cpp b/src/workflow/ApplyMeasure.cpp index ecce2a7d008..36a0feb8999 100644 --- a/src/workflow/ApplyMeasure.cpp +++ b/src/workflow/ApplyMeasure.cpp @@ -86,6 +86,9 @@ void OSWorkflow::applyMeasures(MeasureType measureType, bool energyplus_output_r runner.incrementStep(); result.setStepResult(StepResult::Skip); } + + // Technically here I would need to have gotten className from the measure to match workflow-gem, just to set applicable = false + output_attributes[step.name().value_or(measureDirName)]["applicable"] = openstudio::Variant(false); } continue; } @@ -303,9 +306,7 @@ end // if doing output requests we are done now if (!energyplus_output_requests) { WorkflowStepResult result = runner.result(); - if (auto stepResult_ = result.stepResult()) { - LOG(Debug, "Step Result: " << stepResult_->valueName()); - } + // incrementStep must be called after run runner.incrementStep(); if (auto errors = result.stepErrors(); !errors.empty()) { @@ -313,6 +314,21 @@ end throw std::runtime_error(fmt::format("Measure {} reported an error with [{}]", measureDirName, fmt::join(errors, "\n"))); } + const auto measureName = step.name().value_or(className); + auto& measureAttributes = output_attributes[measureName]; + for (const auto& stepValue : result.stepValues()) { + measureAttributes[stepValue.name()] = stepValue.valueAsVariant(); + } + auto stepResult_ = result.stepResult(); + if (!stepResult_.has_value()) { + LOG_AND_THROW("Step Result not set for " << scriptPath_->generic_string()); + } + + // Add an applicability flag to all the measure results + StepResult stepResult = std::move(*stepResult_); + LOG(Debug, "Step Result: " << stepResult.valueName()); + measureAttributes["applicable"] = openstudio::Variant(!((stepResult == StepResult::NA) || (stepResult == StepResult::Skip))); + if (measureType == MeasureType::ModelMeasure) { updateLastWeatherFileFromModel(); } diff --git a/src/workflow/OSWorkflow.cpp b/src/workflow/OSWorkflow.cpp index 0ed93ab8bed..2b9b5f9584e 100644 --- a/src/workflow/OSWorkflow.cpp +++ b/src/workflow/OSWorkflow.cpp @@ -455,4 +455,35 @@ bool OSWorkflow::run() { } return (state == State::Finished); } + +void OSWorkflow::communicateMeasureAttributes() const { + + Json::Value root(Json::objectValue); + for (const auto& [measureName, argMap] : output_attributes) { + Json::Value measureValues(Json::objectValue); + for (const auto& [argName, variantValue] : argMap) { + if (variantValue.variantType() == VariantType::String) { + measureValues[argName] = variantValue.valueAsString(); + } else if (variantValue.variantType() == VariantType::Double) { + measureValues[argName] = variantValue.valueAsDouble(); + } else if (variantValue.variantType() == VariantType::Integer) { + measureValues[argName] = variantValue.valueAsInteger(); + } else if (variantValue.variantType() == VariantType::Boolean) { + measureValues[argName] = variantValue.valueAsBoolean(); + } + } + root[measureName] = measureValues; + } + Json::StreamWriterBuilder wbuilder; + // mimic the old StyledWriter behavior: + wbuilder["indentation"] = " "; + const std::string result = Json::writeString(wbuilder, root); + + auto jsonPath = workflowJSON.absoluteRunDir() / "measure_attributes.json"; + openstudio::filesystem::ofstream file(jsonPath); + OS_ASSERT(file.is_open()); + file << result; + file.close(); +} + } // namespace openstudio diff --git a/src/workflow/OSWorkflow.hpp b/src/workflow/OSWorkflow.hpp index e3ec4d86474..2cca4fdf0b7 100644 --- a/src/workflow/OSWorkflow.hpp +++ b/src/workflow/OSWorkflow.hpp @@ -15,6 +15,7 @@ #include "../utilities/filetypes/WorkflowJSON.hpp" #include +#include #include #define USE_RUBY_ENGINE 1 @@ -56,6 +57,10 @@ class OSWorkflow // TODO: use a unique_ptr or an Instance? std::unique_ptr m_timers = nullptr; + // TODO: should problably store as json directly... + // { measureName : { arg_name: arg_value }} + std::map> output_attributes; + bool m_no_simulation = false; bool m_post_process_only = false; @@ -122,6 +127,9 @@ class OSWorkflow static void applyArguments(measure::OSArgumentMap& argumentMap, const std::string& argumentName, const openstudio::Variant& argumentValue); void saveOSMToRootDirIfDebug(); void saveIDFToRootDirIfDebug(); + + // write output_attributes to the measure_attributes.json + void communicateMeasureAttributes() const; }; } // namespace openstudio diff --git a/src/workflow/RunEnergyPlusMeasures.cpp b/src/workflow/RunEnergyPlusMeasures.cpp index cf5455a16f5..08c2f5a6856 100644 --- a/src/workflow/RunEnergyPlusMeasures.cpp +++ b/src/workflow/RunEnergyPlusMeasures.cpp @@ -22,6 +22,7 @@ void OSWorkflow::runEnergyPlusMeasures() { applyMeasures(MeasureType::EnergyPlusMeasure, false); LOG(Info, "Finished applying EnergyPlus Measures."); + communicateMeasureAttributes(); saveIDFToRootDirIfDebug(); } } // namespace openstudio diff --git a/src/workflow/RunOpenStudioMeasures.cpp b/src/workflow/RunOpenStudioMeasures.cpp index ab650295598..50d2961261a 100644 --- a/src/workflow/RunOpenStudioMeasures.cpp +++ b/src/workflow/RunOpenStudioMeasures.cpp @@ -32,6 +32,7 @@ void OSWorkflow::runOpenStudioMeasures() { }); } + communicateMeasureAttributes(); saveOSMToRootDirIfDebug(); } diff --git a/src/workflow/RunReportingMeasures.cpp b/src/workflow/RunReportingMeasures.cpp index 64cc286ac54..c28c873bb62 100644 --- a/src/workflow/RunReportingMeasures.cpp +++ b/src/workflow/RunReportingMeasures.cpp @@ -57,6 +57,7 @@ void OSWorkflow::runReportingMeasures() { applyMeasures(MeasureType::ReportingMeasure, false); LOG(Info, "Finished applying Reporting Measures."); + communicateMeasureAttributes(); // TODO // # Parse the files generated by the local output adapter // results, objective_functions = run_extract_inputs_and_outputs @registry[:run_dir], @logger From 7d528b10af8443aac6304ae54db4b9600684f774 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 13 Nov 2023 16:23:30 +0100 Subject: [PATCH 02/10] Port rename_hash_keys from workflow gem util/post_process::run_extract_inputs_and_outputs Add some testing. Too bad the original ruby implementation has zero tests, it would have save me the trouble of creating it. --- src/workflow/CMakeLists.txt | 14 +++++++ src/workflow/Util.cpp | 22 ++++++++++ src/workflow/Util.hpp | 7 ++++ src/workflow/test/Util_GTest.cpp | 70 ++++++++++++++++++++++++++++++++ 4 files changed, 113 insertions(+) create mode 100644 src/workflow/test/Util_GTest.cpp diff --git a/src/workflow/CMakeLists.txt b/src/workflow/CMakeLists.txt index ef4ea3a9911..23e406f23ec 100644 --- a/src/workflow/CMakeLists.txt +++ b/src/workflow/CMakeLists.txt @@ -26,3 +26,17 @@ add_library(openstudio_workflow ) target_link_libraries(openstudio_workflow PRIVATE openstudiolib) + +if(BUILD_TESTING) + set(openstudio_workflow_test_depends + openstudio_workflow + CONAN_PKG::boost # Maybe at some point replace with openstudiolib more simply + CONAN_PKG::fmt + ) + + set(openstudio_workflow_test_src + test/Util_GTest.cpp + ) + + CREATE_TEST_TARGETS(openstudio_workflow "${openstudio_workflow_test_src}" "${openstudio_workflow_test_depends}") +endif() diff --git a/src/workflow/Util.cpp b/src/workflow/Util.cpp index 1c20592acea..ab0a75e4ee8 100644 --- a/src/workflow/Util.cpp +++ b/src/workflow/Util.cpp @@ -30,7 +30,10 @@ #include #include #include +#include #include +#include +#include namespace openstudio::workflow::util { @@ -303,4 +306,23 @@ void zipResults(const openstudio::path& dirPath) { } } +std::string sanitizeKey(std::string key) { + static const std::regex invalidCharsRegex(R"([|!@#$%^&*(){}\\[\];:'",<.>/?+=]+)"); + static const std::regex squeezeUnderscoresRegex(R"(_{2,})"); + static const std::regex trailingUnderscoreOrWhiteSpaceRegex(R"((_|\s)+$)"); + + if (std::regex_search(key, invalidCharsRegex)) { + LOG_FREE(Warn, "openstudio.worklow.Util", fmt::format("Renaming result key '{}' to remove invalid characters", key)); + // Replace invalid characters with underscores + key = std::regex_replace(key, invalidCharsRegex, "_"); + } + + // Squeeze consecutive underscores + key = std::regex_replace(key, squeezeUnderscoresRegex, "_"); + // Strip trailing underscores or whitespace + key = std::regex_replace(key, trailingUnderscoreOrWhiteSpaceRegex, ""); + + return key; +} + } // namespace openstudio::workflow::util diff --git a/src/workflow/Util.hpp b/src/workflow/Util.hpp index 3201cc98b04..fb8fc6ef617 100644 --- a/src/workflow/Util.hpp +++ b/src/workflow/Util.hpp @@ -33,6 +33,13 @@ namespace workflow { void zipResults(const openstudio::path& dirPath); + /** Remove any invalid characters in the measure attribute keys. Periods and Pipes are the most problematic + * because MongoDB does not allow hash keys with periods, and pipes are used in the map/reduce method that + * was written to speed up the data write in OpenStudio-Server. Also, remove any trailing underscores and spaces. + * + * Ported from workflow-gem rename_hash_keys */ + std::string sanitizeKey(std::string key); + } // namespace util } // namespace workflow } // namespace openstudio diff --git a/src/workflow/test/Util_GTest.cpp b/src/workflow/test/Util_GTest.cpp new file mode 100644 index 00000000000..9d931d8e824 --- /dev/null +++ b/src/workflow/test/Util_GTest.cpp @@ -0,0 +1,70 @@ +/*********************************************************************************************************************** +* OpenStudio(R), Copyright (c) Alliance for Sustainable Energy, LLC. +* See also https://openstudio.net/license +***********************************************************************************************************************/ + +#include + +#include "../Util.hpp" + +#include +#include + +class WorkflowFixture : public testing::Test +{ +}; + +TEST_F(WorkflowFixture, Util_sanitizeKey) { + + const std::map hash_with_sanitized_keys = { + {"Regular", "Regular"}, + {"key1!@#", "key1"}, + {"key2.{}\\", "key2"}, + {"key3_ ", "key3"}, + {"invalid|key", "invalid_key"}, + {"another key", "another key"}, + {"special@char", "special_char"}, + {"key.with.dots", "key_with_dots"}, + {"key with spaces", "key with spaces"}, + {"slashes\\/included", "slashes_included"}, + {"pipe|character", "pipe_character"}, + {"!exclamation", "_exclamation"}, + {"@at_symbol", "_at_symbol"}, + {"#hash_symbol", "_hash_symbol"}, + {"$dollar_sign", "_dollar_sign"}, + {"%percent_sign", "_percent_sign"}, + {"^caret", "_caret"}, + {"&ersand", "_ampersand"}, + {"*asterisk", "_asterisk"}, + {"(open_parenthesis)", "_open_parenthesis"}, + {"{open_brace}", "_open_brace"}, + {"}close_brace{", "_close_brace"}, + {"\\backslash\\", "_backslash"}, + {"[open_square]", "_open_square"}, + {"]close_square", "_close_square"}, + {";semicolon;", "_semicolon"}, + {":colon:", "_colon"}, + {"'single_quote'", "_single_quote"}, + {"\"double_quote\"", "_double_quote"}, + {",comma,", "_comma"}, + {"", "_less_than"}, + {".period.", "_period"}, + {">greater_than<", "_greater_than"}, + {"/forward_slash/", "_forward_slash"}, + {"?question_mark?", "_question_mark"}, + {"=equal_sign=", "_equal_sign"}, + {"+plus_sign+", "_plus_sign"}, + {"___", ""}, // Consecutive underscores to be squeezed + {"key_with___underscores", "key_with_underscores"}, + {"key_with__underscores", "key_with_underscores"}, + {"key_with_underscores", "key_with_underscores"}, + {"__underscored__key__", "_underscored_key"}, // Multiple underscores within the key + {"___double___underscores___", "_double_underscores"} // Multiple consecutive underscores + }; + + for (const auto& [originalKey, expectedKey] : hash_with_sanitized_keys) { + const std::string sanitizedKey = openstudio::workflow::util::sanitizeKey(originalKey); + // Check if the sanitized key matches the expected value + EXPECT_EQ(sanitizedKey, expectedKey) << "Error: Key '" << originalKey << "' was not sanitized as expected."; + } +} From 8e6df5a9d6101963a72fd40084ca59ecb0e28128 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 13 Nov 2023 18:26:59 +0100 Subject: [PATCH 03/10] Start a pytest for running classic / labs in an analysis.json case --- .../measures/Standard Reports/measure.rb | 9 ++++ .../Examples/with_analysis/analysis.json | 50 ++++++++++++++++++ .../with_analysis/local/with_analysis.osw | 12 +++++ src/cli/CMakeLists.txt | 6 +++ src/cli/test/test_with_analysis.py | 51 +++++++++++++++++++ 5 files changed, 128 insertions(+) create mode 100644 resources/Examples/with_analysis/analysis.json create mode 100644 resources/Examples/with_analysis/local/with_analysis.osw create mode 100644 src/cli/test/test_with_analysis.py diff --git a/resources/Examples/compact_osw/measures/Standard Reports/measure.rb b/resources/Examples/compact_osw/measures/Standard Reports/measure.rb index b03ff76c0c4..34254c95600 100644 --- a/resources/Examples/compact_osw/measures/Standard Reports/measure.rb +++ b/resources/Examples/compact_osw/measures/Standard Reports/measure.rb @@ -16,6 +16,12 @@ def arguments(model = nil) return args end #end the arguments method + def outputs + result = OpenStudio::Measure::OSOutputVector.new + result << OpenStudio::Measure::OSOutput.makeDoubleOutput('net_site_energy', false) + return result + end + #define what happens when the measure is run def run(runner, user_arguments) super(runner, user_arguments) @@ -130,6 +136,9 @@ def run(runner, user_arguments) end end + runner.registerValue("net_site_energy", "Net Site Energy", sqlFile.netSiteEnergy.get, "GJ") + runner.registerValue("something!with.invalid_chars_", "Test Sanitizing", 1, "") + #closing the sql file sqlFile.close() diff --git a/resources/Examples/with_analysis/analysis.json b/resources/Examples/with_analysis/analysis.json new file mode 100644 index 00000000000..e07c29956ee --- /dev/null +++ b/resources/Examples/with_analysis/analysis.json @@ -0,0 +1,50 @@ +{ + "analysis": { + "display_name": "Test With Analysis JSON", + "name": "analysis_json", + "output_variables": [ + { + "objective_function": true, + "name": "StandardReports.net_site_energy", + "objective_function_index": 0, + "objective_function_target": 0, + "objective_function_group": 1, + "scaling_factor": 1.0, + "display_name": "Net Site Energy, should be there", + "display_name_short": "net_site_energy", + "metadata_id": null, + "visualize": true, + "export": true, + "variable_type": "double" + }, + { + "objective_function": false, + "name": "StandardReports.net_site_energy", + "objective_function_index": 1, + "objective_function_target": 0, + "objective_function_group": 1, + "scaling_factor": 1.0, + "display_name": "net_site_energy", + "display_name_short": "net_site_energy", + "metadata_id": null, + "visualize": true, + "export": true, + "variable_type": "double" + }, + { + "objective_function": true, + "name": "IsNonExisting.NonExisting", + "objective_function_index": 2, + "objective_function_target": 0, + "objective_function_group": 1, + "scaling_factor": 1.0, + "display_name": "net_site_energy", + "display_name_short": "net_site_energy", + "metadata_id": null, + "visualize": true, + "export": true, + "variable_type": "double" + } + ] + } +} diff --git a/resources/Examples/with_analysis/local/with_analysis.osw b/resources/Examples/with_analysis/local/with_analysis.osw new file mode 100644 index 00000000000..61fa84b7a3c --- /dev/null +++ b/resources/Examples/with_analysis/local/with_analysis.osw @@ -0,0 +1,12 @@ +{ + "file_paths": ["../../compact_osw/files/"], + "measure_paths": ["../../compact_osw/measures/"], + "weather_file": "srrl_2013_amy.epw", + "seed_file": "seb.osm", + "steps": [ + { + "measure_dir_name": "Standard Reports", + "arguments": {} + } + ] +} diff --git a/src/cli/CMakeLists.txt b/src/cli/CMakeLists.txt index dced056736a..a2deb5701bb 100644 --- a/src/cli/CMakeLists.txt +++ b/src/cli/CMakeLists.txt @@ -242,6 +242,12 @@ if(BUILD_TESTING) add_test(NAME OpenStudioCLI.test_loglevel COMMAND ${Python_EXECUTABLE} -m pytest --verbose ${Pytest_XDIST_OPTS} --os-cli-path $ "${CMAKE_CURRENT_SOURCE_DIR}/test/test_loglevel.py" ) + + # No Xdist on purpose here + add_test(NAME OpenStudioCLI.test_with_analysis + COMMAND ${Python_EXECUTABLE} -m pytest --verbose --os-cli-path $ "${CMAKE_CURRENT_SOURCE_DIR}/test/test_with_analysis.py" + WORKING_DIRECTORY "${PROJECT_BINARY_DIR}/resources/Examples/with_analysis/local/" + ) else() # TODO: Remove. Fallback on these for now, as I don't know if CI has pytest installed add_test(NAME OpenStudioCLI.Classic.test_logger_rb diff --git a/src/cli/test/test_with_analysis.py b/src/cli/test/test_with_analysis.py new file mode 100644 index 00000000000..99d2cc87d82 --- /dev/null +++ b/src/cli/test/test_with_analysis.py @@ -0,0 +1,51 @@ +import json +import subprocess +from pathlib import Path + +import pytest + + +@pytest.mark.parametrize( + "is_labs", + [pytest.param(True, id="labs"), pytest.param(False, id="classic")], +) +def test_run_with_analysis(osclipath, is_labs: bool): + osw_path = Path("with_analysis.osw").resolve() + assert osw_path.is_file(), f"{osw_path=} is not found" + + command = [str(osclipath)] + if not is_labs: + command.append("classic") + command += ["run", "-w", str(osw_path)] + lines = subprocess.check_output(command, encoding="utf-8").splitlines() + + runDir = Path("./run") + assert runDir.exists() + measure_attributes_path = runDir / "measure_attributes.json" + assert measure_attributes_path.is_file() + results_path = runDir / "results.json" + assert results_path.is_file() + objectives_path = runDir / "objectives.json" + assert objectives_path.is_file() + + measure_attributes = json.loads(measure_attributes_path.read_text()) + assert measure_attributes == { + "StandardReports": {"applicable": True, "net_site_energy": 167.1, "something_with_invalid_chars": 1} + } + + results = json.loads(results_path.read_text()) + assert results == { + "StandardReports": {"applicable": True, "net_site_energy": 167.1, "something_with_invalid_chars": 1} + } + + objectives = json.loads(objectives_path.read_text()) + assert objectives == { + "objective_function_1": 167.1, + "objective_function_3": 1.7976931348623157e308, + "objective_function_group_1": 1.0, + "objective_function_group_3": None, + "objective_function_target_1": 0.0, + "objective_function_target_3": None, + "scaling_factor_1": 1.0, + "scaling_factor_3": None, + } From d34d1379c9299a51387ed653b6e1c50aadf4f396 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 13 Nov 2023 18:28:52 +0100 Subject: [PATCH 04/10] Use a measure that doesn't need a model/sql so testing goes much faster with --postprocess_only (and it tests that flag too!) --- .../Examples/with_analysis/analysis.json | 4 +- .../local/measures/FakeReport/measure.rb | 43 ++++++++++++++ .../local/measures/FakeReport/measure.xml | 57 +++++++++++++++++++ .../with_analysis/local/with_analysis.osw | 6 +- src/cli/test/test_with_analysis.py | 23 ++++++-- 5 files changed, 121 insertions(+), 12 deletions(-) create mode 100644 resources/Examples/with_analysis/local/measures/FakeReport/measure.rb create mode 100644 resources/Examples/with_analysis/local/measures/FakeReport/measure.xml diff --git a/resources/Examples/with_analysis/analysis.json b/resources/Examples/with_analysis/analysis.json index e07c29956ee..3276efcd156 100644 --- a/resources/Examples/with_analysis/analysis.json +++ b/resources/Examples/with_analysis/analysis.json @@ -5,7 +5,7 @@ "output_variables": [ { "objective_function": true, - "name": "StandardReports.net_site_energy", + "name": "FakeReport.net_site_energy", "objective_function_index": 0, "objective_function_target": 0, "objective_function_group": 1, @@ -19,7 +19,7 @@ }, { "objective_function": false, - "name": "StandardReports.net_site_energy", + "name": "FakeReport.net_site_energy", "objective_function_index": 1, "objective_function_target": 0, "objective_function_group": 1, diff --git a/resources/Examples/with_analysis/local/measures/FakeReport/measure.rb b/resources/Examples/with_analysis/local/measures/FakeReport/measure.rb new file mode 100644 index 00000000000..1cbbede8e44 --- /dev/null +++ b/resources/Examples/with_analysis/local/measures/FakeReport/measure.rb @@ -0,0 +1,43 @@ +require 'openstudio' + +class FakeReport < OpenStudio::Measure::ReportingMeasure + + def name + return "Fake Report" + end + + #define the arguments that the user will input + def arguments(model = nil) + args = OpenStudio::Measure::OSArgumentVector.new + + return args + end #end the arguments method + + def outputs + result = OpenStudio::Measure::OSOutputVector.new + result << OpenStudio::Measure::OSOutput.makeDoubleOutput('net_site_energy', false) + return result + end + + #define what happens when the measure is run + def run(runner, user_arguments) + super(runner, user_arguments) + + #use the built-in error checking + if not runner.validateUserArguments(arguments(), user_arguments) + return false + end + + # Register some constant values, so we can only do it during post + # processing + runner.registerValue("net_site_energy", "Net Site Energy", 167.1, "GJ") + runner.registerValue("something!with.invalid_chars_", "Test Sanitizing", 1, "") + runner.registerFinalCondition("Goodbye.") + + return true + + end + +end + +FakeReport.new.registerWithApplication diff --git a/resources/Examples/with_analysis/local/measures/FakeReport/measure.xml b/resources/Examples/with_analysis/local/measures/FakeReport/measure.xml new file mode 100644 index 00000000000..4723800902f --- /dev/null +++ b/resources/Examples/with_analysis/local/measures/FakeReport/measure.xml @@ -0,0 +1,57 @@ + + + 3.1 + fake_report + ca6ba362-ea57-4236-b803-17e37b0c0817 + 32617b1d-91b1-4325-9a3a-0708b9853d29 + 2023-11-13T17:08:41Z + B2AD275E + FakeReport + Fake Report + Change me + Change me + + + + net_site_energy + net_site_energy + net_site_energy + Double + false + + + + + Reporting.QAQC + + + + Measure Type + ReportingMeasure + string + + + Measure Language + Ruby + string + + + Uses SketchUp API + false + boolean + + + + + + OpenStudio + 1.1.2 + 1.1.2 + + measure.rb + rb + script + 4CFB5158 + + + diff --git a/resources/Examples/with_analysis/local/with_analysis.osw b/resources/Examples/with_analysis/local/with_analysis.osw index 61fa84b7a3c..45f7c791883 100644 --- a/resources/Examples/with_analysis/local/with_analysis.osw +++ b/resources/Examples/with_analysis/local/with_analysis.osw @@ -1,11 +1,7 @@ { - "file_paths": ["../../compact_osw/files/"], - "measure_paths": ["../../compact_osw/measures/"], - "weather_file": "srrl_2013_amy.epw", - "seed_file": "seb.osm", "steps": [ { - "measure_dir_name": "Standard Reports", + "measure_dir_name": "FakeReport", "arguments": {} } ] diff --git a/src/cli/test/test_with_analysis.py b/src/cli/test/test_with_analysis.py index 99d2cc87d82..0fda08fe600 100644 --- a/src/cli/test/test_with_analysis.py +++ b/src/cli/test/test_with_analysis.py @@ -10,13 +10,26 @@ [pytest.param(True, id="labs"), pytest.param(False, id="classic")], ) def test_run_with_analysis(osclipath, is_labs: bool): - osw_path = Path("with_analysis.osw").resolve() - assert osw_path.is_file(), f"{osw_path=} is not found" + base_osw_path = Path("with_analysis.osw").resolve() + assert base_osw_path.is_file(), f"{base_osw_path=} is not found" + + osw = json.loads(base_osw_path.read_text()) + suffix = 'labs' if is_labs else 'classic' + osw_path = base_osw_path.parent / f"with_analysis_{suffix}.osw" + runDir = base_osw_path.parent / f"run_{suffix}" + osw["run_directory"] = str(runDir) + with open(osw_path, 'w') as f: + json.dump(osw, fp=f, indent=2, sort_keys=True) + + if not is_labs: + # Fake having an in.idf or it won't run + with open(runDir / "in.idf", "w") as f: + f.write("Building,;") command = [str(osclipath)] if not is_labs: command.append("classic") - command += ["run", "-w", str(osw_path)] + command += ["run", "--postprocess_only", "-w", str(osw_path)] lines = subprocess.check_output(command, encoding="utf-8").splitlines() runDir = Path("./run") @@ -30,12 +43,12 @@ def test_run_with_analysis(osclipath, is_labs: bool): measure_attributes = json.loads(measure_attributes_path.read_text()) assert measure_attributes == { - "StandardReports": {"applicable": True, "net_site_energy": 167.1, "something_with_invalid_chars": 1} + "FakeReport": {"applicable": True, "net_site_energy": 167.1, "something_with_invalid_chars": 1} } results = json.loads(results_path.read_text()) assert results == { - "StandardReports": {"applicable": True, "net_site_energy": 167.1, "something_with_invalid_chars": 1} + "FakeReport": {"applicable": True, "net_site_energy": 167.1, "something_with_invalid_chars": 1} } objectives = json.loads(objectives_path.read_text()) From 3fae861ca18afc304ee390af5bf4d08d68cf2073 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 13 Nov 2023 18:36:29 +0100 Subject: [PATCH 05/10] Implement spitting out results.json and objectives.json when analysis.json is present one dir above osw --- src/workflow/ApplyMeasure.cpp | 2 +- src/workflow/OSWorkflow.cpp | 122 ++++++++++++++++++++++++-- src/workflow/OSWorkflow.hpp | 4 + src/workflow/RunReportingMeasures.cpp | 2 + 4 files changed, 124 insertions(+), 6 deletions(-) diff --git a/src/workflow/ApplyMeasure.cpp b/src/workflow/ApplyMeasure.cpp index 36a0feb8999..84968e30c3d 100644 --- a/src/workflow/ApplyMeasure.cpp +++ b/src/workflow/ApplyMeasure.cpp @@ -325,7 +325,7 @@ end } // Add an applicability flag to all the measure results - StepResult stepResult = std::move(*stepResult_); + const StepResult stepResult = std::move(*stepResult_); LOG(Debug, "Step Result: " << stepResult.valueName()); measureAttributes["applicable"] = openstudio::Variant(!((stepResult == StepResult::NA) || (stepResult == StepResult::Skip))); diff --git a/src/workflow/OSWorkflow.cpp b/src/workflow/OSWorkflow.cpp index 2b9b5f9584e..9d3fd221bc5 100644 --- a/src/workflow/OSWorkflow.cpp +++ b/src/workflow/OSWorkflow.cpp @@ -19,6 +19,7 @@ #include "../utilities/core/Assert.hpp" #include "../utilities/core/Filesystem.hpp" #include "../utilities/core/FileLogSink.hpp" +#include "../utilities/core/Json.hpp" #include "../utilities/core/Logger.hpp" #include "../utilities/data/Variant.hpp" #include "../utilities/filetypes/WorkflowStep.hpp" @@ -30,9 +31,11 @@ #include #include #include +#include #include #include +#include #include #include @@ -222,6 +225,8 @@ bool OSWorkflow::run() { hasDeletedRunDir = true; openstudio::filesystem::remove_all(runDirPath); } + } + if (!openstudio::filesystem::is_directory(runDirPath)) { openstudio::filesystem::create_directory(runDirPath); } FileLogSink logFile(runDirPath / "run.log"); @@ -456,12 +461,14 @@ bool OSWorkflow::run() { return (state == State::Finished); } -void OSWorkflow::communicateMeasureAttributes() const { - +Json::Value outputAttributesToJSON(const std::map>& output_attributes, + bool sanitize = false) { Json::Value root(Json::objectValue); - for (const auto& [measureName, argMap] : output_attributes) { + for (const auto& [oriMeasureName, argMap] : output_attributes) { + const std::string measureName = sanitize ? openstudio::workflow::util::sanitizeKey(oriMeasureName) : oriMeasureName; Json::Value measureValues(Json::objectValue); - for (const auto& [argName, variantValue] : argMap) { + for (const auto& [oriArgName, variantValue] : argMap) { + const std::string argName = sanitize ? openstudio::workflow::util::sanitizeKey(oriArgName) : oriArgName; if (variantValue.variantType() == VariantType::String) { measureValues[argName] = variantValue.valueAsString(); } else if (variantValue.variantType() == VariantType::Double) { @@ -472,11 +479,18 @@ void OSWorkflow::communicateMeasureAttributes() const { measureValues[argName] = variantValue.valueAsBoolean(); } } + root[measureName] = measureValues; } + return root; +} + +void OSWorkflow::communicateMeasureAttributes() const { + + const Json::Value root = outputAttributesToJSON(output_attributes, false); Json::StreamWriterBuilder wbuilder; // mimic the old StyledWriter behavior: - wbuilder["indentation"] = " "; + wbuilder["indentation"] = " "; const std::string result = Json::writeString(wbuilder, root); auto jsonPath = workflowJSON.absoluteRunDir() / "measure_attributes.json"; @@ -486,4 +500,102 @@ void OSWorkflow::communicateMeasureAttributes() const { file.close(); } +void OSWorkflow::runExtractInputsAndOutputs() const { + const Json::Value results = outputAttributesToJSON(output_attributes, true); + Json::StreamWriterBuilder wbuilder; + wbuilder["indentation"] = " "; + + { + const std::string result = Json::writeString(wbuilder, results); + + auto jsonPath = workflowJSON.absoluteRunDir() / "results.json"; + openstudio::filesystem::ofstream file(jsonPath); + OS_ASSERT(file.is_open()); + file << result; + file.close(); + } + + const auto osa_abs_path = workflowJSON.absoluteRootDir().parent_path() / "analysis.json"; + if (!openstudio::filesystem::is_regular_file(osa_abs_path)) { + return; + } + + std::ifstream ifs(osa_abs_path); + + Json::CharReaderBuilder rbuilder; + std::string formattedErrors; + + Json::Value analysis_json; + const bool parsingSuccessful = Json::parseFromStream(rbuilder, ifs, &analysis_json, &formattedErrors); + if (!parsingSuccessful) { + LOG_AND_THROW("OSA Analysis JSON '" << toString(osa_abs_path) << "' cannot be processed, " << formattedErrors); + } + + if (!openstudio::checkKeyAndType(analysis_json, "analysis", Json::objectValue)) { + return; + } + + if (!openstudio::checkKeyAndType(analysis_json["analysis"], "output_variables", Json::arrayValue)) { + return; + } + + Json::Value objectiveFunctions(Json::objectValue); + + auto& outputVars = analysis_json["analysis"]["output_variables"]; + for (const auto& variable : outputVars) { + if (openstudio::checkKeyAndType(variable, "objective_function", Json::booleanValue) && variable["objective_function"].asBool()) { + assertKeyAndType(variable, "name", Json::stringValue); + assertKeyAndType(variable, "objective_function_index", Json::intValue); + const std::string name = variable["name"].asString(); + const int idx = variable["objective_function_index"].asInt() + 1; + + LOG(Info, "Looking for objective function " << name); + + // Splitting on a `.` feels very unrealiable + const size_t pos = name.find('.'); + if (pos == std::string::npos) { + LOG(Warn, "Objective function name='" << name << "' does not contain a dot (`.`)"); + continue; + } + const std::string measureName = name.substr(0, pos); + const std::string argName = name.substr(pos + 1); + if (results.isMember(measureName) && results[measureName].isMember(argName)) { + objectiveFunctions[fmt::format("objective_function_{}", idx)] = results[measureName][argName]; + + if (openstudio::checkKeyAndType(variable, "objective_function_target", Json::realValue)) { + LOG(Info, "Found objective function target for " << name); + objectiveFunctions[fmt::format("objective_function_target_{}", idx)] = variable["objective_function_target"].asDouble(); + } + + if (openstudio::checkKeyAndType(variable, "scaling_factor", Json::realValue)) { + LOG(Info, "Found scaling factor for " << name); + objectiveFunctions[fmt::format("scaling_factor_{}", idx)] = variable["scaling_factor"].asDouble(); + } + + if (openstudio::checkKeyAndType(variable, "objective_function_group", Json::realValue)) { + LOG(Info, "Found objective function group for " << name); + objectiveFunctions[fmt::format("objective_function_group_{}", idx)] = variable["objective_function_group"].asDouble(); + } + + } else { + LOG(Warn, "No results for objective function " << name); + objectiveFunctions[fmt::format("objective_function_{}", idx)] = std::numeric_limits::max(); + objectiveFunctions[fmt::format("objective_function_target_{}", idx)] = Json::nullValue; + objectiveFunctions[fmt::format("scaling_factor_{}", idx)] = Json::nullValue; + objectiveFunctions[fmt::format("objective_function_group_{}", idx)] = Json::nullValue; + } + } + } + + { + const std::string objectives = Json::writeString(wbuilder, objectiveFunctions); + + auto objectivesJsonPath = workflowJSON.absoluteRunDir() / "objectives.json"; + openstudio::filesystem::ofstream file(objectivesJsonPath); + OS_ASSERT(file.is_open()); + file << objectives; + file.close(); + } +} + } // namespace openstudio diff --git a/src/workflow/OSWorkflow.hpp b/src/workflow/OSWorkflow.hpp index 2cca4fdf0b7..5502471cb8b 100644 --- a/src/workflow/OSWorkflow.hpp +++ b/src/workflow/OSWorkflow.hpp @@ -130,6 +130,10 @@ class OSWorkflow // write output_attributes to the measure_attributes.json void communicateMeasureAttributes() const; + + /** Write results.json (same as the final measure_attributes.json but with sanitized keys) + * and if `absoluteRootDir (oswDir) / .. / analysis.json` is found, write the objectives.json */ + void runExtractInputsAndOutputs() const; }; } // namespace openstudio diff --git a/src/workflow/RunReportingMeasures.cpp b/src/workflow/RunReportingMeasures.cpp index c28c873bb62..365e269fcd3 100644 --- a/src/workflow/RunReportingMeasures.cpp +++ b/src/workflow/RunReportingMeasures.cpp @@ -58,6 +58,8 @@ void OSWorkflow::runReportingMeasures() { LOG(Info, "Finished applying Reporting Measures."); communicateMeasureAttributes(); + + runExtractInputsAndOutputs(); // TODO // # Parse the files generated by the local output adapter // results, objective_functions = run_extract_inputs_and_outputs @registry[:run_dir], @logger From 6174aeef021f9ed3b65c1b50879899a51473f873 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 13 Nov 2023 18:37:43 +0100 Subject: [PATCH 06/10] Add a script + a convenience target for updating the **build_dir** seb.osm: it's at 1.11.5 so it takes a long time build/resources/Examples/compact_osw/files/seb.osm --- resources/Examples/compact_osw/update_seb_model.rb | 14 ++++++++++++++ src/cli/CMakeLists.txt | 6 ++++++ src/cli/test/test_with_analysis.py | 11 ++++++----- 3 files changed, 26 insertions(+), 5 deletions(-) create mode 100644 resources/Examples/compact_osw/update_seb_model.rb diff --git a/resources/Examples/compact_osw/update_seb_model.rb b/resources/Examples/compact_osw/update_seb_model.rb new file mode 100644 index 00000000000..a497af28cfc --- /dev/null +++ b/resources/Examples/compact_osw/update_seb_model.rb @@ -0,0 +1,14 @@ +# Just for convenience: having to VT the seb model which is at 1.11.5 +# can be very time consuming, especially on debug builds + +require 'openstudio' + +model_path = File.join(__dir__, 'files/seb.osm') +raise "#{model_path} not found" if !File.file?(model_path) + +# Starting at 3.7.0, no need to explicitly call vt, but still doing it +# m = OpenStudio::Model::Model::load(model_path).get() +vt = OpenStudio::OSVersion::VersionTranslator.new +OpenStudio::Logger.instance.standardOutLogger.setLogLevel(OpenStudio::Debug) +m = vt.loadModel(model_path).get() +m.save(model_path, true) diff --git a/src/cli/CMakeLists.txt b/src/cli/CMakeLists.txt index a2deb5701bb..8f9db4837de 100644 --- a/src/cli/CMakeLists.txt +++ b/src/cli/CMakeLists.txt @@ -83,6 +83,12 @@ endif() if(BUILD_TESTING) + # Just for convenience: having to VT the seb model which is at 1.11.5 can be very time consuming, especially on debug builds + add_custom_target(update_seb_osm_in_build_dir + COMMAND $ execute_ruby_script "${PROJECT_BINARY_DIR}/resources/Examples/compact_osw/update_seb_model.rb" + DEPENDS openstudio + ) + add_test(NAME OpenStudioCLI.help COMMAND $ --help ) diff --git a/src/cli/test/test_with_analysis.py b/src/cli/test/test_with_analysis.py index 0fda08fe600..421637bfb4d 100644 --- a/src/cli/test/test_with_analysis.py +++ b/src/cli/test/test_with_analysis.py @@ -18,13 +18,15 @@ def test_run_with_analysis(osclipath, is_labs: bool): osw_path = base_osw_path.parent / f"with_analysis_{suffix}.osw" runDir = base_osw_path.parent / f"run_{suffix}" osw["run_directory"] = str(runDir) + runDir.mkdir(exist_ok=True) with open(osw_path, 'w') as f: json.dump(osw, fp=f, indent=2, sort_keys=True) - if not is_labs: - # Fake having an in.idf or it won't run - with open(runDir / "in.idf", "w") as f: - f.write("Building,;") + # Fake having an in.idf or it won't run in the "classic" subcommand, doing it for labs too so that it's less + # confusing + # if not is_labs: + with open(runDir / "in.idf", "w") as f: + f.write("Building,;") command = [str(osclipath)] if not is_labs: @@ -32,7 +34,6 @@ def test_run_with_analysis(osclipath, is_labs: bool): command += ["run", "--postprocess_only", "-w", str(osw_path)] lines = subprocess.check_output(command, encoding="utf-8").splitlines() - runDir = Path("./run") assert runDir.exists() measure_attributes_path = runDir / "measure_attributes.json" assert measure_attributes_path.is_file() From 44c2236355b8fe402dbb898df7ad045bfaef8010 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 13 Nov 2023 18:54:19 +0100 Subject: [PATCH 07/10] Making a test fail on purpose with TODO: Need to clarify which of the measure_attributes.json, results.json and data_point_out.json are needed and when: they have the SAME info! --- src/cli/test/test_with_analysis.py | 32 +++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/src/cli/test/test_with_analysis.py b/src/cli/test/test_with_analysis.py index 421637bfb4d..25a7910d0f0 100644 --- a/src/cli/test/test_with_analysis.py +++ b/src/cli/test/test_with_analysis.py @@ -14,12 +14,12 @@ def test_run_with_analysis(osclipath, is_labs: bool): assert base_osw_path.is_file(), f"{base_osw_path=} is not found" osw = json.loads(base_osw_path.read_text()) - suffix = 'labs' if is_labs else 'classic' + suffix = "labs" if is_labs else "classic" osw_path = base_osw_path.parent / f"with_analysis_{suffix}.osw" runDir = base_osw_path.parent / f"run_{suffix}" osw["run_directory"] = str(runDir) runDir.mkdir(exist_ok=True) - with open(osw_path, 'w') as f: + with open(osw_path, "w") as f: json.dump(osw, fp=f, indent=2, sort_keys=True) # Fake having an in.idf or it won't run in the "classic" subcommand, doing it for labs too so that it's less @@ -48,9 +48,7 @@ def test_run_with_analysis(osclipath, is_labs: bool): } results = json.loads(results_path.read_text()) - assert results == { - "FakeReport": {"applicable": True, "net_site_energy": 167.1, "something_with_invalid_chars": 1} - } + assert results == {"FakeReport": {"applicable": True, "net_site_energy": 167.1, "something_with_invalid_chars": 1}} objectives = json.loads(objectives_path.read_text()) assert objectives == { @@ -63,3 +61,27 @@ def test_run_with_analysis(osclipath, is_labs: bool): "scaling_factor_1": 1.0, "scaling_factor_3": None, } + + expected_files_in_run_dir = { + "data_point.zip", + "finished.job", + "in.idf", + "measure_attributes.json", + "objectives.json", + "results.json", + "run.log", + "started.job", + # TODO: see below + "data_point_out.json", + } + # TODO: I'm letting this test fail so it's obvious this needs to be addressed + if True: # not is_labs: + # We get the SAME exact info in measure_attributes.json, results.json and data_point_out.json... + # measure_attributes.json is flushed after each apply measure Step (ModelMeasures, EnergyPlusMeasures, + # ReportingMeasures), then at the end of ReportingMeasures it's done once again and results.json is spat out too + # Do we really need the data_point_out.json in addition to this? + # Seems like we could just run the output of results.json/data_point_out.json at the end of the workflow run + # instead + expected_files_in_run_dir.add("data_point_out.json") + + assert set([x.name for x in runDir.glob("*")]) == expected_files_in_run_dir From 7ed29be48350220c47aef25073c33f8bbb7056b9 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 13 Nov 2023 21:32:50 +0100 Subject: [PATCH 08/10] Fix build error --- src/workflow/OSWorkflow.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/workflow/OSWorkflow.cpp b/src/workflow/OSWorkflow.cpp index 9d3fd221bc5..89effc5b3a8 100644 --- a/src/workflow/OSWorkflow.cpp +++ b/src/workflow/OSWorkflow.cpp @@ -520,7 +520,7 @@ void OSWorkflow::runExtractInputsAndOutputs() const { return; } - std::ifstream ifs(osa_abs_path); + std::ifstream ifs(openstudio::toSystemFilename(osa_abs_path)); Json::CharReaderBuilder rbuilder; std::string formattedErrors; From 659cd674696b663979e7be5bff5e9207b9e5c3ca Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 13 Nov 2023 21:32:40 +0100 Subject: [PATCH 09/10] Implement skip zip results in RunOptions. It's part of the osw schema and use in openstuido-workflow-gem --- src/utilities/filetypes/RunOptions.cpp | 49 +++++++++++++++++++ src/utilities/filetypes/RunOptions.hpp | 14 ++++++ src/utilities/filetypes/RunOptions_Impl.hpp | 9 ++++ .../filetypes/test/WorkflowJSON_GTest.cpp | 17 +++++++ 4 files changed, 89 insertions(+) diff --git a/src/utilities/filetypes/RunOptions.cpp b/src/utilities/filetypes/RunOptions.cpp index 9c1aabb7cce..affdfcfd006 100644 --- a/src/utilities/filetypes/RunOptions.cpp +++ b/src/utilities/filetypes/RunOptions.cpp @@ -50,6 +50,10 @@ namespace detail { root["skip_energyplus_preprocess"] = m_skipEnergyPlusPreprocess; } + if (!m_is_skipZipResults_defaulted) { + root["skip_zip_results"] = m_skipZipResults; + } + if (!m_is_cleanup_defaulted) { root["cleanup"] = m_cleanup; } @@ -200,6 +204,27 @@ namespace detail { onUpdate(); } + bool RunOptions_Impl::skipZipResults() const { + return m_skipZipResults; + } + + bool RunOptions_Impl::isSkipZipResultsDefaulted() const { + return m_is_skipZipResults_defaulted; + } + + bool RunOptions_Impl::setSkipZipResults(bool skipZipResults) { + m_skipZipResults = skipZipResults; + m_is_skipZipResults_defaulted = false; + onUpdate(); + return true; + } + + void RunOptions_Impl::resetSkipZipResults() { + m_skipZipResults = DEFAULT_SKIPZIPRESULTS; + m_is_skipZipResults_defaulted = true; + onUpdate(); + } + bool RunOptions_Impl::cleanup() const { return m_cleanup; } @@ -277,6 +302,10 @@ namespace detail { setSkipEnergyPlusPreprocess(other.skipEnergyPlusPreprocess()); } + if (!other.isSkipZipResultsDefaulted()) { + setSkipZipResults(other.skipZipResults()); + } + if (!other.isCleanupDefaulted()) { setCleanup(other.cleanup()); } @@ -382,6 +411,10 @@ boost::optional RunOptions::fromString(const std::string& s) { result.setSkipEnergyPlusPreprocess(value["skip_energyplus_preprocess"].asBool()); } + if (value.isMember("skip_zip_results") && value["skip_zip_results"].isBool()) { + result.setSkipZipResults(value["skip_zip_results"].asBool()); + } + if (value.isMember("output_adapter")) { Json::Value outputAdapter = value["output_adapter"]; if (outputAdapter.isMember("custom_file_name") && outputAdapter.isMember("class_name")) { @@ -504,6 +537,22 @@ void RunOptions::resetSkipEnergyPlusPreprocess() { getImpl()->resetSkipEnergyPlusPreprocess(); } +bool RunOptions::skipZipResults() const { + return getImpl()->skipZipResults(); +} + +bool RunOptions::isSkipZipResultsDefaulted() const { + return getImpl()->isSkipZipResultsDefaulted(); +} + +bool RunOptions::setSkipZipResults(bool skipZipResults) { + return getImpl()->setSkipZipResults(skipZipResults); +} + +void RunOptions::resetSkipZipResults() { + getImpl()->resetSkipZipResults(); +} + bool RunOptions::cleanup() const { return getImpl()->cleanup(); } diff --git a/src/utilities/filetypes/RunOptions.hpp b/src/utilities/filetypes/RunOptions.hpp index 83623e97af4..1705c5627fe 100644 --- a/src/utilities/filetypes/RunOptions.hpp +++ b/src/utilities/filetypes/RunOptions.hpp @@ -62,16 +62,19 @@ class UTILITIES_API RunOptions /// Serialize to JSON formatted string std::string string() const; + /** Print debugging messages, defaults to false */ bool debug() const; bool isDebugDefaulted() const; bool setDebug(bool debug); void resetDebug(); + /** Create, export and run using epjson format. Default is false */ bool epjson() const; bool isEpjsonDefaulted() const; bool setEpjson(bool epjson); void resetEpjson(); + /** Speeds up workflow by skipping steps not needed for running simulations, defaults to false */ bool fast() const; bool isFastDefaulted() const; bool setFast(bool fast); @@ -82,16 +85,27 @@ class UTILITIES_API RunOptions bool setPreserveRunDir(bool preserveRunDir); void resetPreserveRunDir(); + /** Skips the call to the EnergyPlus ExpandObjects program, defaults to false */ bool skipExpandObjects() const; bool isSkipExpandObjectsDefaulted() const; bool setSkipExpandObjects(bool skipExpandObjects); void resetSkipExpandObjects(); + /** Does not add add default output requests to EnergyPlus input if true. + * Requests from reporting measures are added in either case, defaults to false */ bool skipEnergyPlusPreprocess() const; bool isSkipEnergyPlusPreprocessDefaulted() const; bool setSkipEnergyPlusPreprocess(bool skipEnergyPlusPreprocess); void resetSkipEnergyPlusPreprocess(); + /** Skips creating the data_point.zip file. Setting to `true` can cause issues with workflows expecting .zip files to signal completion + * (e.g., OpenStudio Analysis Framework), defaults to false */ + bool skipZipResults() const; + bool isSkipZipResultsDefaulted() const; + bool setSkipZipResults(bool skipZipResults); + void resetSkipZipResults(); + + /** Remove unnecessary files during post processing, defaults to true */ bool cleanup() const; bool isCleanupDefaulted() const; bool setCleanup(bool cleanup); diff --git a/src/utilities/filetypes/RunOptions_Impl.hpp b/src/utilities/filetypes/RunOptions_Impl.hpp index db961e45999..9c97d07cf7e 100644 --- a/src/utilities/filetypes/RunOptions_Impl.hpp +++ b/src/utilities/filetypes/RunOptions_Impl.hpp @@ -66,6 +66,11 @@ namespace detail { bool setCleanup(bool cleanup); void resetCleanup(); + bool skipZipResults() const; + bool isSkipZipResultsDefaulted() const; + bool setSkipZipResults(bool skipZipResults); + void resetSkipZipResults(); + boost::optional customOutputAdapter() const; bool setCustomOutputAdapter(const CustomOutputAdapter& adapter); void resetCustomOutputAdapter(); @@ -94,6 +99,7 @@ namespace detail { static constexpr bool DEFAULT_SKIPEXPANDOBJECTS = false; static constexpr bool DEFAULT_SKIPENERGYPLUSPREPROCESS = false; static constexpr bool DEFAULT_CLEANUP = true; + static constexpr bool DEFAULT_SKIPZIPRESULTS = false; bool m_debug = DEFAULT_DEBUG; bool m_is_debug_defaulted = true; @@ -117,6 +123,9 @@ namespace detail { bool m_cleanup = DEFAULT_CLEANUP; bool m_is_cleanup_defaulted = true; + bool m_skipZipResults = DEFAULT_SKIPZIPRESULTS; + bool m_is_skipZipResults_defaulted = true; + ForwardTranslatorOptions m_forwardTranslatorOptions; boost::optional m_customOutputAdapter; }; diff --git a/src/utilities/filetypes/test/WorkflowJSON_GTest.cpp b/src/utilities/filetypes/test/WorkflowJSON_GTest.cpp index 5028f4bd578..d88b5b8247a 100644 --- a/src/utilities/filetypes/test/WorkflowJSON_GTest.cpp +++ b/src/utilities/filetypes/test/WorkflowJSON_GTest.cpp @@ -1236,6 +1236,19 @@ TEST(Filetypes, RunOptions_GettersSetters) { ASSERT_TRUE(runOptions.isSkipEnergyPlusPreprocessDefaulted()); ASSERT_TRUE(runOptions.isSkipEnergyPlusPreprocessDefaulted()); + // Ctor Default + ASSERT_FALSE(runOptions.skipZipResults()); + ASSERT_TRUE(runOptions.isSkipZipResultsDefaulted()); + // Set to opposite of default + ASSERT_TRUE(runOptions.setSkipZipResults(true)); + ASSERT_TRUE(runOptions.skipZipResults()); + ASSERT_FALSE(runOptions.isSkipZipResultsDefaulted()); + // Reset + runOptions.resetSkipZipResults(); + ASSERT_FALSE(runOptions.skipZipResults()); + ASSERT_TRUE(runOptions.isSkipZipResultsDefaulted()); + ASSERT_TRUE(runOptions.isSkipZipResultsDefaulted()); + // Ctor Default ASSERT_TRUE(runOptions.cleanup()); ASSERT_TRUE(runOptions.isCleanupDefaulted()); @@ -1362,6 +1375,8 @@ TEST(Filetypes, RunOptions_overrideValuesWith) { ASSERT_TRUE(runOptions.isSkipExpandObjectsDefaulted()); ASSERT_FALSE(runOptions.skipEnergyPlusPreprocess()); ASSERT_TRUE(runOptions.isSkipEnergyPlusPreprocessDefaulted()); + ASSERT_FALSE(runOptions.skipZipResults()); + ASSERT_TRUE(runOptions.isSkipZipResultsDefaulted()); ASSERT_TRUE(runOptions.cleanup()); ASSERT_TRUE(runOptions.isCleanupDefaulted()); @@ -1401,6 +1416,8 @@ TEST(Filetypes, RunOptions_overrideValuesWith) { ASSERT_TRUE(runOptions.isSkipExpandObjectsDefaulted()); ASSERT_FALSE(runOptions.skipEnergyPlusPreprocess()); ASSERT_TRUE(runOptions.isSkipEnergyPlusPreprocessDefaulted()); + ASSERT_FALSE(runOptions.skipEnergyPlusPreprocess()); + ASSERT_TRUE(runOptions.isSkipEnergyPlusPreprocessDefaulted()); ASSERT_TRUE(runOptions.cleanup()); ASSERT_TRUE(runOptions.isCleanupDefaulted()); From 51936795f0318fd2ab63e28a667256ffe7d78d50 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 13 Nov 2023 21:51:34 +0100 Subject: [PATCH 10/10] Add data_point_out.json as requested I talked to some folks who suggested just adding it, and I filed #5029 for investigating at a later date if we can just use a single file. --- src/cli/test/test_with_analysis.py | 17 ++++++--------- src/workflow/OSWorkflow.cpp | 34 ++++++++++++++++++++++++------ src/workflow/OSWorkflow.hpp | 3 +++ 3 files changed, 37 insertions(+), 17 deletions(-) diff --git a/src/cli/test/test_with_analysis.py b/src/cli/test/test_with_analysis.py index 25a7910d0f0..a61e1058736 100644 --- a/src/cli/test/test_with_analysis.py +++ b/src/cli/test/test_with_analysis.py @@ -62,6 +62,13 @@ def test_run_with_analysis(osclipath, is_labs: bool): "scaling_factor_3": None, } + data_point_out_path = runDir / "data_point_out.json" + assert data_point_out_path.is_file() + data_point_out = json.loads(data_point_out_path.read_text()) + assert data_point_out == { + "FakeReport": {"applicable": True, "net_site_energy": 167.1, "something_with_invalid_chars": 1} + } + expected_files_in_run_dir = { "data_point.zip", "finished.job", @@ -71,17 +78,7 @@ def test_run_with_analysis(osclipath, is_labs: bool): "results.json", "run.log", "started.job", - # TODO: see below "data_point_out.json", } - # TODO: I'm letting this test fail so it's obvious this needs to be addressed - if True: # not is_labs: - # We get the SAME exact info in measure_attributes.json, results.json and data_point_out.json... - # measure_attributes.json is flushed after each apply measure Step (ModelMeasures, EnergyPlusMeasures, - # ReportingMeasures), then at the end of ReportingMeasures it's done once again and results.json is spat out too - # Do we really need the data_point_out.json in addition to this? - # Seems like we could just run the output of results.json/data_point_out.json at the end of the workflow run - # instead - expected_files_in_run_dir.add("data_point_out.json") assert set([x.name for x in runDir.glob("*")]) == expected_files_in_run_dir diff --git a/src/workflow/OSWorkflow.cpp b/src/workflow/OSWorkflow.cpp index 89effc5b3a8..321506dd3c0 100644 --- a/src/workflow/OSWorkflow.cpp +++ b/src/workflow/OSWorkflow.cpp @@ -401,13 +401,7 @@ bool OSWorkflow::run() { } if (!workflowJSON.runOptions()->fast()) { - if (m_add_timings) { - m_timers->newTimer("Zip datapoint"); - } - openstudio::workflow::util::zipResults(runDirPath); - if (m_add_timings) { - m_timers->tockCurrentTimer(); - } + communicateResults(); } if (state == State::Errored) { @@ -491,6 +485,7 @@ void OSWorkflow::communicateMeasureAttributes() const { Json::StreamWriterBuilder wbuilder; // mimic the old StyledWriter behavior: wbuilder["indentation"] = " "; + const std::string result = Json::writeString(wbuilder, root); auto jsonPath = workflowJSON.absoluteRunDir() / "measure_attributes.json"; @@ -598,4 +593,29 @@ void OSWorkflow::runExtractInputsAndOutputs() const { } } +void OSWorkflow::communicateResults() const { + if (!workflowJSON.runOptions()->skipZipResults()) { + if (m_add_timings) { + m_timers->newTimer("Zip datapoint"); + } + openstudio::workflow::util::zipResults(workflowJSON.absoluteRunDir()); + if (m_add_timings) { + m_timers->tockCurrentTimer(); + } + } + + const Json::Value root = outputAttributesToJSON(output_attributes, true); + Json::StreamWriterBuilder wbuilder; + // mimic the old StyledWriter behavior: + wbuilder["indentation"] = " "; + + const std::string result = Json::writeString(wbuilder, root); + + auto jsonPath = workflowJSON.absoluteRunDir() / "data_point_out.json"; + openstudio::filesystem::ofstream file(jsonPath); + OS_ASSERT(file.is_open()); + file << result; + file.close(); +} + } // namespace openstudio diff --git a/src/workflow/OSWorkflow.hpp b/src/workflow/OSWorkflow.hpp index 5502471cb8b..0a14f877ebc 100644 --- a/src/workflow/OSWorkflow.hpp +++ b/src/workflow/OSWorkflow.hpp @@ -134,6 +134,9 @@ class OSWorkflow /** Write results.json (same as the final measure_attributes.json but with sanitized keys) * and if `absoluteRootDir (oswDir) / .. / analysis.json` is found, write the objectives.json */ void runExtractInputsAndOutputs() const; + + // Zip and write data_point_out.osw + void communicateResults() const; }; } // namespace openstudio