diff --git a/src/measure/OSRunner.cpp b/src/measure/OSRunner.cpp index a4e1421c446..926a59838e6 100644 --- a/src/measure/OSRunner.cpp +++ b/src/measure/OSRunner.cpp @@ -226,6 +226,10 @@ namespace measure { //} void OSRunner::prepareForMeasureRun(const OSMeasure& /*measure*/) { + prepareForMeasureRun(); + } + + void OSRunner::prepareForMeasureRun() { if (m_halted) { LOG(Error, "Wokflow halted, cannot prepate for measure run"); return; diff --git a/src/measure/OSRunner.hpp b/src/measure/OSRunner.hpp index 492c3358053..37db45f5f44 100644 --- a/src/measure/OSRunner.hpp +++ b/src/measure/OSRunner.hpp @@ -19,6 +19,7 @@ namespace openstudio { +class OSWorkflow; class Workspace; class WorkspaceObject; @@ -290,6 +291,13 @@ namespace measure { bool registerMsgAlsoLogs() const; void setRegisterMsgAlsoLogs(bool registerMsgAlsoLogs); + protected: + friend class openstudio::OSWorkflow; + // prepareForMeasureRun(OSMeasure) actually doesn't use the OSMeasure, but it's a check that an OSMeasure is there. + // This is the actual implementation though, and it's protected by OSWorkflow is friended so it can call it before SKIPPING a step without the + // performance penatly of actually loading and resolving the measure just to skip it + void prepareForMeasureRun(); + private: REGISTER_LOGGER("openstudio.measure.OSRunner"); diff --git a/src/utilities/data/Variant.cpp b/src/utilities/data/Variant.cpp index 35325f69e69..c78cd2d3c21 100644 --- a/src/utilities/data/Variant.cpp +++ b/src/utilities/data/Variant.cpp @@ -25,41 +25,35 @@ VariantType Variant::variantType() const { } bool Variant::valueAsBoolean() const { - // Note JM 2019-05-17: This is functionally equivalent to `std::get(m_value)` except it doesn't risk throwing - // std::bad_variant_access which isn't available on mac prior to 10.14 - if (const auto* p = std::get_if(&m_value)) { - return *p; - } else { + if (m_type != VariantType::Boolean) { LOG_AND_THROW("Variant does not hold a boolean"); } + return std::get(m_value); } int Variant::valueAsInteger() const { - if (const auto* p = std::get_if(&m_value)) { - return *p; - } else { + if (m_type != VariantType::Integer) { LOG_AND_THROW("Variant does not hold an int"); } + return std::get(m_value); } double Variant::valueAsDouble() const { if (m_type == VariantType::Integer) { - return (double)valueAsInteger(); + return static_cast(valueAsInteger()); } - if (const auto* p = std::get_if(&m_value)) { - return *p; - } else { + if (m_type != VariantType::Double) { LOG_AND_THROW("Variant does not hold a double"); } + return std::get(m_value); } std::string Variant::valueAsString() const { - if (const auto* p = std::get_if(&m_value)) { - return *p; - } else { + if (m_type != VariantType::String) { LOG_AND_THROW("Variant does not hold a string"); } + return std::get(m_value); } // helper constant for the visitor below so we static assert we didn't miss a type @@ -85,8 +79,27 @@ Json::Value Variant::valueAsJSON() const { m_value); } +bool Variant::isTrueish() const { + return std::visit( + [](auto&& arg) -> bool { + using T = std::decay_t; + if constexpr (std::is_same_v) { // NOLINT(bugprone-branch-clone) + return arg; + } else if constexpr (std::is_same_v) { + return arg != 0; + } else if constexpr (std::is_same_v) { + return arg != 0.0; + } else if constexpr (std::is_same_v) { + return arg == "true"; + } else { + static_assert(always_false_v, "non-exhaustive visitor!"); + } + }, + m_value); +} + std::ostream& operator<<(std::ostream& os, const Variant& variant) { - VariantType variantType = variant.variantType(); + const VariantType variantType = variant.variantType(); if (variantType == VariantType::String) { os << variant.valueAsString(); } else if (variantType == VariantType::Double) { diff --git a/src/utilities/data/Variant.hpp b/src/utilities/data/Variant.hpp index b0878252a2a..9ef756f58af 100644 --- a/src/utilities/data/Variant.hpp +++ b/src/utilities/data/Variant.hpp @@ -73,6 +73,10 @@ class UTILITIES_API Variant Json::Value valueAsJSON() const; + // No matter the underlying type, will check if it's true-ish + // Used for skipping a measure. eg true, "true", 1, 1.0 are all trueish + bool isTrueish() const; + private: REGISTER_LOGGER("openstudio.Variant"); diff --git a/src/workflow/ApplyMeasure.cpp b/src/workflow/ApplyMeasure.cpp index 0f8816857f5..d2d9a2e1220 100644 --- a/src/workflow/ApplyMeasure.cpp +++ b/src/workflow/ApplyMeasure.cpp @@ -39,19 +39,8 @@ bool isStepMarkedSkip(const std::map& stepArgs // handle skip first if (!stepArgs.empty() && stepArgs.contains("__SKIP__")) { - // TODO: handling of SKIP is incomplete here, will need to increment the runner and co, and not process the measure const openstudio::Variant& argumentValue = stepArgs.at("__SKIP__"); - VariantType variantType = argumentValue.variantType(); - - if (variantType == VariantType::String) { - skip_measure = openstudio::ascii_to_lower_copy(argumentValue.valueAsString()) == "true"; - } else if (variantType == VariantType::Double) { - skip_measure = (argumentValue.valueAsDouble() != 0.0); - } else if (variantType == VariantType::Integer) { - skip_measure = (argumentValue.valueAsInteger() != 0); - } else if (variantType == VariantType::Boolean) { - skip_measure = argumentValue.valueAsBoolean(); - } + skip_measure = argumentValue.isTrueish(); } return skip_measure; } @@ -76,20 +65,30 @@ void OSWorkflow::applyMeasures(MeasureType measureType, bool energyplus_output_r auto stepArgs = step.arguments(); const bool skip_measure = isStepMarkedSkip(stepArgs); - if (skip_measure || runner.halted()) { // TODO: or halted + if (skip_measure || runner.halted()) { if (!energyplus_output_requests) { if (runner.halted()) { LOG(Info, fmt::format("Skipping measure '{}' because simulation halted", measureDirName)); } else { LOG(Info, fmt::format("Skipping measure '{}'", measureDirName)); + // required to update current step + runner.prepareForMeasureRun(); + WorkflowStepResult result = runner.result(); runner.incrementStep(); - // addResultMeasureInfo(result, bclMeasure); // TODO: Should I really instantiate the BCLMeasure just for this? + const auto measureDirPath_ = workflowJSON.findMeasure(measureDirName); + if (measureDirPath_) { + BCLMeasure bclMeasure(measureDirPath_.get()); + workflow::util::addResultMeasureInfo(result, bclMeasure); + const std::string className = bclMeasure.className(); + const auto measureName = step.name().value_or(className); + output_attributes[measureName]["applicable"] = openstudio::Variant(false); + } else { + LOG(Warn, "Could not find measure '" << measureDirName << "', but it's marked as skipped anyways so ignoring."); + output_attributes[step.name().value_or(measureDirName)]["applicable"] = openstudio::Variant(false); + } 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; }