Native E+ CSV output and conditional output file selection by mbadams5 · Pull Request #7904 · NatLabRockies/EnergyPlus

mbadams5 · 2020-04-07T02:29:46Z

Pull request overview

This pull request implements a native E+ CSV output capability as well as add a new object to conditionally turn on/off output files.
Fixes #6552

Overall E+ runtime

OutPatient Model (seconds)	original/yes	all/yes	original/no	all/no
9.3 runtime	353.76	2941.5
branch runtime	267.6	1662	308.4	964.8
% reduction	24.4%	43.5%	-15.2%	41.9%
speed up	1.32x	1.77x	0.87x	1.72x

Native E+ vs ReadVarsESO

OutPatient Model (seconds)	original/yes	all/yes
ReadVarsESO	0.156	1782.81
Native E+ CSV	.518	555.6
% reduction	-232.1%	68.8%
speed up	0.3x	3.21x

MediumOffice Model (seconds)	all/yes
ReadVarsESO	183
Native E+ CSV	34.12
% reduction	81.4%
speed up	5.36x

original: OutPatient model as in testfiles
all: OutPatient model with ALL Output:Variables/Output:Meters from RDD/MDD file
yes: Outputs all normal output files during simulation
no: Outputs no output files during simulation using new Output:Control object

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.

…improve_file_io_with_fmt_refactor

…th_fmt_refactor

…improve_file_io_with_fmt_refactor

…th_fmt_refactor

mbadams5 · 2020-04-07T17:28:41Z

The Mac and Linux CI issues are related to #7907 and not this PR.

…th_fmt_refactor

# Conflicts: # src/EnergyPlus/CommandLineInterface.cc # src/EnergyPlus/DElightManagerF.cc # src/EnergyPlus/DataStringGlobals.hh # src/EnergyPlus/DataSystemVariables.cc # src/EnergyPlus/DaylightingManager.cc # src/EnergyPlus/EMSManager.cc # src/EnergyPlus/HeatBalanceManager.cc # src/EnergyPlus/HeatBalanceSurfaceManager.cc # src/EnergyPlus/IOFiles.cc # src/EnergyPlus/OutputFiles.hh # src/EnergyPlus/OutputProcessor.cc # src/EnergyPlus/OutputReportTabular.cc # src/EnergyPlus/OutputReports.cc # src/EnergyPlus/ResultsFramework.cc # src/EnergyPlus/ResultsFramework.hh # src/EnergyPlus/ScheduleManager.cc # src/EnergyPlus/SimulationManager.cc # src/EnergyPlus/SizingManager.cc # src/EnergyPlus/StateManagement.cc # src/EnergyPlus/UtilityRoutines.cc # src/EnergyPlus/WindowManager.cc # src/EnergyPlus/api/EnergyPlusPgm.cc # tst/EnergyPlus/unit/OutputReportTabular.unit.cc

…th_fmt_refactor

Myoldmopar · 2020-08-20T13:43:01Z

Minor build warning popped up on Mac, but otherwise this last commit is looking fantastic. I have not looked at the changes yet, though. If we got the build warning fixed up, does this address the discussion during the call yesterday or are there still more changes to make?

…th_fmt_refactor

Myoldmopar · 2020-08-20T21:38:55Z

Outside of the ERR diffs, this looks good. I'm assuming we don't actually want those warnings popping up in there, right?

I think I'm going to make an "IO-Freeze-Release-Candidate" just to make sure packages are doing OK since it has been a while since we made them on CI. I will plan on closing the IO freeze door by tomorrow morning, so if you can get the ERR diffs cleaned up, that would be great. If not, we can discuss dropping this in with those temporarily in there. Assuming it's otherwise ready to go.

Myoldmopar

This is an incredibly awesome new feature. I am hopeful that the CI results continue to look promising and we can get this merged in right away. I think at this point, even if there are a few rough edges, I'm still inclined to get it in and clean up anything remaining with a follow-up issue.

Myoldmopar · 2020-08-21T01:15:22Z

idd/Energy+.idd.in


+
+OutputControl:Files,
+	   \memo Conditionally turn on/off output from EnergyPlus.


Tab character got left in here.

Myoldmopar · 2020-08-21T01:15:44Z

src/EnergyPlus/CMakeLists.txt

-  ResultsSchema.cc
-  ResultsSchema.hh
+  ResultsFramework.cc
+  ResultsFramework.hh


Myoldmopar · 2020-08-21T02:16:42Z

Gah, tons of diffs due to develop moving.

…th_fmt_refactor

Myoldmopar · 2020-08-21T13:49:02Z

