Fix #5164 - Unless --show-stdout, do not print any ExpandObjects / EnergyPlus messages #5169

jmarrec · 2024-04-25T08:00:20Z

Pull request overview

Fix CLI: Allow disabling EnergyPlus result output #5164

Pull Request Author

Model API Changes / Additions
Any new or modified fields have been implemented in the EnergyPlus ForwardTranslator (and ReverseTranslator as appropriate)
Model API methods are tested (in src/model/test)
EnergyPlus ForwardTranslator Tests (in src/energyplus/Test)
If a new object or method, added a test in NREL/OpenStudio-resources: Add Link
If needed, added VersionTranslation rules for the objects (src/osversion/VersionTranslator.cpp)
Verified that C# bindings built fine on Windows, partial classes used as needed, etc.
All new and existing tests passes
If methods have been deprecated, update rest of code to use the new methods

Labels:

If change to an IDD file, add the label IDDChange
If breaking existing API, add the label APIChange
If deemed ready, add label Pull Request - Ready for CI so that CI builds your PR

Review Checklist

This will not be exhaustively relevant to every PR.

Perform a Code Review on GitHub
Code Style, strip trailing whitespace, etc.
All related changes have been implemented: model changes, model tests, FT changes, FT tests, VersionTranslation, OS App
Labeling is ok
If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
If feature, test running new feature, try creative ways to break it
CI status: all green or justified

jmarrec

This is mimic'ing the way openstudio classic run -w workflow.osw would do it, completely silently.

IMHO this feels weird having zero output when it just works.

I'm thinking we should have --show-stdout (and maybe --style-stdout as being the default. and you could pass --no-show-stdout to shush it, but that's only my opinion. @kbenne FYI

jmarrec · 2024-04-25T08:00:48Z

src/workflow/RunEnergyPlus.cpp

@@ -154,14 +154,30 @@ void OSWorkflow::runEnergyPlus() {
    // TODO: is this the right place /Do we want to do that if we chose epJSON?
    detailedTimeBlock("Saving IDF", [this, &inIDF] { workspace_->save(inIDF, true); });

+    std::ofstream stdout_ofs(openstudio::toString(runDirPath / "stdout-energyplus"), std::ofstream::trunc);


Declare it higher, to trunc it first, because I'm going to pipe expand objects output to it as well.

jmarrec · 2024-04-25T08:01:23Z

src/workflow/RunEnergyPlus.cpp

+      detailedTimeBlock("Running ExpandObjects", [this, /*&cmd,*/ &result, &runDirPath, &runDirResults, &stdout_ofs] {
+        // result = std::system(cmd.c_str());
+        namespace bp = boost::process;
+        bp::ipstream is;
+        std::string line;
+        bp::child c(runDirResults.expandObjectsExe, bp::std_out > is);
+        while (c.running() && std::getline(is, line)) {
+          stdout_ofs << openstudio::ascii_trim_right(line) << '\n';  // Fix for windows...
+          if (m_show_stdout) {
+            fmt::print("{}\n", line);
+          }
+        }
+        c.wait();
+        result = c.exit_code();
+      });


Swallow the ExpandObjects stdout, otherwise you'll still get these

ExpandObjects Started. No expanded file generated. ExpandObjects Finished. Time: 0.021

jmarrec · 2024-04-25T08:02:12Z

src/workflow/RunEnergyPlus.cpp

        std::string line;
-        // bp::child c(cmd, bp::std_out > is);
-        bp::child c(runDirResults.energyPlusExe, inIDF.filename(), bp::std_out > is);
+        bp::child c(runDirResults.energyPlusExe, inIDF.filename(), (bp::std_out & bp::std_err) > is);


Pipe both stdout and stderr for E+, because the final EnergyPlus Completed Successfully. is printed to stderr and we want to swallow it as well.

jmarrec · 2024-04-25T08:02:50Z

src/workflow/RunEnergyPlus.cpp

+        if (m_show_stdout) {
+          if (m_style_stdout) {
+            fmt::print("RunEnergyPlus: {} with ", status);
+            fmt::print(fmt::fg(fmt::color::red), "{} Fatal Errors, ", errFile.fatalErrors().size());
+            fmt::print(fmt::fg(fmt::color::orange), "{} Severe Errors, ", errFile.severeErrors().size());
+            fmt::print(fmt::fg(fmt::color::yellow), "{} Warnings.", errFile.warnings().size());
+            fmt::print("\n");
+          } else {
+            fmt::print("RunEnergyPlus: {} with {} Fatal Errors, {} Severe Errors, {} Warnings.\n", status, errFile.fatalErrors().size(),
+                       errFile.severeErrors().size(), errFile.warnings().size());
+          }
+        }


Unless --show-stdout is passed: do not print the numer of fatal/severe/warnings

And do not style it (colors) unless --style-stdout is passed.

jmarrec · 2024-04-25T08:36:53Z

src/cli/main.cpp

+    auto* const logLevelOpt = app
+                                .add_option_function<LogLevel>(
+                                  "-l,--loglevel",
+                                  [](const LogLevel& level) {
+                                    const auto loglLevelStr = logLevelStrs[static_cast<size_t>(level) - static_cast<size_t>(LogLevel::Trace)];
+                                    openstudio::Logger::instance().standardOutLogger().setLogLevel(level);
+                                    LOG_FREE(Debug, "openstudio.CLI", "Setting Log Level to " << loglLevelStr << "(" << level << ")");
+                                  },
+                                  "LogLevel settings: One of {Trace, Debug, Info, Warn, Error, Fatal} [Default: Warn] Excludes: --verbose")
+                                ->excludes(verboseOpt)
+                                ->option_text("LEVEL")
+                                ->transform(CLI::CheckedTransformer(logLevelMap, CLI::ignore_case));


Don't print the "Setting Log Level to xx (n)" unless Debug or Trace.

jmarrec · 2024-04-25T08:37:58Z

src/utilities/xml/XMLValidator.cpp

@@ -140,7 +140,7 @@ openstudio::path XMLValidator::schematronToXslt(const openstudio::path& schemaPa
  saveXmlDocToFile(xsltPath, compiled);

  xmlFreeDoc(compiled);
-  LOG(Info, "Saved transformed XSLT Stylesheet at '" << toString(xsltPath) << "'.");
+  LOG(Trace, "Saved transformed XSLT Stylesheet at '" << toString(xsltPath) << "'.");


e7f46c8 is optional, but I'm sick and tired of seeing too much verbosity in XMLValidator (which I'm responsible for, mind you), so I downgraded every Debug/Info message to a Trace.

You shouldn't have to think about what it's doing, and if you do need to debug something, just use Trace

ci-commercialbuildings · 2024-04-25T09:17:47Z

CI Results for e7f46c8:

Ubuntu 20.04:
Ubuntu 22.04:
- OpenStudio-3.8.0-beta+e7f46c8dd5-Ubuntu-22.04-x86_64.deb
- OpenStudio-3.8.0-beta+e7f46c8dd5-Ubuntu-22.04-x86_64.tar.gz
Darwin:
Windows:
Code Coverage:

…ergyPlus messages

…ebug or Trace

…annoying

shorowit · 2024-05-02T20:58:14Z

Thanks @jmarrec!

jmarrec added Enhancement Request component - CLI component - Workflow Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Apr 25, 2024

jmarrec requested a review from shorowit April 25, 2024 08:00

jmarrec self-assigned this Apr 25, 2024

jmarrec commented Apr 25, 2024

View reviewed changes

jmarrec force-pushed the 5164_shush_eplus branch 3 times, most recently from 9920ad1 to 6b23cad Compare April 25, 2024 12:58

jmarrec added this to the OpenStudio SDK 3.8.0 milestone Apr 29, 2024

jmarrec added 4 commits May 2, 2024 22:48

Fix #5164 - Unless --show-stdout, do not print any ExpandObjects / En…

9d05273

…ergyPlus messages

Do not print in color unless --style-stdout is asked for...

6b2d1ca

Shush the "Setting the Log Level to XXX" messages in the CLI unless D…

cfe3d67

…ebug or Trace

Downgrade the XMLValidator Infos / Debug to a Trace. They are really …

c0a25aa

…annoying

jmarrec force-pushed the 5164_shush_eplus branch from 6b23cad to c0a25aa Compare May 2, 2024 20:48

kbenne merged commit 7d0fedf into develop May 2, 2024
2 of 6 checks passed

kbenne deleted the 5164_shush_eplus branch May 2, 2024 20:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #5164 - Unless --show-stdout, do not print any ExpandObjects / EnergyPlus messages #5169

Fix #5164 - Unless --show-stdout, do not print any ExpandObjects / EnergyPlus messages #5169

jmarrec commented Apr 25, 2024

jmarrec left a comment

jmarrec Apr 25, 2024

jmarrec Apr 25, 2024

jmarrec Apr 25, 2024

jmarrec Apr 25, 2024

jmarrec Apr 25, 2024

jmarrec Apr 25, 2024

ci-commercialbuildings commented Apr 25, 2024 •

edited

Loading

shorowit commented May 2, 2024

Fix #5164 - Unless --show-stdout, do not print any ExpandObjects / EnergyPlus messages #5169

Fix #5164 - Unless --show-stdout, do not print any ExpandObjects / EnergyPlus messages #5169

Conversation

jmarrec commented Apr 25, 2024

Pull request overview

Pull Request Author

Review Checklist

jmarrec left a comment

Choose a reason for hiding this comment

jmarrec Apr 25, 2024

Choose a reason for hiding this comment

jmarrec Apr 25, 2024

Choose a reason for hiding this comment

jmarrec Apr 25, 2024

Choose a reason for hiding this comment

jmarrec Apr 25, 2024

Choose a reason for hiding this comment

jmarrec Apr 25, 2024

Choose a reason for hiding this comment

jmarrec Apr 25, 2024

Choose a reason for hiding this comment

ci-commercialbuildings commented Apr 25, 2024 • edited Loading

shorowit commented May 2, 2024

ci-commercialbuildings commented Apr 25, 2024 •

edited

Loading