Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix water coil user-specified sizing and Component Sizing Summary table for fan coils #7988

Merged
merged 29 commits into from
Sep 21, 2020

Conversation

matthew-larson
Copy link
Contributor

@matthew-larson matthew-larson commented May 12, 2020

Pull request overview

  • Fixes Coil:Cooling:Water in Component Sizing Summary has incorrect Design Size and User-Specified Air Flow Rates #7914 and Incorrect Coil:Cooling:Water sizing when hard-sizing ZoneHVAC:FourPipeFanCoil air flow rate #7998
  • This pull request addresses incorrect sizing in the chilled water coils when the water flow and/or air flow are specified for water coils referenced by ZoneHVAC:FourPipeFanCoil objects. For the pre-sizing data calculations, I modified the DataWaterFlowUsedForSizing to Autosize, similar to the other pre-sizing data parameters. In order to pass in the original design parameter value for the RequestSizing function, it now references the DataAirFlowUsedForSizing and DataWaterFlowUsedForSizing, which outputs these as the Design Size values in the Component Sizing Summary table and the hard-sized values as User-Specified values. This also ensures the user-specified values are the values being used for the sizing calculations for coil capacity.
  • The unit test is in the WaterCoils.unit.cc but uses a built-up zone fan coil system that verifies the final design air flow and water flow for the coils are the user-specified values instead of the autosized design values.

Before:
image

After:
image

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 structural output changes, add to output rules file and add OutputChange label

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

