FixHVACUnitarySystem SetOnOffMassFlowRate NAN problem #4761

mjwitte · 2015-03-01T02:59:46Z

… assignments for MSHPMassFlowRateLow and MSHPMassFlowRateHigh to prevent divide by zero in CalcMultiSpeedDXCoilCooling/Heating.

…ixNANProblems-Issue4557

mjwitte · 2015-03-02T04:48:30Z

@rraustad This is ready for your review. The BadAxe-Base_141019_4spd-MJW defect file runs now, with dx coil airflow per capacity warnings. Not sure if those warnings are expected or if that indicates a problem with the fix. Also, the fix does not really address how a multispeed coil should be controlled when using a unitary system with setpoint control. This just addresses avoiding a divide by zero.

…d revised logic in SetOnOffMassFlowRate subroutine to always set multi-speed coil mass flow rate globals.

…rected missed setting of variables).

mjwitte · 2015-03-03T20:28:58Z

@rraustad Thanks for the second pass on this. I see one unit test is failing. I'll wait for the CI backlog on new features to settle down then merge develop back in here and fix that.

Myoldmopar · 2015-03-03T20:34:48Z

At this point I don't mind if you load up the robots. Thanks for the patience.

rraustad · 2015-03-03T20:35:50Z

All unit tests passed here. I expanded on the unit test you created and pushed that up. If it's failing now you didn't get the latest version of unit tests?

mjwitte · 2015-03-03T21:59:15Z

@rraustad The CI reports showed a failure. I'll reconfirm when I merge in develop (and resolve conflicts ).

rraustad · 2015-03-03T22:19:55Z

HVACTemplate-5ZoneUnitarySystem has audit, rdd, and err (less max iters) diff's. Also convergence diff's in the eio. I typically don't believe those results since it's a moving target.

rraustad · 2015-03-04T13:18:43Z

I went through the Win64-Windows-7-VisualStudio-12 failures and all I see are aud, err, rdd, and eio (convergence) diff's for HVACTemplate-5ZoneUnitarySystem and aud, rdd, and eio (convergence) for UnitarySystem_*. Where do I see csv diff's?

Myoldmopar · 2015-03-04T13:28:49Z

They aren't showing up. However, the CSV and eio numeric diffs have been tightly aligned in testing I've done. If I see eio diffs I usually run the regressions locally.

…ixNANProblems-Issue4557 Conflicts: tst/EnergyPlus/unit/CMakeLists.txt

mjwitte · 2015-03-04T20:42:57Z

@Myoldmopar @rraustad Brought this branch up to date, resolved the conflict, added deallocates to a bunch of unit tests (trying not to conflict withe @jasondegraw changes on the fluid cooler branch), and a couple of other cleanups to the IDD and dx coil warnings.

mjwitte · 2015-03-05T15:02:01Z

@Myoldmopar Concerned that the regression diffs here show max iteration warnings for HVACTemplate-5ZoneUnitarySystem in both develop and this branch. When did those creep in? But I have yet to reproduce those warnings locally. See error diffs here
https://nrel.github.io/EnergyPlusBuildResults/EnergyPlus-8af21d60e78a6f2424d04fb2e9c524839841637c-Win64-Windows-7-VisualStudio-12.html

mjwitte · 2015-03-05T15:53:05Z

@Myoldmopar Oh - CI forces design day runs, which do produce max iteration warnings for HVACTemplate-5ZoeneUnitarySystem - even with 8.2.0. Shouldn't be. Need to work on figuring out why for that (and any others that don't run well with design days only. Post that as an issue or a story or?

mjwitte · 2015-03-05T18:01:13Z

@Myoldmopar Please review changes to unit tests. I restarted the integration coverage tests (seemed stuck for too long) . Because I've made unit test changes both here and in #4624 this should wait until 4624 gets merged in and then I'll merge develop back here to resolve any conflicts in the unit tests.

mjwitte · 2015-03-05T18:33:21Z

@rraustad So, these changes are causing new max iteration warnings with HVACTemplate-5ZoneUnitarySystem when run as-is (using the short runperiods in the example file). Current develop does not have these warnings. So, we need to double-check these changes. UnitarySystem_VSHeatPumpWaterToAirEquationFit has max iteration warnings both before and after this fix - which isn't good either, but perhaps a different issue.

rraustad · 2015-03-05T20:33:55Z

So these fixes broke something. I thought this was rather straight-forward, and would only affect the multispeed coils. I also thought the max iterations were already there so it didn't strike me when I saw them during this fix. The zone temps are no longer maintained.

rraustad · 2015-03-05T20:39:02Z

