Reformat eplusout.dbg file for easier excel viewing

[rraustad]

Pull request overview

• Fixes a format issue with eplusout.dbg where the results can now be plotted in excel.
• The previous format was not conducive to plotting data.

NOTE: ENHANCEMENTS MUST FOLLOW A SUBMISSION PROCESS INCLUDING A FEATURE PROPOSAL AND DESIGN DOCUMENT PRIOR TO SUBMITTING CODE

Pull Request Author

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

• Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
• Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
• Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
• Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
• If any diffs are expected, author must demonstrate they are justified using plots and descriptions
• If changes fix a defect, the fix should be demonstrated in plots and descriptions
• If any defect files are updated to a more recent version, upload new versions here or on DevSupport
• If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
• If structural output changes, add to output rules file and add OutputChange label
• If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies

Reviewer

This will not be exhaustively relevant to every PR.

• Perform a Code Review on GitHub
• If branch is behind develop, merge develop and build locally to check for side effects of the merge
• 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
• Check that performance is not impacted (CI Linux results include performance check)
• Run Unit Test(s) locally
• Check any new function arguments for performance impacts
• Verify IDF naming conventions and styles, memos and notes and defaults
• If new idf included, locally check the err file and other outputs

[rraustad]

Previous format, shown in Notepad, which when pulled into excel did not allow plotting of the data:

New format, shown in excel, which allows easy plotting of the data:

[nrel-bot-3]

@rraustad @Myoldmopar it has been 28 days since this pull request was last updated.

[rraustad]

No defect file for this branch. I took an existing file and made results for develop and this branch to make this review easier. The excel files are included but can also open these dbg files from excel and choose comma delimited to create the excel files in this zip file.

FurnaceWithDXSystem_DebugData.zip

This is certainly much easier to use in excel (plot from this branch).

[rraustad]

I kinda liked the header of the debug output. The new eplusout.dbl, debug node list file, shows what data is in the eplusout.dbg csv file.

node #   Node Type      Name
   1        Air         ZONE 1 INLET NODE
   2        Air         ZONE 2 INLET NODE
   3        Air         ZONE 3 INLET NODE
   4        Air         AIR LOOP INLET NODE

[rraustad]

The discussion here would be 1- whether to move the header node list to a separate file (eplusout.dbl) or keep both the header and data in the same csv file (eplusout.dbg), and 2- I saw other places where debug output is used and maybe those areas of code also need mods.

[mjwitte]

The discussion here would be 1- whether to move the header node list to a separate file (eplusout.dbl) or keep both the header and data in the same csv file (eplusout.dbg), and 2- I saw other places where debug output is used and maybe those areas of code also need mods.

1. I would drop the header. The same information is at the top of the bnd output, and for runs with many nodes, it would mean a lot of extra lines at the top of the dbg file.
2. That's up to you @rraustad if you want to dig deeper on this. I personally haven't used debug output in ages (I usually add my own focused output), but if you want to update it, go ahead.

[Myoldmopar]

My gut says we should try to do a quick wrap on any changes of this for the I/O freeze, or push until the next release. Even though this file is likely not used much, I'd hate to make structural changes after I/O freeze. Given that, @rraustad and @mjwitte do you think we should:

• change the scope of this to make fewer changes and get something cleaned up for 24.1 I/O freeze?
• forge ahead with enough effort to get all changes merged for 24.1 I/O freeze?
• just punt with any changes until 24.2?

[nrel-bot]

@Myoldmopar @rraustad @Myoldmopar it has been 28 days since this pull request was last updated.

[nrel-bot-2b]

@Myoldmopar @rraustad @Myoldmopar it has been 28 days since this pull request was last updated.

[rraustad]

I have been using this branch for detailed testing of simulations so I think this can be safely merged.

[mjwitte]

@rraustad Testing with some other files. May add some additional values. And will update the I/O Ref which still talks about inputs of 1 and 0 instead of Yes/No. Also, would it be useful to add a third input field for Zone or HVAC timestep?

[mjwitte]

@rraustad Thanks for making this useful.

