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

Option to enable Verbose output to run tab + styled fallback when no socket #513

Merged
merged 13 commits into from
May 3, 2022

Conversation

jmarrec
Copy link
Collaborator

@jmarrec jmarrec commented May 2, 2022

  • Fixes Hard to see errors generated in ModelToIdf job in App #110

  • Fixes No error message for failed simulation with empty spaces #437

  • Add a "verbose output" checkbox to the run tab

  • This is saved in the user preferences

  • When enabled, this will do openstudio --verbose run -w workflow.osw, and style the stdout/stderr

  • When no socket can be opened, the fallback will also be styled similarly to when sockets are used

    • This cannot be the default behavior though (I'm keeping the explicit socket first in the line) because during the E+ sims the updates don't come directly, there may be a stdout flush missing in the openstudio_cli.rb

Verbose_simulation_output

@jmarrec jmarrec added Enhancement Request New feature or request Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. component - UI labels May 2, 2022
@jmarrec jmarrec requested a review from macumber May 2, 2022 15:44
@jmarrec jmarrec self-assigned this May 2, 2022
} else {
// If not verbose, we save the stdout/stderr to a file, like historical
// Actually, we don't, we just read it
// m_runProcess->setStandardOutputFile(toQString(stdoutPath));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will E+ stdout and stderr still be in files in the sim dir for the user to read later? I see stdout-energyplus and stderr-energyplus files but I'm not entirely sure which process was writing those, the app or the cli.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stdout-energyplus stderr-energyplus are written by workflow gem.

What I am no longer outputing is the model_companion_dir/stdout and stderr files. I see no value in keeping those, when we are seeing them live. I don't see any use case for seeing them afterwards, but perhaps there is one. In which case the RunView::readyReadStandardOutput and RunView::readyReadStandardError should be writting to these files in append mode in addition of displaying them live. thoughts @macumber ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok yeah, I don't think those two files are useful. Thanks Julian!

Copy link
Collaborator Author

@jmarrec jmarrec May 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cf dd97abe

Copy link
Collaborator

@macumber macumber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just want to make sure the user still has stdout and stderr in files for later

@jmarrec jmarrec merged commit 4b78578 into develop May 3, 2022
@jmarrec jmarrec deleted the 110_workflow_stdout branch May 3, 2022 09:01
@github-actions github-actions bot locked and limited conversation to collaborators May 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component - UI Enhancement Request New feature or request Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge.
Projects
None yet
2 participants