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

Add new output variables for ZoneHVAC:IdealLoadsAirSystem supply and outdoor air flow rates #5293

Merged

Conversation

Projects
None yet
6 participants
@JasonGlazer
Copy link
Contributor

commented Oct 30, 2015

to show the outdoor and system air flow rates as both mass and volumetric units. Fixing #4325.

Add new output variables for ZoneHVACIdealLoadsAirSystem to show the …
…outdoor and system air flow rates as both mass and volumetric units
@JasonGlazer

This comment has been minimized.

Copy link
Contributor Author

commented Oct 30, 2015

Looks like this commit is mostly about documentation changes but those are just because my editor got rid of a bunch of trailing spaces. The code change are in the .hh and .cc files.

@JasonGlazer

This comment has been minimized.

Copy link
Contributor Author

commented Oct 30, 2015

@rraustad or @mjwitte can one of you look through these changes and make sure I put the code in the right place in the .cc file. I have not messed with the purchair related code before and want to make sure I am not missing something.

@@ -700,6 +705,12 @@ namespace PurchasedAirManager {

SetupOutputVariable( "Zone Ideal Loads Hybrid Ventilation Available Status []", PurchAir( PurchAirNum ).AvailStatus, "System", "Average", PurchAir( PurchAirNum ).Name );

//air flows

This comment has been minimized.

Copy link
@mjwitte

mjwitte Oct 30, 2015

Contributor

@JasonGlazer Looks like the indentation is wrong around here. Maybe spaces instead of tabs?

This comment has been minimized.

Copy link
@JasonGlazer

JasonGlazer Oct 30, 2015

Author Contributor

@mjwitte it looks ok to me. The bracket above the line you commented on is from a much earlier set of lines.
The file uses tabs but all the files use tabs on my machine (is that correct?).
Do you really want me to use spaces instead?

This comment has been minimized.

Copy link
@mjwitte

mjwitte Oct 30, 2015

Contributor

No - tabs are correct. Maybe github lied to me when I expanded this.

This comment has been minimized.

Copy link
@mjwitte

mjwitte Oct 30, 2015

Contributor

When you look at this on github, line 706 is left of 704. Doesn't seem correct.

This comment has been minimized.

Copy link
@JasonGlazer

JasonGlazer Oct 30, 2015

Author Contributor

Weird, it doesn't render that way for me on Github

This comment has been minimized.

Copy link
@mjwitte

mjwitte Oct 30, 2015

Contributor

And it doesn't render that way for me when I look in Chrome instead of IE. Fascinating. Never mind.

@mjwitte

This comment has been minimized.

Copy link
Contributor

commented Oct 30, 2015

@JasonGlazer Otherwise, the changes look OK. You should be able to verify by comparing to the equivalent System Node . . . outputs.

@nrel-bot-3

This comment has been minimized.

Copy link

commented on b0dceda Oct 30, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-MacOS-10.9-clang: OK (1909 of 1955 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Oct 30, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - i386-Windows-7-VisualStudio-12: OK (1913 of 1961 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Oct 30, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-gcc-4.8: OK (1913 of 1961 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Oct 30, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-cppcheck-1.61: OK (0 of 0 tests passed)

Build Badge

This comment has been minimized.

Copy link

replied Oct 30, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - Win64-Windows-7-VisualStudio-12: OK (1913 of 1961 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Oct 30, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-UnitTestsCoverage-Debug: OK (887 of 887 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Oct 30, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-IntegrationCoverage-Debug: OK (1425 of 1425 tests passed)

Build Badge Test Badge Coverage Badge

@nrel-bot-3

This comment has been minimized.

Copy link

commented on 8f070eb Oct 30, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-MacOS-10.9-clang: OK (1910 of 1956 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Oct 30, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-gcc-4.8: OK (1914 of 1962 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Oct 30, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-cppcheck-1.61: OK (0 of 0 tests passed)

Build Badge

This comment has been minimized.

Copy link

replied Oct 30, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-UnitTestsCoverage-Debug: OK (888 of 888 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Oct 30, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-IntegrationCoverage-Debug: OK (1426 of 1426 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Oct 30, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - i386-Windows-7-VisualStudio-12: OK (1914 of 1962 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Oct 30, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - Win64-Windows-7-VisualStudio-12: OK (1914 of 1962 tests passed)

Build Badge Test Badge

@JasonGlazer

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2015

Reviewed the many diffs and they all seem reasonable. The audit and rdd diffs are directly due to the addition of four new output variables. The differences in the tabular HTML file is just the build number on the input verification report. It should probably not be shown as a difference - @Myoldmopar would this be possible?

@JasonGlazer

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2015

I also did some checks of the new supply air mass flow rate and volume flow rate compared to system node values and they match.

@JasonGlazer

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2015

@Myoldmopar and @mjwitte I believe this is ready to be merged.

Merge remote-tracking branch 'remotes/origin/develop' into Additional…
…-report-variables-ZoneHVACIdealLoadsAirSystem

Conflicts:
	tst/EnergyPlus/unit/Fixtures/EnergyPlusFixture.cc

@mjwitte mjwitte changed the title Add new output variables for ZoneHVACIdealLoadsAirSystem Add new output variables for ZoneHVAC:IdealLoadsAirSystem supply and outdoor air flow rates Nov 9, 2015

@mjwitte mjwitte added the NewFeature label Nov 9, 2015

@nrel-bot-2

This comment has been minimized.

Copy link

commented on cdebda1 Nov 9, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (mjwitte) - x86_64-Linux-Ubuntu-14.04-cppcheck-1.61: OK (0 of 0 tests passed)

Build Badge

This comment has been minimized.

Copy link

replied Nov 9, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (mjwitte) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-UnitTestsCoverage-Debug: OK (889 of 889 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Nov 9, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (mjwitte) - x86_64-Linux-Ubuntu-14.04-gcc-4.8: OK (1915 of 1963 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Nov 9, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (mjwitte) - Win64-Windows-7-VisualStudio-12: OK (1915 of 1963 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Nov 9, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (mjwitte) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-IntegrationCoverage-Debug: OK (1427 of 1427 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Nov 9, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (mjwitte) - i386-Windows-7-VisualStudio-12: OK (1915 of 1963 tests passed)

Build Badge Test Badge

@@ -7152,6 +7152,15 @@ An example of this object in an IDF is:

* HVAC,Average,Zone Ideal Loads Hybrid Ventilation Available Status []

* HVAC,Average,Zone Ideal Loads Outdoor Air Mass Flow Rate [kg/s]

This comment has been minimized.

Copy link
@mjwitte

mjwitte Nov 9, 2015

Contributor

@JasonGlazer Please add the variable descriptions further down. You can adapt the text from the equivalent System Node flow variable descriptions. Be sure to pull this branch before editing - I've merged in develop and resolved a conflict today.

EXPECT_EQ( PurchAir( 1 ).DehumidCtrlType, ConstantSupplyHumidityRatio );
EXPECT_EQ( PurchAir( 1 ).HumidCtrlType, ConstantSupplyHumidityRatio );

}

This comment has been minimized.

Copy link
@mjwitte

mjwitte Nov 9, 2015

Contributor

Also, this needs a newline after the last line.

@mjwitte

This comment has been minimized.

The two volume flows should include the explanation of standard density or refer to System Node Standard Density Volume Flow Rate for more details.

@mjwitte

This comment has been minimized.

Copy link
Contributor

commented Nov 9, 2015

Reviewed and tested, ready to merge once CI tests are complete. @Myoldmopar Any comments?

@Myoldmopar

This comment has been minimized.

Copy link
Member

commented Nov 9, 2015

No significant comment. The code looks perfect. And it looks like your editor cleaned up lots of whitespace changes in the docs. (Easy enough to filter using Github by appending ?w=1 to the end of the URL when viewing the files changed tab.). The only comment I'd say is in the future, instead of adding more variables to the unused member constructor, just take the member constructor out.

Merge away when you are happy with CI, or I can merge if you are looking for a volunteer.

@nrel-bot-2

This comment has been minimized.

Copy link

commented on 5a78fc1 Nov 9, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-gcc-4.8: OK (1915 of 1963 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Nov 9, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-cppcheck-1.61: OK (0 of 0 tests passed)

Build Badge

This comment has been minimized.

Copy link

replied Nov 9, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - i386-Windows-7-VisualStudio-12: OK (1915 of 1963 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Nov 9, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-UnitTestsCoverage-Debug: OK (889 of 889 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Nov 9, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-IntegrationCoverage-Debug: OK (1427 of 1427 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Nov 9, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - Win64-Windows-7-VisualStudio-12: OK (1915 of 1963 tests passed)

Build Badge Test Badge

@nrel-bot-2

This comment has been minimized.

Copy link

commented on a225fab Nov 9, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-cppcheck-1.61: OK (0 of 0 tests passed)

Build Badge

This comment has been minimized.

Copy link

replied Nov 9, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-gcc-4.8: OK (1915 of 1963 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Nov 9, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-UnitTestsCoverage-Debug: OK (889 of 889 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Nov 9, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - i386-Windows-7-VisualStudio-12: OK (1914 of 1963 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Nov 9, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-IntegrationCoverage-Debug: OK (1427 of 1427 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Nov 10, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - Win64-Windows-7-VisualStudio-12: OK (1915 of 1963 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Nov 10, 2015

Additional-report-variables-ZoneHVACIdealLoadsAirSystem (JasonGlazer) - x86_64-MacOS-10.9-clang: OK (1911 of 1957 tests passed)

Build Badge Test Badge

mjwitte added a commit that referenced this pull request Nov 10, 2015

Merge pull request #5293 from NREL/Additional-report-variables-ZoneHV…
…ACIdealLoadsAirSystem

Add new output variables for ZoneHVAC:IdealLoadsAirSystem supply and outdoor air flow rates

@mjwitte mjwitte merged commit ec548f8 into develop Nov 10, 2015

6 of 7 checks passed

x86_64-MacOS-10.9-clang Build Pending
Details
Win64-Windows-7-VisualStudio-12 OK (1915 of 1963 tests passed)
Details
i386-Windows-7-VisualStudio-12 OK (1914 of 1963 tests passed)
Details
x86_64-Linux-Ubuntu-14.04-cppcheck-1.61 OK (0 of 0 tests passed)
Details
x86_64-Linux-Ubuntu-14.04-gcc-4.8 OK (1915 of 1963 tests passed)
Details
x86_64-Linux-Ubuntu-14.04-gcc-4.8-IntegrationCoverage-Debug OK (1427 of 1427 tests passed)
Details
x86_64-Linux-Ubuntu-14.04-gcc-4.8-UnitTestsCoverage-Debug OK (889 of 889 tests passed)
Details

@mjwitte mjwitte deleted the Additional-report-variables-ZoneHVACIdealLoadsAirSystem branch Nov 10, 2015

@JasonGlazer JasonGlazer restored the Additional-report-variables-ZoneHVACIdealLoadsAirSystem branch Jan 29, 2016

@JasonGlazer JasonGlazer deleted the Additional-report-variables-ZoneHVACIdealLoadsAirSystem branch Jan 29, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.