It's not as bad as first thought once I got the right variables compared.

rraustad · 2015-03-06T04:06:21Z

I haven't been able to find a problem with changes to this PR. I do see where the load is very close to the comp off capacity (~1W) which appeared to confuse the model. A few time steps later, it runs fine with only a couple iterations. I'll need to look more closely at what is happening. I can't say yet if this is a new problem but does look so thus far.

mjwitte · 2015-03-06T14:59:36Z

@rraustad Thanks for looking at this more. Do you have an pending changes on your machine? If not, I will merge develop into here again and resolve the conflict (which is probably in the unit tests again).

rraustad · 2015-03-06T15:01:13Z

No pending changes at this time. I will look at this more over the weekend.

…ixNANProblems-Issue4557 Conflicts: tst/EnergyPlus/unit/PurchasedAirManager.unit.cc

mjwitte · 2015-03-06T23:03:01Z

@rraustad I've merged in develop and pushed this up, so be sure to pull down the latest before you look at this again.

…iteration on PLR (actually CycRatio or SpeedRatio).

rraustad · 2015-03-09T00:38:14Z

** Warning ** SimHVAC: Maximum iterations (20) exceeded for all HVAC loops, at LONDON - - - WMO#=-, 01/14 08:30 - 08:45
** ~~~ ** The solution for one or more of the Air Loop HVAC systems did not appear to converge
** ~~~ ** ...use Output:Diagnostics,DisplayExtraWarnings; to show more details on each max iteration exceeded.

I know what causes it but have not found a solution.
Starting with a previous time step air flow rate is at the heating flow rate. Heating coil is gas with a single speed. Cooling coil is multipseed DX. The outdoor air system determines the mixed air temp is ~19C given the 23C return temp, 6C outdoor air temp, 0.29 kg/s recirc flow and 0.05 kg/s OA flow. The UnitarySystem is simulated and finds a small cooling load and operates at speed 1 (0.06 kg/s) to meet that load. The OA system is simulated again and now finds a 13C mixed air temp given the approximate equal recirc and OA flow. The UnitarySystem, still with a cooling load, finds that operating the constant fan at this low inlet temp pushes the zone temp below the heating set point. It switches to heating mode, adjusts flow rate to the heating flow (0.29 kg/s), and meets the load to heating set point. This high recirc flow rate triggers a change in mixed air temp, back to the original 21C. Next time through the UnitarySystem finds that speed 1 cooling can meet the cooling load. And round and round we go.
The system does control the loads well and hits the cool/heat temps fairly close.

mjwitte · 2015-03-09T13:25:01Z

@rraustad So, what it really needs is to operate at a mixture of flow rates during a single timestep. Could that be done? Would a reasonable compromise be to lock the flow rate based on the initial zone load request? In this case, zone needs cooling (albeit a very small amount) so operate this timestep at the cooling supply flow rate. Other solutions?

rraustad · 2015-03-09T14:18:46Z

Well, the zone needs cooling if the air flow is at the heating flow rate. The zone needs heating if operating at the cooling air flow rate. Constant fan is used during the day. If we change the example file to be multispeed heating and cooling, I think the max iters would go away (nope, they were not eliminated, I tried 2- and 3-spd heating, that surprises me). I wonder if industry has ever ran into this? What would a real system do if OA was provided and heating used full flow only while cooling allowed multiple speeds? A real system would operate in one mode based on zone load as you suggested. Eventually, the zone would float to the deadband while the system fan continued to operate. Either cooling or heating mode would turn on eventually. What's different with E+ is the OA system uses the previous time step flow to determine mixed air temp, this sometimes screws up the model.

Maybe if flow was determined by load, this wouldn't be an issue. I guess that's what a real system would do so maybe it's a reasonable solution. This would mean then that constant fan systems would at times overshoot the (alternate) set point and also lead to smaller system time steps. I'll see if I can test this theory (comment out where I switch modes).

mjwitte · 2015-03-09T14:29:38Z

@rraustad Longer term we should consider making the OA system a child of the unitary system so it gets simulated with whatever airflow rate the unitary system is operating at (no back and forth on that). And there may be some control logic changes that are in order as well. But for now, the changes you pushed up yesterday have resolved the iteration errors for the two single-day runperiods. I'm running an annual simulation now.

rraustad · 2015-03-09T14:33:53Z

Remember I was using London, I wanted the mild loads.

mjwitte · 2015-03-09T15:02:45Z

@rraustad That went right past me (London). And Chicago annual simulation hits max iteration errors as well.

rraustad · 2015-03-09T15:36:47Z

