Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
macumber opened this issue Jan 2, 2024 · 8 comments
Open

Comments

@macumber
Copy link
Contributor

macumber commented Jan 2, 2024

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):

  • Platform (Operating system, version): Windows 11
  • Version of OpenStudio (if using an intermediate build, include SHA): 3.7.0

Context

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

@macumber macumber added the Triage Issue needs to be assessed and labeled, further information on reported might be needed label Jan 2, 2024
@macumber
Copy link
Contributor Author

macumber commented Jan 2, 2024

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
Copy link
Collaborator

jmarrec commented Jan 4, 2024

cf a877336

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
Copy link
Collaborator

jmarrec commented Jan 5, 2024

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).

@DavidGoldwasser DavidGoldwasser added severity - Major Bug and removed Triage Issue needs to be assessed and labeled, further information on reported might be needed labels Jan 23, 2024
@DavidGoldwasser DavidGoldwasser added this to the OpenStudio SDK 3.8.0 milestone Jan 23, 2024
@chriswmackey
Copy link

chriswmackey commented Mar 9, 2024

@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
Copy link
Collaborator

jmarrec commented Mar 11, 2024

@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
Copy link

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
Copy link
Collaborator

jmarrec commented Mar 11, 2024

@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
Copy link

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.

@DavidGoldwasser DavidGoldwasser removed this from the OpenStudio SDK 3.8.0 milestone May 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants