Fixes and Documentation for Performance Overrides and Reporting #7822

JasonGlazer · 2020-02-27T18:18:47Z

Some tasks were identified prior to the I/O freeze for #7761 that are being completed in the pull request. This includes some code fixes but primarily is the documentation related to that work.

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

Reviewer

This will not be exhaustively relevant to every PR.

JasonGlazer · 2020-03-10T20:02:50Z

Dr. Latex (@Myoldmopar) could you look at the tables I just added? They have a lot of columns and could probably be made to render better.

@mjwitte could you look over the IOref docs for the PerformancePrecisionTradeoffs object that is new and let me know what you think?

…ists.txt

JasonGlazer · 2020-03-11T17:29:03Z

@mjwitte I ended up making updates to the "classic" EP-Launch for this update, could you please do some checking when you test the next installer?

JasonGlazer · 2020-03-13T12:22:32Z

Fixed the unit test that was failing but I did not see any issue with HospitalLowEnergy when run locally. Maybe it is just a timeout since it runs so slowly.

Myoldmopar · 2020-03-13T19:35:45Z

I pushed a change that you can evaluate @JasonGlazer. It uses footnotes for long column names to help fit very wide tables onto a normal page. If you like it, you can adapt it and use it as you need. If not, just discard those changes. Here's one table I modified in this branch now:

JasonGlazer · 2020-03-16T15:24:37Z

@Myoldmopar Is there a way to indicate where the wrapping should occur? I think that might be better than using abbreviations.

Myoldmopar · 2020-03-16T15:37:42Z

With a tabularx environment, you can use a \newline to force a break, I'm not sure of a similar thing in the regular tabular environment. I think no matter how you break it, it's going to be a little bit off. It's one of those things where if you play nice with LaTeX, it will make beautiful documents. Once you start manipulating things in custom ways, it's a constant battle. My suggestion would be to take a second pass at maybe presenting the data in a different way that doesn't require this much header text. If you can't come up with anything, I would try to create shorthands, like abbreviations, or at least less text. If you still don't like that, then we would need to consider switching to tabularx or something which could be problematic if some devs don't have that package installed (I can't recall if it is in the minimal LaTeX distro).

JasonGlazer · 2020-03-16T17:51:45Z

Hopefully the tables are good enough.

JasonGlazer · 2020-03-17T11:44:48Z

@Myoldmopar and @mjwitte I would guess these failed tests don't have anything to do with my code but just that it has already fallen behind develop. Should I remerge develop or would you rather not have me put more onto the CI machines?

Myoldmopar · 2020-03-17T13:29:56Z

@JasonGlazer The convection unit test is unrelated, and I'm hoping I have a fix in a different branch that will merge soon.

However, the two performance log unit tests are popping up in a bunch of branches spuriously. It looks to me like it could be due to one of two things.

The most likely culprit is that you are reading/writing to the same perflog csv file name in each unit test. Because the unit tests run in parallel, the timing of the unit tests is likely hitting the files at different times, and sometimes there is data in there already. I would suggest changing the names of the files used to be something different between the two. And maybe something that is obviously made up like "unit_test_perflog_file_1.csv" and "...flog_file_2.csv".
The other thing that could be doing it is the use of statics in there. I see how you are handling it and I don't think that's the root cause of this, but it could still be problematic later. The ideal design here would be that the performance log would be managed by its own class, with member methods like addHeader(...), and addBody(...), and writeContents(...). Then you wouldn't have to have a single method with all this responsibility. At a minimum, you need to make sure you are calling to clear those static variables from a clear_state call. Or, you could move them to module level variables and reset them from a clear_state call.

I would advise you to make the changes to the unit test file names, and make sure you are calling to clear those variables during the clear_state process. Then pull in develop one more time, CI should be happier, and we'll get this merged.

Myoldmopar · 2020-03-17T15:33:14Z

doc/output-details-and-examples/src/output-files/eplusout-perflogcsv.tex

+
+\begin{lstlisting}
+
+\end{lstlisting}


Did you mean to put something in this block?

Good question

No need for this, I put examples in the IOref docs

Myoldmopar · 2020-03-17T15:38:18Z

doc/input-output-reference/src/overview/group-simulation-parameters.tex

+13 & FALSE & CarrollMRT & MODE05 & 1 & 1 & TRUE  & 1.00 & 32.76  \tabularnewline
+\bottomrule
+\end{longtable}
+}


The tables look good in the CI built IORef.

…. Made use of two files for append perflog unit tests

…tput-files/eplusout-perflogcsv.tex