I don't know where that unit test failure on Mac came from, but I don't think it's the changes made here. This looks great. @mbadams5 are you satisfied with where it is at? If so I'll pull it and test it out a bit.

mbadams5 · 2020-08-21T13:58:03Z

@Myoldmopar, yes I think this is ready to go. Please feel free to review it and test it out.

Myoldmopar · 2020-08-21T17:32:05Z

OK, so I built and tested this locally. Some issues, but I think I'm going to merge it in for now to get the I/O side done for freeze.

I took an example file and tried to run it with the new object and got a seg fault. It turns out I entered "output:control:files" instead of "outputcontrol:files". Seg fault because of a bad object name?
I changed the object name and it built fine, but the output files were not responding to my inputs. I found that the get input routine was using the wrong name as well. In the IDD it is "OutputControl:Files", but in the code it was just "Output:Files". So it was not actually reading in my new object
I built with the new name and it was back to a seg fault. I found that in the input processing function, it was looking for keys in the json input processor with the names "json" and "tabular", instead of "output_json" and "output_tabular". Seg fault from bad key lookup.
I then fixed the names it was looking up and it ran, and appeared to generally respond well. Several of the files were omitted which is great, but the meter file seemed to be produced even if I asked it to not be produced.

Obviously there is still some work to do here, but the I/O is settled, and as long as you don't use the new object, EnergyPlus runs fine. I'm going to merge and we can open up a follow-up ticket to address these post-freeze.

Thanks @mbadams5, once polished this is going to be a really great addition!

mjwitte · 2020-09-08T22:34:21Z

src/EnergyPlus/OutputProcessor.cc

+        print(state.files.rdd, "Program Version,{},{}{}", VerString, IDDVerString, DataStringGlobals::NL);
+        print(state.files.rdd, "Var Type (reported time step),Var Report Type,Variable Name [Units]{}", DataStringGlobals::NL);
+
+        print(state.files.mdd, "Program Version,{},{}{}", VerString, IDDVerString, DataStringGlobals::NL);
+        print(state.files.mdd, "Var Type (reported time step),Var Report Type,Variable Name [Units]{}", DataStringGlobals::NL);


@mbadams5 @Myoldmopar This reintroduced DataStringGloblas::NL which was removed in #8178. The rdd and mdd files now have an extra line ending character in the header line 0D 0D 0A. This causes text editors on Windows to prompt to fix line endings.

mbadams5 added 13 commits March 31, 2020 16:28

Initial commit of native CSV output

4629493

Update trying to fix monthly output

92e54c4

Merge remote-tracking branch 'origin/move_bnd_audit_eso_to_fmt' into …

1fa0858

…improve_file_io_with_fmt_refactor

Add Output:Control object to conditionally turn off output files

62ceaa2

Merge remote-tracking branch 'origin/move_bnd_audit_eso_to_fmt' into …

44a5183

…improve_file_io_with_fmt_refactor

Large update which adds most conditional outputs and mtr csv output

fd46ad2

Merge remote-tracking branch 'origin/develop' into improve_file_io_wi…

f270b99

…th_fmt_refactor

Merge remote-tracking branch 'origin/move_bnd_audit_eso_to_fmt' into …

a6279f0

…improve_file_io_with_fmt_refactor

Fix build error and one integration error

5211273

Fix runtime issues and being adding unit tests

a1127f2

Fix potential bugs in results framework reporting

5a3292f

Update to get meteronlyfile working properly

df7db47

Merge remote-tracking branch 'origin/develop' into improve_file_io_wi…

324807c

…th_fmt_refactor

mbadams5 added the Performance Includes code changes that are directed at improving the runtime performance of EnergyPlus label Apr 7, 2020

mbadams5 added this to the EnergyPlus 9.4.0 milestone Apr 7, 2020

mbadams5 requested review from Myoldmopar and mjwitte April 7, 2020 19:28

mbadams5 marked this pull request as ready for review April 7, 2020 19:35

mbadams5 added 11 commits April 7, 2020 16:39

Add YRMeters initialization

2b155bb

Convert to internal CSV output when ReadVarsESO is selected in CLI

c4bc546

Fix Windows build issues

dfb667f

Merge remote-tracking branch 'origin/develop' into improve_file_io_wi…

5eac63c

…th_fmt_refactor

Fix issue with ReadWrite files, mainly delight

963e972

Merge remote-tracking branch 'origin/develop' into improve_file_io_wi…

6308ba7

…th_fmt_refactor

Merge branch 'address_7907' into improve_file_io_with_fmt_refactor

c4df9df

Fixing failing integration tests

56ab68a

Fix bug in Scheduled Output:Variable handling

