Implement new gas steam humidifier simulation model #4740

Nigusse · 2015-02-20T14:45:29Z

This branch is ready for review.

Conflicts: src/EnergyPlus/Humidifiers.cc tst/EnergyPlus/unit/CMakeLists.txt

nrel-bot · 2015-03-02T00:52:29Z

@Myoldmopar @Nigusse @lgentile it has been 7 days since this pull request was last updated.

Myoldmopar · 2015-03-02T15:11:44Z

src/EnergyPlus/Psychrometrics.hh

@@ -719,8 +719,8 @@ namespace Psychrometrics {
 		// FUNCTION LOCAL VARIABLE DECLARATIONS:

 		Int64 const Tdb_tag( bit::bit_shift( TRANSFER( T, Grid_Shift ), -Grid_Shift ) ); // Note that 2nd arg to TRANSFER is not used: Only type matters
-//		Int64 const hash( bit::bit_and( Tdb_tag, psatcache_mask ) ); //Tuned Replaced by below


@Nigusse Ummm, why was this changed?

@Myoldmopar. I do not think this change was related to this work. I do remember going through this section of the code when I was creating unit test for gas humidifier. I had to include the "Psychomterics" initialization to circumvent the crash of the unit test for gas humidifier.

OK, well it was changed during this commit. I'll revert the change and pull out bit.hh from the unit test file below and see what happens. Other than these issues, this branch does look really good though. Thanks for the efforts in getting unit tests included and also objectifying the code.

Myoldmopar · 2015-03-02T15:28:22Z

tst/EnergyPlus/unit/Humidifiers.unit.cc

+#include <ObjexxFCL/FArray1D.hh>
+#include <ObjexxFCL/IOFlags.hh>
+#include <ObjexxFCL/gio.hh>
+#include <ObjexxFCL/bit.hh>


@Nigusse I noticed bit.hh is included here as well, but I don't see where it is actually used in here. Is it possible this (and the psychrometric change) is just an artifact from playing around? Or is it intentional? Thanks.

@Myoldmopar. I was trying to resolve the unit test crash. The solution was was to add Psychometric initialization in the unit test. If I changed any in Psychometric it must be accidental. Also the "bit.hh" include was done for the same reason. I guess it may not be required, I did not check after the unit test crash issue was resolved.

Myoldmopar · 2015-03-02T16:06:57Z

@Nigusse This looks good, except for one thing. Humidifiers.cc was almost completely conflicted. The Objexx commit went through and cleaned up the code in Humidifiers at the same time you went through and made it object oriented and the diff engine can't understand what's up. I pulled in develop and just chose your version of Humidifiers.cc, basically disregarding the version in develop. This isn't the right final solution, but it lets me build anyway to test.

@DeadParrot I'm trying to look through the commit in that Objexx branch from January. It's such a big commit, and Git is seeming to show lines changed when I don't see changes.

If I push this branch up, with exactly develop's Humidifiers.cc, could you (@DeadParrot) comment on what changes you had made during cleanup? I know some changes are like the stray " " characters in the strings. Other changes may not be as relevant since @Nigusse made so many changes to the source.

Myoldmopar · 2015-03-02T16:25:25Z

@Nigusse I see now it was also changes from @wfbuhl's sizing work. This will likely need to be a team effort to get it merged together. I'll try to sum up the issues in an email and send it out.

Myoldmopar · 2015-03-02T16:53:29Z

@Nigusse @wfbuhl @DeadParrot @mjwitte

OK, I cleaned up this branch, and then I did the merge from develop, first choosing @Nigusse's Humidifiers.cc file directly, then bringing in changes from @wfbuhl's coil/space peak sizing work. EnergyPlus built fine. Once I cleaned up the unit tests, they also built fine. But they fail when trying to execute them:

Mismatch was found in the Rated Gas Use Rate and Thermal Efficiency for gas fired steam humidifier

Maybe something else needs to be initialized on the class before calling SizeHumdifier? Let me know if you have any thoughts. I'd like to get this merged in soon.

PS: I also commented out the SQLite initialization in the unit tests; I don't think it's needed anymore, but I could be wrong. If it is we can un-comment it.

mbadams5 · 2015-03-02T17:02:48Z

@Myoldmopar The SQLite initialization isn't required anymore (with current develop), however you might still need to keep the regular IO initialization here...

EnergyPlus/tst/EnergyPlus/unit/Humidifiers.unit.cc

Line 113 in beef742

    
           //{ IOFlags flags; flags.ACTION( "write" ); flags.STATUS( "UNKNOWN" ); gio::open( OutputFileInits, "eplusout.eio", flags ); write_stat = flags.ios(); }

It depends if any of these functions write to IO behind the scenes, but I am less certain about that.

Also don't forget to remove the SQLiteProcedures.hh include...

EnergyPlus/tst/EnergyPlus/unit/Humidifiers.unit.cc

Line 20 in beef742

#include <EnergyPlus/SQLiteProcedures.hh>

mjwitte · 2015-03-02T17:18:32Z

@mbadams5 I know I had added an SQLite initialization to one (or more) of my unit tests. We should find those and remove?

Myoldmopar · 2015-03-02T17:20:42Z

I believe so, but it's definitely not a priority issue right now. Just another thing on the list...

Myoldmopar · 2015-03-02T17:21:01Z

Oh, but I'll try to remove them as I see them, especially on new additions.

mbadams5 · 2015-03-02T17:21:02Z

@mjwitte I cleaned up your unit tests in my pull request, but I may have missed some.

Nigusse · 2015-03-02T17:25:07Z

@Myoldmopar. I think for now just comment out "ErrorsFound = true;" on line # 778. And initialize the "ThermalEffRated" to 1.0. May be add a statement like this:

"Thermal Efficiency is set to 1.0 and the simulation continues."

Nigusse · 2015-03-02T17:28:14Z

@Myoldmopar @mjwitte. I believer, the crash should be due to the "ErrorsFound = true;" statement, which I added in the last minute. The SQLITE was working fine.

Myoldmopar · 2015-03-02T17:41:10Z

@Nigusse I did that, and when I did, the Humidifiers unit tests completed successfully.

However, now the PurchasedAirManager unit tests are failing. I traced it in through SizePurchaedAir to the RequestSizing routine where on line 399 the UnitarySysEqSizing array is being accessed when presumably it isn't allocated yet. I don't see how this would have just shown up in your branch when it seems to be working fine in develop.

Any thoughts? It's probably just a matter of allocating that global array in the unit test before calling SizePurchasedAir, but frankly I don't want to chase bugs right now if they aren't relevant to the code being merged.

Nigusse · 2015-03-02T17:54:19Z

@Myoldmopar. I have no clue how the humidifier change affects SizePurchaedAir. The humidifier sizing is self contained, it does not use RequestSizing.

Myoldmopar · 2015-03-02T17:58:31Z

@Nigusse Yeah, that's what I figured. I don't think I'll attempt a fix with it right now. A side investigation is underway to figure out what's up with the PurchasedAirManager tests. OK, I'll check everything on this branch once more, push up a final commit with the unit test issue fixed, and see how things go.

DeadParrot · 2015-03-02T18:45:52Z

@Myoldmopar Yes, I can review the Humidifiers diffs when you are ready. If the goal is to bring still-relevant changes into the revised code I can take a shot at that.

Myoldmopar · 2015-03-02T21:47:28Z

@DeadParrot I have a commit that I'm not pushing up yet, but it is only white-space changes. For some reason, this branch got a bunch of extra spaces following open-parentheses. They aren't affecting anything, and I'm not going to clog up CI right now by pushing that up when other more important commits will come. So, in summary, if you can look through the code, now is a fine time. I'll push my commit tomorrow. Or tonight.

DeadParrot · 2015-03-02T22:14:51Z

@Myoldmopar OK, I will look at it tonight.

Myoldmopar · 2015-03-03T12:46:37Z

@Nigusse don't worry about the failed tests here, they are all just audit and rdd diff's. If @DeadParrot is satisfied after his perusal I think this will be ready to go in.

Humidifier changes. Red CI results are all audit and rdd diffs. Good to go.

Nigusse added 8 commits February 13, 2015 15:38

Test.

7dff619

save changes in the local machine.

6477f0b

Merge branch 'develop' into GasHumidifier

5cb7c74

Conflicts: src/EnergyPlus/Humidifiers.cc tst/EnergyPlus/unit/CMakeLists.txt

Merge conflict cleaned.

1bb246e

Added Unit Test File.

2da9454

Added Example IDF and Unit Test File.

9299065

Minor cleanup.

e93de47

Minor cleanup.

13be9ae

Nigusse assigned Myoldmopar Feb 20, 2015

Myoldmopar added the NewFeature Includes code to add a new feature to EnergyPlus label Feb 22, 2015

Myoldmopar reviewed Mar 2, 2015
View reviewed changes

Minor formatting mods

beefc6e

Myoldmopar reviewed Mar 2, 2015
View reviewed changes

Myoldmopar added 2 commits March 2, 2015 08:49

Clean up odd psychrometric changes

beefbf2

Merged develop, chose Humidifers from @Nigusse

beefc95

Update Humidifiers and cleanup humidifier unit tests

beef742

Whitespace only changes

beefd68

Myoldmopar added a commit that referenced this pull request Mar 3, 2015

Merge pull request #4740 from NREL/GasHumidifier

9999b3b

Humidifier changes. Red CI results are all audit and rdd diffs. Good to go.

Myoldmopar merged commit 9999b3b into develop Mar 3, 2015

Myoldmopar deleted the GasHumidifier branch March 3, 2015 14:40

Myoldmopar changed the title ~~Gas humidifier~~ Implement new gas steam humidifier simulation model Mar 7, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement new gas steam humidifier simulation model #4740

Implement new gas steam humidifier simulation model #4740

Nigusse commented Feb 20, 2015

nrel-bot commented Mar 2, 2015

Myoldmopar Mar 2, 2015

Nigusse Mar 2, 2015

Myoldmopar Mar 2, 2015

Myoldmopar Mar 2, 2015

Nigusse Mar 2, 2015

Myoldmopar commented Mar 2, 2015

Myoldmopar commented Mar 2, 2015

Myoldmopar commented Mar 2, 2015

mbadams5 commented Mar 2, 2015

mjwitte commented Mar 2, 2015

Myoldmopar commented Mar 2, 2015

Myoldmopar commented Mar 2, 2015

mbadams5 commented Mar 2, 2015

Nigusse commented Mar 2, 2015

Nigusse commented Mar 2, 2015

Myoldmopar commented Mar 2, 2015

Nigusse commented Mar 2, 2015

Myoldmopar commented Mar 2, 2015

DeadParrot commented Mar 2, 2015

Myoldmopar commented Mar 2, 2015

DeadParrot commented Mar 2, 2015

Myoldmopar commented Mar 3, 2015

Implement new gas steam humidifier simulation model #4740

Implement new gas steam humidifier simulation model #4740

Conversation

Nigusse commented Feb 20, 2015

nrel-bot commented Mar 2, 2015

Myoldmopar Mar 2, 2015

Choose a reason for hiding this comment

Nigusse Mar 2, 2015

Choose a reason for hiding this comment

Myoldmopar Mar 2, 2015

Choose a reason for hiding this comment

Myoldmopar Mar 2, 2015

Choose a reason for hiding this comment

Nigusse Mar 2, 2015

Choose a reason for hiding this comment

Myoldmopar commented Mar 2, 2015

Myoldmopar commented Mar 2, 2015

Myoldmopar commented Mar 2, 2015

mbadams5 commented Mar 2, 2015

mjwitte commented Mar 2, 2015

Myoldmopar commented Mar 2, 2015

Myoldmopar commented Mar 2, 2015

mbadams5 commented Mar 2, 2015

Nigusse commented Mar 2, 2015

Nigusse commented Mar 2, 2015

Myoldmopar commented Mar 2, 2015

Nigusse commented Mar 2, 2015

Myoldmopar commented Mar 2, 2015

DeadParrot commented Mar 2, 2015

Myoldmopar commented Mar 2, 2015

DeadParrot commented Mar 2, 2015

Myoldmopar commented Mar 3, 2015