@nealkruis nealkruis added Defect Includes code to repair a defect in EnergyPlus NotIDDChange Code does not impact IDD (can be merged after IO freeze) labels May 14, 2020
@matthew-larson matthew-larson marked this pull request as ready for review June 5, 2020 12:23
@matthew-larson matthew-larson added this to the EnergyPlus 9.4.0 milestone Jun 8, 2020
} else {
DataAirFlowUsedForSizing = TempSize; // many autosized inputs use the design (autosized) air volume flow rate, save this value
DataFlowUsedForSizing = TempSize;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This first change looks OK to me.

@@ -2233,9 +2244,19 @@ namespace WaterCoils {
TempSize = AutoSize;
RequestSizing(state, CompType, CompName, CoolingCapacitySizing, SizingString, TempSize, bPRINT, RoutineName);
DataCapacityUsedForSizing = TempSize;
TempSize = WaterCoil(CoilNum).MaxWaterVolFlowRate;
TempSize = AutoSize;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This second change always autosizes the HW coil water flow rate. What if the coil input is hard-sized? And the coil is on the main branch. I am on the fence whether this is OK for zone equipment, but if the fan coil HW flow rate is hard-sized or autosized then that flow rate will be used in the HW coil, even if the HW coil hard-sized the HW flow rate. This is an input over-specification problem and this does correct that. Do all other zone equipment also pass the ZoneEqSizing data, I assume they do, so again this is probably OK.

@@ -2579,7 +2600,7 @@ namespace WaterCoils {

FieldNum = 2; // N2 , \field Maximum Water Flow Rate
SizingString = WaterCoilNumericFields(CoilNum).FieldNames(FieldNum) + " [m3/s]";
TempSize = WaterCoil(CoilNum).MaxWaterVolFlowRate;
TempSize = AutoSize;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above for always autosizing the HW flow rate whether or not the HW coil input is hard-sized.

@rraustad
Copy link
Contributor

It is interesting that only the fan coil example files show diff's. However, there may not be example files where a water coil on the main branch is hard-sized. I think a quick test of 5ZoneAirCooled using hard-sized water flow rates for the main branch coils would show any problems. Also, what if a test file is completely hard-sized? And there are no sizing objects? That file would fail with a warning to add sizing objects. And users would say "huh".

@rraustad
Copy link
Contributor

The fan coil was already passing the information. For some reason the water coil was not picking that information up in RequestSizing.

ZoneEqSizing(CurZoneEqNum).MaxHWVolFlow = FanCoil(FanCoilNum).MaxHotWaterVolFlow;
ZoneEqSizing(CurZoneEqNum).MaxCWVolFlow = FanCoil(FanCoilNum).MaxColdWaterVolFlow;

I think if you step through the sizing of a fan coil's water coil, by stepping into RequestSizing, you will see where the problem lies.

@matthew-larson
Copy link
Contributor Author

Thanks @rraustad, I'll go back and step through the sizing portion and see if I can find the disconnect.

@nrel-bot-2
Copy link

@matthew-larson @lgentile it has been 28 days since this pull request was last updated.

@matthew-larson
Copy link
Contributor Author

@rraustad I ended up removing the changes related to the water flow rates for this pull request. I felt that I was getting beyond the scope of these issues which are addressing the design air flow rates and need to spend more time investigating RequestSizing for the water flow rates as you recommended previously. Thanks again for your assistance with this. Let me know if you have any additional comments regarding this pull request.

@rraustad
Copy link
Contributor

Well, the logic here looks better. Good thing this is in water coils, because I have removed RequestSizing in the Autosizing Library branch. Can you paste in the eio diff's from a defect file so I have a reference for what you think is the correct result? And tell me what defect file or example file you used.

@matthew-larson
Copy link
Contributor Author

I used the _FanCoilHybridVentAFN.idf testfile to show the effect. The ZoneHVAC:FourPipeFanCoil object is autosized to a Design Size Maximum Supply Air Flow of 0.21702 m3/s. The Coil:Cooling:Water associated with the fan coil specifies a Design Size Design Air Flow Rate of 0.21702 m3/s but a User-Specified Design Air Flow Rate of 0.12390 m3/s, which is used for the sizing of the coil, leaving the coil undersized compared to the fan coil specifications. After the fix, the Coil:Cooling:Water gets a User-Specified Design Air Flow Rate of 0.21702 m3/s to match the size determined by the ZoneHVAC:FourPipeFanCoil object. You can see the change in Nominal Total Capacity of the cooling coil and other design parameters below (before on left, after on right):

image

I also tested when the Maximum Supply Air Flow Rate for the ZoneHVAC:FourPipeFanCoil object is hard-sized to 0.2 m3/s instead of autosized. Originally, hard-sizing this has no effect on the Coil:Cooling:Water Nominal Total Capacity as it still uses the User-Specified Design Air Flow Rate of 0.12390 m3/s. After the fix, the Coil:Cooling:Water gets a User-Specified Design Air Flow Rate of 0.2 m3/s to match the size determined by the ZoneHVAC:FourPipeFanCoil object and increases the Nominal Total Capacity. Same before and after comparison with the hard-sized flow rate shown below:

image

Let me know if you need any other info/results or have any additional or followup questions.

@rraustad
Copy link
Contributor

Well, this goes to what the user intended. What if the user wanted to under (or over) size the cooling coil? With these changes there is no way to size a coil different from the fan coil size. Using the autosize method, all inputs should be autosized. If a user specifies a hard-size then E+ uses that information, regardless of result.

@matthew-larson
Copy link
Contributor Author

I see what you mean, I need to address the ability to hard-size the water coil to be different than the fan coil. I'll spend more time on this, thanks.

@matthew-larson
Copy link
Contributor Author

I made a number of updates to get the eio results to reflect what I believe we want to see based on different scenarios of autosizing and hardsizing the fan coil and cooling coil inputs. Below is a table showing the eio results for the ZoneHVAC:FourPipeFanCoil Maximum Supply Air Flow Rate and Coil:Cooling:Water Design Air Flow Rate using the same test file (_FanCoilHybridVentAFN.idf).

image

The warnings and failed regressions tests are all due to the Coil:Cooling:Water now reporting 'Design Size Design Air Flow Rate' instead of 'User-Specified Design Air Flow Rate' when the coil is connected to a fan coil which are both autosized for air flow rate, which I think is appropriate. There is an error added to the results for '_FanCoilHybridVentAFN.idf' due to the change in the Coil:Cooling:Water air flow rate but wanted to get this all reviewed before addressing this if needed.

WaterCoil(WhichCoil).DesAirVolFlowRate = CoilDesFlow;
} else {
WaterCoil(WhichCoil).DesAirVolFlowRate = CoilDesFlow;
//WaterCoil(WhichCoil).DesAirVolFlowRate = CoilDesFlow;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the design air vol flow rate will just stay at zero for this case?

@Myoldmopar
Copy link
Member

I've got no problem with this. I think all the review comments have been addressed. CI results look OK given the expected string change. I built locally with develop and ran tests and everything is still fine. I'm inclined to merge but will hold for a while to see if anyone has more thoughts.

@Myoldmopar
Copy link
Member

Merging. Thanks @matthew-larson for the fix and @rraustad for looking at the changes multiple times.

@Myoldmopar Myoldmopar merged commit 1f94ff5 into NREL:develop Sep 21, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Defect Includes code to repair a defect in EnergyPlus NotIDDChange Code does not impact IDD (can be merged after IO freeze)
Projects
None yet
9 participants