c4ada85

Fix a few bugs

39418f8

Change name from ResultsSchema to ResultsFramework

f03a598

Myoldmopar added this to the EnergyPlus 9.4.0 IOFreeze milestone Aug 14, 2020

mbadams5 added 4 commits August 19, 2020 14:54

Merge remote-tracking branch 'origin/develop' into improve_file_io_wi…

81d5684

…th_fmt_refactor

Merge remote-tracking branch 'origin/develop' into improve_file_io_wi…

7a83e3d

…th_fmt_refactor

Revert command line readvars

d8219e8

mbadams5 added 3 commits August 20, 2020 11:11

Merge remote-tracking branch 'origin/develop' into improve_file_io_wi…

0313cb8

…th_fmt_refactor

Fix build warning

5b351e5

Fix err file issue

1b3dcea

Myoldmopar reviewed Aug 21, 2020

View reviewed changes

mbadams5 added 3 commits August 20, 2020 22:41

Fix error file reporting

6a05aa1

Merge remote-tracking branch 'origin/develop' into improve_file_io_wi…

cce3df3

…th_fmt_refactor

Fix extra tab

1668cf5

joseph-robertson mentioned this pull request Aug 21, 2020

Update to EnergyPlus 9.4.0 NatLabRockies/OpenStudio#4042

Closed

21 tasks

Myoldmopar merged commit 908c5ce into develop Aug 21, 2020

Myoldmopar deleted the improve_file_io_with_fmt_refactor branch August 21, 2020 17:32

mitchute mentioned this pull request Aug 24, 2020

Output Files Not Being Overwritten #8229

Closed

3 tasks

mjwitte mentioned this pull request Sep 1, 2020

Add Missing Argument on Call to InputOutputFile::open #8236

Merged

20 tasks

Myoldmopar mentioned this pull request Sep 2, 2020

Fix missing MTR output #8246

Merged

20 tasks

jmarrec mentioned this pull request Sep 3, 2020

OutputControl:Files isn't processed correctly. #8249

Closed

3 tasks

This was referenced Sep 3, 2020

csv output problems #8252

Closed

Input errors cause segfault with empty err output #8253

Closed

mjwitte reviewed Sep 8, 2020

View reviewed changes

mbadams5 mentioned this pull request Sep 22, 2020

CSV output, I/O, IDD parser, and file conversion bugfixes #8294

Merged

20 tasks

shorowit mentioned this pull request Mar 17, 2023

Allow more unique objects in reporting measure's energyPlusOutputRequests() NatLabRockies/OpenStudio-workflow-gem#147

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Native E+ CSV output and conditional output file selection#7904

Native E+ CSV output and conditional output file selection#7904
Myoldmopar merged 48 commits intodevelopfrom
improve_file_io_with_fmt_refactor

mbadams5 commented Apr 7, 2020 •

edited by mjwitte

Loading

Uh oh!

mbadams5 commented Apr 7, 2020

Uh oh!

Myoldmopar commented Aug 20, 2020

Uh oh!

Myoldmopar commented Aug 20, 2020

Uh oh!

Myoldmopar left a comment

Uh oh!

Myoldmopar Aug 21, 2020

Uh oh!

Myoldmopar Aug 21, 2020

Uh oh!

Myoldmopar commented Aug 21, 2020

Uh oh!

Myoldmopar commented Aug 21, 2020

Uh oh!

mbadams5 commented Aug 21, 2020

Uh oh!

Myoldmopar commented Aug 21, 2020

Uh oh!

mjwitte Sep 8, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants



		OutputControl:Files,
		\memo Conditionally turn on/off output from EnergyPlus.

Conversation

mbadams5 commented Apr 7, 2020 • edited by mjwitte Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request overview

Pull Request Author

Reviewer

Uh oh!

mbadams5 commented Apr 7, 2020

Uh oh!

Myoldmopar commented Aug 20, 2020

Uh oh!

Myoldmopar commented Aug 20, 2020

Uh oh!

Myoldmopar left a comment

Choose a reason for hiding this comment

Uh oh!

Myoldmopar Aug 21, 2020

Choose a reason for hiding this comment

Uh oh!

Myoldmopar Aug 21, 2020

Choose a reason for hiding this comment

Uh oh!

Myoldmopar commented Aug 21, 2020

Uh oh!

Myoldmopar commented Aug 21, 2020

Uh oh!

mbadams5 commented Aug 21, 2020

Uh oh!

Myoldmopar commented Aug 21, 2020

Uh oh!

mjwitte Sep 8, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

mbadams5 commented Apr 7, 2020 •

edited by mjwitte

Loading