After commenting out the mode switching, this looks like what I would expect. What I didn't expect is to still get max iteration warnings (starts at 1/14 8 AM ~100 of them). Uses 3-spd DX cooling and heating coils. I don't know if this is a different issue or not (i.e., if the model is working as I intend). Since the heating speed never got above 1 (10 in the figure), the OA system should have reasonably determined the mixed air temp from the previous time step recirc flow.
Component Sizing Information, Coil:Heating:DX:MultiSpeed, SYS 2 FURNACE DX HEAT MULTISPD HEATING COIL, Speed 3 Design Size Rated Air Flow Rate [m3/s], 0.24293
Component Sizing Information, Coil:Heating:DX:MultiSpeed, SYS 2 FURNACE DX HEAT MULTISPD HEATING COIL, Speed 2 Design Size Rated Air Flow Rate [m3/s], 0.16195
Component Sizing Information, Coil:Heating:DX:MultiSpeed, SYS 2 FURNACE DX HEAT MULTISPD HEATING COIL, Speed 1 Design Size Rated Air Flow Rate [m3/s], 8.09775E-002

nrel-bot · 2015-03-17T00:10:50Z

@rraustad @mjwitte @lgentile it has been 7 days since this pull request was last updated.

mjwitte · 2015-03-18T02:39:41Z

@rraustad I've lost track of our status on this one. Is it ready to go in?

…ixNANProblems-Issue4557 Conflicts: tst/EnergyPlus/unit/CMakeLists.txt

mjwitte · 2015-03-18T13:47:10Z

src/EnergyPlus/HVACUnitarySystem.cc

