NREL / OpenStudio

OpenStudio is a cross-platform collection of software tools to support whole building energy modeling using EnergyPlus and advanced daylight analysis using Radiance.
https://www.openstudio.net/
Other
484 stars 185 forks source link

Off by one error in measure step_values when _SKIP_ is true #5140

Closed macumber closed 2 months ago

macumber commented 3 months ago

Issue overview

When adding measures to a workflow, there is a special SKIP argument that can be used to skip running a measure. Setting SKIP to true results in step_values being added to the wrong measure in the out.osw file for subsequent measures in the workflow.

Current Behavior

step_values are associated with the wrong measure in out.osw when a measure is skipped

Expected Behavior

step_values should be associated with the correct measure in out.osw when a measure is skipped

Steps to Reproduce

  1. Unzip MixedUpStepsSkip.zip
  2. openstudio.exe run -w workflow.osw
  3. The step_results should be associated with the correct measure indexes but they are off by one when a measure is skipped
  4. Additionally, [openstudio.measure.OSRunner] <Error> Not prepared for step is shown when measure is skipped

Possible Solution

Details

Environment

Some additional details about your environment for this issue (if relevant):

Context

Parsing of results in out.osw file broken for test suite

macumber commented 3 months ago

This does not affect the classic CLI.

jmarrec commented 2 months ago

Aren't TODOs a great way to remind you to finish something and never actually do it? https://github.com/NREL/OpenStudio/blob/b8093180dc61a47935c2fcf067e4cd8bb0fd8a28/src/workflow/ApplyMeasure.cpp#L42

Edit: hem, I thought I did it though here: https://github.com/NREL/OpenStudio/blob/b8093180dc61a47935c2fcf067e4cd8bb0fd8a28/src/workflow/ApplyMeasure.cpp#L78-L89

jmarrec commented 2 months ago

Ok yeah, I need to prepareForMeasureRun before I can set the step Result and increment the step

jmarrec commented 2 months ago

https://github.com/NREL/OpenStudio/commit/c6ff09ce86a4b1d618737ce3ca21cef8c5ccc746 fixes it. I tested locally.

image

macumber commented 2 months ago

Thanks @jmarrec !