JasonGlazer · 2020-03-18T13:41:51Z

@Myoldmopar I think I addressed everything you brought up.

Myoldmopar · 2020-03-18T14:16:20Z

Excellent! I'll take a final look and we'll get this in.

Myoldmopar

OK, I think I'll just build this locally and test it quickly instead of waiting on CI to do it all. Save the CI cycles for other PRs.

Thanks for cleaning all this up, this is much nicer.

Myoldmopar · 2020-03-18T14:20:38Z

doc/output-details-and-examples/src/output-files/eplusout-perflogcsv.tex

@@ -62,8 +62,3 @@ \section{eplusout\_perflog.csv}
 When using this file it is good to set PerformancePrecisionTradeoffs to default values the first time to establish a baseline then change one input at a time and rerunning to understand the impact of that input. The total runtime decrease 
 and the impact on energy and the ocscillation variables should be considered prior to committing to a specific PerformancePrecisionTradeoffs scheme.

-An example is shown below:


Myoldmopar · 2020-03-18T14:21:12Z

src/EnergyPlus/StateManagement.cc

@@ -404,6 +404,7 @@ void EnergyPlus::clearAllStates()
    UnitHeater::clear_state();
    UnitVentilator::clear_state();
    UserDefinedComponents::clear_state();
+    UtilityRoutines::clear_state();


Very nice. There was a time when clear_states were being added almost daily, but now it's a very rare thing for a namespace/module to not have one. This is good.

Myoldmopar · 2020-03-18T14:26:21Z

src/EnergyPlus/UtilityRoutines.cc