[rraustad]

Also, would it be useful to add a third input field for Zone or HVAC timestep?

If this is already reporting at the HVAC time step I see only a minor benefit of reporting averages at the zone time step so that graphs are easier to interpret. The other thing I thought of was to supplement ReportDuringWarmup with a flag for ReportDuringSizing and then this debug code would be nearly obsolete except for the fact that this method does not require adding node reports to the input file (but how hard is that really to add a few node reports using wildcards). There could certainly be improvements to what and how this debug data reports but I am more interested at this point to putting this to good use.

[mjwitte]

Also, would it be useful to add a third input field for Zone or HVAC timestep?

If this is already reporting at the HVAC time step I see only a minor benefit of reporting averages at the zone time step so that graphs are easier to interpret. The other thing I thought of was to supplement ReportDuringWarmup with a flag for ReportDuringSizing and then this debug code would be nearly obsolete

I asked this because the docs (somewhere) said it reports debug output at the zone timestep. If it's already reporting at the HVAC timestep, then we can just update the docs. And I didn't realize this reports during sizing, so we should mention that, too in the I/O Ref.

[rraustad]

I assumed it was reporting at the HVAC time step give the location of code in HVACManager. I can't tell from the time stamp what frequency is being reported.

[rraustad]

I just verified reporting during sizing. And this does look like reports for the HVAC time step.

[rraustad]

This line is the issue for reporting time correctly.

[rraustad]

From what I see now, the debug data is reporting at the zone time step. I see the zone sizing getting hit more times when downshifting and then debug reports at the end of the time step. So the doc is correct.

[rraustad]

@Myoldmopar the last thing needed here is to get the list of nodes and the column headings correct. This data is reported very early in the simulation, before air loop and plant nodes are read in. So Node().size is less than what it will be when the simulation hits state.dataGlobal->BeginEnvrnFlag = true and as a result the list of nodes and the column headings are incomplete as shown in the figure. I thought I could just close the file, read that file into memory, write the header, then add in what was previously written similar to this. No luck trying.

if (!state.dataHVACMgr->DebugNamesReported && state.dataGlobal->BeginEnvrnFlag && !state.dataLoopNodes->Node.empty()) {
    state.files.debug.close();
    std::ifstream file("eplusout.dbg");
    std::stringstream stream << file.rdbuf();
    state.files.debug.open();
        // print all node names and entire column heading here, cut-n-paste from existing code in this branch (too long to show here)
    state.files.debug << stream.rdbuf();
    state.dataHVACMgr->DebugNamesReported = true;
}

[rraustad]

Interesting. Adding a 2nd column heading "row" where the number of nodes increases does not seem to impact a column plot. I can't even tell where the row of text is, like excel just ignores it. If I zoom way in to 5 rows before that row of text and 5 after I can see that data point, which plots as 0.

Now if I repeat the entire node list (which is a text string in column A many rows long) you do see a gap in the data (for the first set of data (columns) only reported during sizing), if you zoom in. I zoomed in to 30 rows before the column heading and 30 rows after the list of nodes. Again, this is only for the first set of sizing data.

There's no impact at all on the new node data reported after zone sizing since there is just a bigger gap of blank rows before getting to the new column heading row and the data directly below that heading. Plotting the entire data set of OAT in column D, you can't even see that gap (row 2771 to 2846, but this gap could be hundreds of rows).

[rraustad]

This is workable for now so I think this is ready to merge. The initial size of Node() is 158 when the node list and column headers are reported. When sizing is complete and other objects are read in the size of Node() increases to 389 and a new node list and column header is reported to the dbg file. The figure shown is one of the nodes that was not initially present and reports after sizing is complete (a plenum node, the plenum objects are read in after sizing).

Used to report debug node data and also during warmup:

Output:DebuggingData,Yes,Yes;
Output:Diagnostics, ReportDuringWarmup;

[Myoldmopar]

The EpJSON failure is unrelated and fixed in develop. I'm inclined to merge this, and we can always tweak it further later. Thanks @rraustad !