@@ -10987,13 +11094,21 @@ namespace HVACUnitarySystem {
 		InletNode = UnitarySystem( UnitarySysNum ).UnitarySystemInletNodeNum;

 		if ( SpeedNum > 1 ) {
-			AverageUnitMassFlow = CompOnMassFlow;
+			if( CoolingLoad && MultiOrVarSpeedCoolCoil( UnitarySysNum ) || HeatingLoad && MultiOrVarSpeedHeatCoil( UnitarySysNum ) ) {


@rraustad The clang tests are throwing four warnings like this:
'&&' within '||' [-Wlogical-op-parentheses]
This line looks like it needs some clarification with parentheses. Also 11107 below. Not sure why four warnings.

mjwitte · 2015-03-18T18:42:36Z

@Myoldmopar @jasondegraw This is the pull request where I mixed in a bunch of new unit test deallocates.

rraustad · 2015-03-19T02:13:40Z

I'll fix that as soon as I can. Should be ( && ) || ( && )

mjwitte · 2015-03-19T02:19:08Z

@rraustad If you can't get to it, I can make that change.

…ixNANProblems-Issue4557

rraustad · 2015-03-19T03:16:56Z

Corrected () || () issue and merged develop again. Defect file runs to completion. I still don't like some of the code I see, but since this model was presumably thoroughly tested, I am hesitant to make significant changes. I do see some areas that still need to be revised but these are not associated with this issue. So I suggest this gets merged. I'm pushing now to see what CI does.

Myoldmopar · 2015-03-19T11:42:10Z

Looks like CI should be OK with it. The semi-expected unit test issues plus a few isolated regressions and reduced max-hvac-iteration warnings. I'll wait until I'm in the office and there should be a few more results by then, then I can merge or whoever else.

mjwitte · 2015-03-20T16:24:19Z

@Myoldmopar @rraustad This branch has merge conflicts now. I can fix unless one of you has already started.

…ixNANProblems-Issue4557 Conflicts: idd/Energy+.idd.in tst/EnergyPlus/unit/PurchasedAirManager.unit.cc

mjwitte · 2015-03-20T18:36:28Z

@Myoldmopar @rraustad Develop merged, conflict resolved, IDD reference re-fixed. One more round of CI, then this can go in.

FixHVACUnitarySystem SetOnOffMassFlowRate NAN problem

mjwitte added 3 commits February 28, 2015 20:52

Fix issue #4557 HVACUnitarySystem SetOnOffMassFlowRate to add missing…

44b4fac

… assignments for MSHPMassFlowRateLow and MSHPMassFlowRateHigh to prevent divide by zero in CalcMultiSpeedDXCoilCooling/Heating.

Merge remote-tracking branch 'remotes/origin/develop' into 89224666-F…

72dab18

…ixNANProblems-Issue4557

Add unit test for HVACUnitarySystem SetOnOffMassFlowRate

ffa5445

mjwitte assigned rraustad Mar 2, 2015

rraustad added 2 commits March 3, 2015 11:19

Expanded warning in DXCoils (due to error in HVACUnitarySystem.cc) an…

7e44b41

…d revised logic in SetOnOffMassFlowRate subroutine to always set multi-speed coil mass flow rate globals.

Updated unit test HVACUnitarySystem for SetOnOffMassFlowRate (and cor…

50912a8

…rected missed setting of variables).

mjwitte added 5 commits March 4, 2015 13:07

Merge remote-tracking branch 'remotes/origin/develop' into 89224666-F…

1181cfa

…ixNANProblems-Issue4557 Conflicts: tst/EnergyPlus/unit/CMakeLists.txt

Add missing deallocates in unit tests.

728f3d4

Fix IDD object-list reference for FaultModel:HumidistatOffset

f6ce9e8

Add more deallocates to unit tests

7dd6bb9

DX coil recurring warnings add space for readability

8af21d6

Merge remote-tracking branch 'remotes/origin/develop' into 89224666-F…

570e876

…ixNANProblems-Issue4557 Conflicts: tst/EnergyPlus/unit/PurchasedAirManager.unit.cc

Fixed a few issues with multispeed coil. Air flow was not set during …

fb7f8a7

…iteration on PLR (actually CycRatio or SpeedRatio).

Merge remote-tracking branch 'remotes/origin/develop' into 89224666-F…

1ede89e

…ixNANProblems-Issue4557 Conflicts: tst/EnergyPlus/unit/CMakeLists.txt

mjwitte reviewed Mar 18, 2015
View reviewed changes

rraustad added 2 commits March 18, 2015 23:04

clean up and correct ( && ) || ( && ) issue

3087f5d

Merge remote-tracking branch 'remotes/origin/develop' into 89224666-F…

d94b9ff

…ixNANProblems-Issue4557

Correct () || () issue and deleted comments

83e67f7

mjwitte added 2 commits March 20, 2015 13:05

Merge remote-tracking branch 'remotes/origin/develop' into 89224666-F…

65b4144

…ixNANProblems-Issue4557 Conflicts: idd/Energy+.idd.in tst/EnergyPlus/unit/PurchasedAirManager.unit.cc

IDD cleanup fix object-list reference to ZoneControlHumidistatNames

fbc3ab2

mjwitte added a commit that referenced this pull request Mar 21, 2015

Merge pull request #4761 from NREL/89224666-FixNANProblems-Issue4557

2970be4

FixHVACUnitarySystem SetOnOffMassFlowRate NAN problem

mjwitte merged commit 2970be4 into develop Mar 21, 2015

Myoldmopar added the Defect Includes code to repair a defect in EnergyPlus label Mar 26, 2015

mjwitte deleted the 89224666-FixNANProblems-Issue4557 branch March 28, 2015 19:36

FixHVACUnitarySystem SetOnOffMassFlowRate NAN problem #4761

FixHVACUnitarySystem SetOnOffMassFlowRate NAN problem #4761

Conversation

mjwitte commented Mar 1, 2015

mjwitte commented Mar 2, 2015

mjwitte commented Mar 3, 2015

Myoldmopar commented Mar 3, 2015 via email

rraustad commented Mar 3, 2015

mjwitte commented Mar 3, 2015

rraustad commented Mar 3, 2015

rraustad commented Mar 4, 2015

Myoldmopar commented Mar 4, 2015 via email

mjwitte commented Mar 4, 2015

mjwitte commented Mar 5, 2015

mjwitte commented Mar 5, 2015

mjwitte commented Mar 5, 2015

mjwitte commented Mar 5, 2015

rraustad commented Mar 5, 2015

rraustad commented Mar 5, 2015

rraustad commented Mar 6, 2015

mjwitte commented Mar 6, 2015

rraustad commented Mar 6, 2015

mjwitte commented Mar 6, 2015

rraustad commented Mar 9, 2015

mjwitte commented Mar 9, 2015

rraustad commented Mar 9, 2015

mjwitte commented Mar 9, 2015

rraustad commented Mar 9, 2015

mjwitte commented Mar 9, 2015

rraustad commented Mar 9, 2015

nrel-bot commented Mar 17, 2015

mjwitte commented Mar 18, 2015

mjwitte Mar 18, 2015

Choose a reason for hiding this comment

mjwitte commented Mar 18, 2015

rraustad commented Mar 19, 2015

mjwitte commented Mar 19, 2015

rraustad commented Mar 19, 2015

Myoldmopar commented Mar 19, 2015

mjwitte commented Mar 20, 2015

mjwitte commented Mar 20, 2015