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
494 stars 188 forks source link

Measure runner.registerInfo output only shows up with --verbose flag #5069

Open macumber opened 8 months ago

macumber commented 8 months ago

Issue overview

When running a measure with the new CLI, runner.registerWarn messages will show up in the output with --show-stdout but runner.registerInfo messages will not. When using the classic CLI, runner.registerInfo messages do show up in the output without the --verbose flag.

Current Behavior

Info messages from the measures do not show up in the stdout output.

Expected Behavior

Info messages from the measures shouldshow up in the stdout output.

Steps to Reproduce

  1. Create a measure with runner.registerInfo statements
  2. Add measure to a workflow
  3. Run the workflow with openstudio run --show-stdout --style-stdout -w c:\test\untitled\workflow.osw
  4. Verify that info statements are not present
  5. Run the workflow with openstudio --verbose run --show-stdout --style-stdout -w c:\test\untitled\workflow.osw
  6. Verify that info statements are present

Environment

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

Context

Info messages are important for users, these messages should be present without needing the --verbose flag

macumber commented 8 months ago

This also seems to apply to Initial Condition and Final Condition messages. Additionally, it is not possible to distinguish Info, Initial Condition, and Final Condition messages. For example, in the old CLI the "Add EMPD Material Properties" measure output is:

Applying AddEMPDMaterialProperties
[21:28:14.772589 WARN] The Moisture Equation Coefficient A has been left as 0. This is usally a non-zero value.
[21:28:14.772614 WARN] The Moisture Equation Coefficient B has been left as 0. This is usally a non-zero value.
[21:28:14.772621 WARN] The Moisture Equation Coefficient C has been left as 0. This is usally a non-zero value.
[21:28:14.772627 WARN] The Moisture Equation Coefficient D has been left as 0. This is usally a non-zero value.
Result: Success
Initial Condition: The building has 10 materials.
Final Condition: Moisture properties were added to F16 Acoustic tile.
Warn: The Moisture Equation Coefficient A has been left as 0. This is usally a non-zero value.
Warn: The Moisture Equation Coefficient B has been left as 0. This is usally a non-zero value.
Warn: The Moisture Equation Coefficient C has been left as 0. This is usally a non-zero value.
Warn: The Moisture Equation Coefficient D has been left as 0. This is usally a non-zero value.
Info: Surface layer penetration depth set to: AutoCalculate
Info: Deep layer penetration depth set to: AutoCalculate
Applied AddEMPDMaterialProperties

with the new CLI the output (with verbose flag) is:

[openstudio.measure.OSRunner] <Warn> The Moisture Equation Coefficient A has been left as 0. This is usally a non-zero value.
[openstudio.measure.OSRunner] <Warn> The Moisture Equation Coefficient B has been left as 0. This is usally a non-zero value.
[openstudio.measure.OSRunner] <Warn> The Moisture Equation Coefficient C has been left as 0. This is usally a non-zero value.
[openstudio.measure.OSRunner] <Warn> The Moisture Equation Coefficient D has been left as 0. This is usally a non-zero value.
[openstudio.measure.OSRunner] <Info> The building has 10 materials.
[openstudio.measure.OSRunner] <Info> Surface layer penetration depth set to: AutoCalculate
[openstudio.measure.OSRunner] <Info> Deep layer penetration depth set to: AutoCalculate
[openstudio.measure.OSRunner] <Info> Moisture properties were added to F16 Acoustic tile.
[openstudio.workflow.OSWorkflow] <Debug> Step Result: Success

with the new CLI the output (without verbose flag) is:

[openstudio.measure.OSRunner] <Warn> The Moisture Equation Coefficient A has been left as 0. This is usally a non-zero value.
[openstudio.measure.OSRunner] <Warn> The Moisture Equation Coefficient B has been left as 0. This is usally a non-zero value.
[openstudio.measure.OSRunner] <Warn> The Moisture Equation Coefficient C has been left as 0. This is usally a non-zero value.
[openstudio.measure.OSRunner] <Warn> The Moisture Equation Coefficient D has been left as 0. This is usally a non-zero value.
jmarrec commented 8 months ago

cf https://github.com/NREL/OpenStudio/commit/a8773366dedb6c478c9b01261c6293b8902f4a75

The registerInitialCondition/FinalCondition logs an Info level. Maybe I should have used a higher level? I should probably have added the prefix "Initial Condition" though.

This is meant to mimic stuff that the workflow-gem used to do, eg https://github.com/NREL/OpenStudio-workflow-gem/blob/696306b5b24cd597edc6ab04faa1e618f99db190/lib/openstudio/workflow/adapters/output/socket.rb#L52-L70

jmarrec commented 8 months ago

I should probably have added the prefix Initial Condition: (Final too) at least. Maybe log to stdout directly instead of going with a LOG(Info).

chriswmackey commented 6 months ago

@jmarrec ,

I think this may be connected to a larger issue that we have been experiencing and I wanted to post the question here just to see whether it merits a separate GitHub issue.

@macumber wrote a custom adapter for us to read out the simulation progress of EnergyPlus whenever we run a reporting measure (otherwise we tend to get people reporting that the simulation progress is frozen). This custom adapter inherits from the OpenStudio::Workflow::OutputAdapter module that you linked to there. Here it is on our GitHub:

https://github.com/ladybug-tools/honeybee-openstudio-gem/blob/master/lib/files/honeybee_adapter.rb

In OpenStudio 3.6.1, the adapter gave us the full simulation progress of EnergyPlus in the stdout but, in OpenStudio 3.7, the only thing we get is:

'C:/Program' is not recognized as an internal or external command,
operable program or batch file.
RunEnergyPlus: Completed Successfully with 0 Fatal Errors, 0 Severe Errors, 26 Warnings.

I have had a hard time tracking down where the error message is coming from but we are running OpenStudio out of the C:/Program Files/ folder on the user's machine since this is the only location that the IT overlords of our customers will accept.

But I sense there is some part of your new implementation where you are forgetting to put a file path in double-quotation marks, making the adapter un-usable whenever the simulation is run from a directory with a space in it.

In any event, let me know if this is related to this issue or whether I should open up a separate one.

Thanks as always.

jmarrec commented 6 months ago

@chriswmackey Are you using the "classic" subcommand or the default, C++ one?

The OutputAdapter wasn't ported to C++ yet... I guess this proves that people actually relied on it :)

chriswmackey commented 6 months ago

Thanks for the response, @jmarrec . I was originally using the default C++ one but I just tried it with the "classic" subcommand and it gives me the exact same result:

'C:/Program' is not recognized as an internal or external command,
operable program or batch file.
RunEnergyPlus: Completed Successfully with 0 Fatal Errors, 0 Severe Errors, 26 Warnings.

Does this mean that this is a separate issue from the one that was originally brought up here? If so, I'll be happy to open a new issue.

jmarrec commented 6 months ago

@chriswmackey Yeah open a new issue. Chew the investigation for me as much as you reasonably can please. Ideally a MCVE, where I can just try one (or a few) commands to reproduce. I'll need to boot windows (🤮 ) to begin with, and I can't spend hours trying to guess how to recreate the issue.

Have you started by just using a dummy workflow (the Examples/compact_osw in your install for eg, copy it to a writable directory) and running it from the CLI directly? It's possible that fails directly, since the default CLI location is C:/openstudio-xxx it could have been overlooked.

chriswmackey commented 5 months ago

Thanks, @jmarrec . You can see I opened a separate issue and, after doing some tests, I think I found a way for you to avoid booting Windows in order to recreate the problem. It appears to be very different from this issue that @macumber opened so we'll continue the conversation on the new issue that I opened.