@@ -99,6 +99,14 @@ namespace UtilityRoutines {
    bool outputErrorHeader(true);
    ObjexxFCL::gio::Fmt fmtLD("*");
    ObjexxFCL::gio::Fmt fmtA("(A)");
+    std::string appendPerfLog_headerRow("");


I see that these are only in the cc file, not the header. That is a very good thing. There is no reason the implementation of this needs to be exposed to any other file, and the clear_state function clears these. 👍

Myoldmopar · 2020-03-18T14:27:24Z

src/EnergyPlus/UtilityRoutines.cc

        // the following was added for unit testing to clear the static strings
        if (colHeader == "RESET" && colValue == "RESET") {
-            headerRow = "";
-            valuesRow = "";
+            appendPerfLog_headerRow = "";


I think a more ideal thing would be to just have a function called clearPerfLogHeaders() rather than relying on special header names to clear these. But that is OK for right now. Next time you work on this area, please clean this out.

Myoldmopar · 2020-03-18T14:28:04Z

tst/EnergyPlus/unit/UtilityRoutines.unit.cc

@@ -179,7 +179,7 @@ TEST_F(EnergyPlusFixture, UtilityRoutines_appendPerfLog2)
    // make sure the static variables are cleared
    UtilityRoutines::appendPerfLog("RESET", "RESET");

-    DataStringGlobals::outputPerfLogFileName = "eplusout_perflog.csv";
+    DataStringGlobals::outputPerfLogFileName = "eplusout_2_perflog.csv";


Myoldmopar · 2020-03-18T15:11:23Z

The only failure is the AirTerminalSingleDuctMixer_SimPTAC_ATMInletSide unit test failure that has suddenly popped up, not just in this branch, but others also. This looks good to go now, I'm not going to wait on other CI machines to run. Merging.

JasonGlazer added 4 commits February 27, 2020 09:29

Fix createPerfLog variable name

41ae56e

Fix OverrideZoneAirSolutionAlgo comment

966970d

fix createPerfLog in one more place

ad3aa63

Only compute oscillations when variables requested

0962274

JasonGlazer added the NewFeature Includes code to add a new feature to EnergyPlus label Feb 27, 2020

JasonGlazer added this to the EnergyPlus 9.3.0 milestone Feb 27, 2020

JasonGlazer self-assigned this Feb 27, 2020

JasonGlazer added 11 commits March 2, 2020 09:23

Remove use of ObjexxFCL:gio to detect if file exists

3e2b8ea

Add oscillating outputs to perflog

9fafcb8

Trigger oscillation variables if PerformancePrecision object is present.

2ab0ae1

Merge branch 'develop' into PerfOverride_Part2

be5d3d4

Add IOref section on new fields in PerformancePrecisionTradeoffs

d204656

Add documentation for new oscillation output variables

15e0a79

Add perflog.csv to output details and example doc

0d6088e

Adding to ioref documentation for PerformancePrecisionTradeoffs

32fa514

Merge branch 'develop' into PerfOverride_Part2

4218cec

Fix error in gathering hours and output variable values

52493c6

Finish up ioref doc changes. Fix significant digits in _perflog.csv

f69ed28

JasonGlazer added 9 commits March 10, 2020 15:06

Merge branch 'develop' into PerfOverride_Part2

667023c

Re-add the perflog.csv part in the output-details-and-examples/CMakeL…

b4e03fb

…ists.txt

Merge branch 'develop' into PerfOverride_Part2

95ddd15

Remove line responsible for changing RDD and Audit files

428ca3f

updated workflow for EP-Launch 3

f20d3f2

Add support for _perflog.csv to RunEPlus.bat

5cd8f52

Add _perflog.csv support to Epl-run.bat

efe8843

Update EP-launch to support new perflog.csv file and update help menu

36b32dd

Merge branch 'develop' into PerfOverride_Part2

1533808

JasonGlazer mentioned this pull request Mar 11, 2020

Update EP-Launch and IDF Editor links to acknowledgments doc #6379

Closed

3 tasks

JasonGlazer added 3 commits March 12, 2020 06:10

Merge branch 'develop' into PerfOverride_Part2

2df3572

Merge branch 'develop' into PerfOverride_Part2

6c606c1

Fix unit test to show more significant figures.

dd935c5

Myoldmopar added 3 commits March 13, 2020 11:53

Merge remote-tracking branch 'origin/develop' into PerfOverride_Part2

f212d86

Use footnotes for long column header names

adedbd8

Merge remote-tracking branch 'origin/develop' into PerfOverride_Part2

9533b0b

JasonGlazer added 2 commits March 16, 2020 10:49

Merge branch 'develop' into PerfOverride_Part2

6a873ff

Neatened up tables.

0c7bf81

Myoldmopar reviewed Mar 17, 2020

View reviewed changes

JasonGlazer added 4 commits March 17, 2020 10:47

Hoist statics to the module and add clear_state to UtilityRoutines.cc…

b448f97

…. Made use of two files for append perflog unit tests

Merge branch 'develop' into PerfOverride_Part2

9e8e258

Merge branch 'develop' into PerfOverride_Part2

58977be

Removed unused stub for example in output-details-and-examples/src/ou…

73c5b2d

…tput-files/eplusout-perflogcsv.tex

Myoldmopar approved these changes Mar 18, 2020

View reviewed changes

Myoldmopar merged commit 4836252 into develop Mar 18, 2020

Myoldmopar deleted the PerfOverride_Part2 branch March 18, 2020 15:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes and Documentation for Performance Overrides and Reporting #7822

Fixes and Documentation for Performance Overrides and Reporting #7822

JasonGlazer commented Feb 27, 2020

JasonGlazer commented Mar 10, 2020

JasonGlazer commented Mar 11, 2020

JasonGlazer commented Mar 13, 2020

Myoldmopar commented Mar 13, 2020

JasonGlazer commented Mar 16, 2020

Myoldmopar commented Mar 16, 2020

JasonGlazer commented Mar 16, 2020

JasonGlazer commented Mar 17, 2020

Myoldmopar commented Mar 17, 2020

Myoldmopar Mar 17, 2020

JasonGlazer Mar 18, 2020

JasonGlazer Mar 18, 2020

Myoldmopar Mar 17, 2020

JasonGlazer commented Mar 18, 2020

Myoldmopar commented Mar 18, 2020

Myoldmopar left a comment

Myoldmopar Mar 18, 2020

Myoldmopar Mar 18, 2020

Myoldmopar Mar 18, 2020

Myoldmopar Mar 18, 2020

Myoldmopar Mar 18, 2020

Myoldmopar commented Mar 18, 2020


		\begin{lstlisting}

		\end{lstlisting}

Fixes and Documentation for Performance Overrides and Reporting #7822

Fixes and Documentation for Performance Overrides and Reporting #7822

Conversation

JasonGlazer commented Feb 27, 2020

Pull Request Author

Reviewer

JasonGlazer commented Mar 10, 2020

JasonGlazer commented Mar 11, 2020

JasonGlazer commented Mar 13, 2020

Myoldmopar commented Mar 13, 2020

JasonGlazer commented Mar 16, 2020

Myoldmopar commented Mar 16, 2020

JasonGlazer commented Mar 16, 2020

JasonGlazer commented Mar 17, 2020

Myoldmopar commented Mar 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JasonGlazer commented Mar 18, 2020

Myoldmopar commented Mar 18, 2020

Myoldmopar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Myoldmopar commented Mar 18, 2020