-
Notifications
You must be signed in to change notification settings - Fork 392
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
Fix #10663 - Fan runtime fraction < 1 when flow rate scaling using DesignSpecification:ZoneHVAC:Sizing #10673
base: develop
Are you sure you want to change the base?
Conversation
@@ -1011,7 +1011,7 @@ namespace WindowAC { | |||
zoneEqSizing.SizingMethod(SizingMethod) = CapSizingMethod; | |||
if (CapSizingMethod == CoolingDesignCapacity || CapSizingMethod == CapacityPerFloorArea || | |||
CapSizingMethod == FractionOfAutosizedCoolingCapacity) { | |||
if (CapSizingMethod == HeatingDesignCapacity) { | |||
if (CapSizingMethod == CoolingDesignCapacity) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was an existing logic error that I noticed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Without example files or unit tests these are hard to find.
// Make the Fan be sized by this | ||
zoneEqSizing.CoolingAirFlow = true; | ||
zoneEqSizing.CoolingAirVolFlow = windowAC.MaxAirVolFlow; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will size the Fan to match.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EnergyPlus/src/EnergyPlus/Autosizing/SystemAirFlowSizing.cc
Lines 237 to 238 in f274ccb
} else if (this->zoneEqSizing(this->curZoneEqNum).CoolingAirFlow && !this->zoneEqSizing(this->curZoneEqNum).HeatingAirFlow) { | |
this->autoSizedValue = this->dataFracOfAutosizedCoolingAirflow * this->zoneEqSizing(this->curZoneEqNum).CoolingAirVolFlow; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In SystemAirFlowSizing, if CoolingAirVolFlow = windowAC.MaxAirVolFlow (which should be already adjusted?) and dataFracOfAutosizedCoolingAirflow = the target fraction, then how does this get the right answer? I would think this would either be 1 * adjusted air flow
or fraction * design air flow
. This looks like it's fraction * adjusted air flow
. Maybe windowAC.MaxAirVolFlow is the design air flow which gets multiplied by the fraction. In any event you are getting the right answer now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies, I linked to the wrong bit in SystemAirFlowSizing. The correct bit is:
EnergyPlus/src/EnergyPlus/Autosizing/SystemAirFlowSizing.cc
Lines 619 to 620 in 54f875d
if (this->zoneEqSizing(this->curZoneEqNum).CoolingAirFlow && !this->zoneEqSizing(this->curZoneEqNum).HeatingAirFlow) { | |
this->autoSizedValue = this->zoneEqSizing(this->curZoneEqNum).CoolingAirVolFlow; |
Process 2757796 stopped
* thread #1, name = 'energyplus', stop reason = step over
frame #0: 0x00007ffff50d4370 libenergyplusapi.so.24.2.0`EnergyPlus::SystemAirFlowSizer::size(this=0x00007fffffff4d88, state=0x00007fffffffa980, _originalValue=-99999, errorsFound=0x00007fffffff529f) at SystemAirFlowSizing.cc:620:54
617 }
618 } else {
619 if (this->zoneEqSizing(this->curZoneEqNum).CoolingAirFlow && !this->zoneEqSizing(this->curZoneEqNum).HeatingAirFlow) {
-> 620 this->autoSizedValue = this->zoneEqSizing(this->curZoneEqNum).CoolingAirVolFlow;
621 if (this->finalZoneSizing(this->curZoneEqNum).CoolDDNum > 0 &&
622 this->finalZoneSizing(this->curZoneEqNum).CoolDDNum <= state.dataEnvrn->TotDesDays) {
623 DDNameFanPeak = state.dataWeather->DesDayInput(this->finalZoneSizing(this->curZoneEqNum).CoolDDNum).Title;
(lldb) p this->zoneEqSizing(this->curZoneEqNum).CoolingAirVolFlow
(Real64) 0.28246116346780509
" WindowACSizing; !- Design Specification ZoneHVAC Sizing Object Name", | ||
|
||
// NOTE: Only difference here is that I'm settting a DesignSpecification:ZoneHVAC:Sizing | ||
" DesignSpecification:ZoneHVAC:Sizing,", | ||
" WindowACSizing, !- Name", | ||
" FractionOfAutosizedCoolingAirflow, !- Cooling Supply Air Flow Rate Method", | ||
" , !- Cooling Supply Air Flow Rate {m3/s}", | ||
" , !- Cooling Supply Air Flow Rate Per Floor Area {m3/s-m2}", | ||
" 0.5, !- Cooling Fraction of Autosized Cooling Supply Air Flow Rate", | ||
" , !- Cooling Supply Air Flow Rate Per Unit Cooling Capacity {m3/s-W}", | ||
" FractionOfAutosizedCoolingAirflow, !- No Load Supply Air Flow Rate Method", | ||
" , !- No Load Supply Air Flow Rate {m3/s}", | ||
" , !- No Load Supply Air Flow Rate Per Floor Area {m3/s-m2}", | ||
" 0.5, !- No Load Fraction of Cooling Supply Air Flow Rate", | ||
" , !- No Load Fraction of Heating Supply Air Flow Rate", | ||
" None, !- Heating Supply Air Flow Rate Method", | ||
" , !- Heating Supply Air Flow Rate {m3/s}", | ||
" , !- Heating Supply Air Flow Rate Per Floor Area {m3/s-m2}", | ||
" , !- Heating Fraction of Heating Supply Air Flow Rate", | ||
" , !- Heating Supply Air Flow Rate Per Unit Heating Capacity {m3/s-W}", | ||
" FractionOfAutosizedCoolingCapacity, !- Cooling Design Capacity Method", | ||
" , !- Cooling Design Capacity {W}", | ||
" , !- Cooling Design Capacity Per Floor Area {W/m2}", | ||
" 0.5; !- Fraction of Autosized Cooling Design Capacity", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New unit test that uses a DesignSpecification:ZoneHVAC:Sizing
double constexpr expected_full_airflow = 0.041484382187390034; | ||
double constexpr scaler_cooling_saf = 0.5; | ||
double constexpr scaler_cooling_cap = 0.5; | ||
double constexpr expected_full_cap = 622.50474573886743; | ||
|
||
auto const &windowAC = state->dataWindowAC->WindAC(compIndex); | ||
|
||
ASSERT_GT(windowAC.FanIndex, 0); | ||
auto const &fan = state->dataFans->fans(windowAC.FanIndex); | ||
|
||
auto const &finalZoneSizing = state->dataSize->FinalZoneSizing(1); | ||
|
||
EXPECT_NEAR(expected_full_airflow, finalZoneSizing.DesCoolVolFlow, 0.0001); | ||
|
||
ASSERT_GT(windowAC.HVACSizingIndex, 0); | ||
auto const &zoneHVACSizing = state->dataSize->ZoneHVACSizing(windowAC.HVACSizingIndex); | ||
|
||
EXPECT_EQ(DataSizing::FractionOfAutosizedCoolingAirflow, zoneHVACSizing.CoolingSAFMethod); | ||
EXPECT_EQ(scaler_cooling_saf, zoneHVACSizing.MaxCoolAirVolFlow); | ||
EXPECT_EQ(DataSizing::FractionOfAutosizedCoolingCapacity, zoneHVACSizing.CoolingCapMethod); | ||
EXPECT_EQ(scaler_cooling_cap, zoneHVACSizing.ScaledCoolingCapacity); | ||
|
||
EXPECT_EQ(windowAC.DXCoilType_Num, HVAC::Coil_CoolingAirToAirVariableSpeed); | ||
ASSERT_GT(windowAC.DXCoilIndex, 0); | ||
auto const &varSpeedCoil = state->dataVariableSpeedCoils->VarSpeedCoil(windowAC.DXCoilIndex); | ||
|
||
// check Sizing | ||
EXPECT_NEAR(expected_full_airflow * scaler_cooling_saf, windowAC.MaxAirVolFlow, 0.0001); | ||
EXPECT_NEAR(expected_full_airflow * scaler_cooling_saf, fan->maxAirFlowRate, 0.0001); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check that the Fan too is size at 0.5 of full flow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my previous comment about SystemAirFlowSizing, windowAC.MaxAirVolFlow is already adjusted. Now I'm confused.
EXPECT_NEAR(expected_full_airflow * scaler_cooling_saf, windowAC.MaxAirVolFlow, 0.0001); | ||
EXPECT_NEAR(expected_full_airflow * scaler_cooling_saf, fan->maxAirFlowRate, 0.0001); | ||
|
||
EXPECT_NEAR(622.505, varSpeedCoil.RatedCapCoolTotal, 0.001); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that the Coil:Cooling:DX:VariableSpeed is not affected.
- I don't know if we would want that.
- SizeVarSpeedCoil seem to not call the Autosizing modules. Instead, it uses the FinalZoneSizing's DesCoolVolFlow / DesCoolHeatFlow. cf
EnergyPlus/src/EnergyPlus/VariableSpeedCoils.cc
Lines 4715 to 4717 in f274ccb
CheckZoneSizing(state, format("COIL:{}{}", varSpeedCoil.CoolHeatType, CurrentObjSubfix), varSpeedCoil.Name); RatedAirVolFlowRateDes = max(state.dataSize->FinalZoneSizing(state.dataSize->CurZoneEqNum).DesCoolVolFlow, state.dataSize->FinalZoneSizing(state.dataSize->CurZoneEqNum).DesHeatVolFlow);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The VS coil should be adjusted but it was never converted to use the sizing routines. That begs an open issue to adjust VS coil sizing when DesignSpecification:ZoneHVAC:Sizing is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed at #10699
// capacity sizing methods (HeatingDesignCapacity, CapacityPerFloorArea, FractionOfAutosizedCoolingCapacity, and | ||
// FractionOfAutosizedHeatingCapacity ) | ||
int const CapSizingMethod = zoneHVACSizing.CoolingCapMethod; | ||
zoneEqSizing.SizingMethod(SizingMethod) = CapSizingMethod; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: I'm not sure if this is right... (this is pre-existing)
At this point in time, SizingMethod is either HVAC::CoolingAirflowSizing
, or HVAC::CoolingCapacitySizing
if the SAFMethod is FlowPerCoolingCapacity
.
I think the intent here is to set the CoolingCapacitySizing method specifically, so it should be HVAC::CoolingCapacitySizing
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intent for FlowPerCoolingCapacity is:
- calc design capacity using zone design air flow rate
- set fraction that will adjust zone design air flow when air flow is sized (which will also adjust capacity when capacity is sized)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to be clear, you're saying this is currently correct right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then the next time you call using HVAC::CoolingCapacitySizing the adjusted air flow rate would be used. It appears to me that the capacity sizer should be called here using SizingMethod = HVAC::CoolingCapacitySizing;. It looks like 1) state.dataSize->DataFlowUsedForSizing was not reset above to be an air flow rate (and DataFlowPerCoolingCapacity should be reset to 0), and 2) the sizing method should be explicitly set for capacity sizing (right now is looks to still be fraction of capacity).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you single step this a couple times you should see the capacity getting set using zone design air flow, the air flow being adjusted based on capacity, and the capacity being calculated again using the new air flow rate.
… the variables, initialize them, don't check 4 times if CurZoneEqNum is > 0
ddc41c2
to
a5d32de
Compare
Note: these files are on DefectFiles support repo. Taking the WindACAuto.idf and a modified Version WindACAutoScaled.idf that adds a I also added
There is zero change for the WindACAuto.idf, as expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does look right except that 1 question with SystemAirFlowSizing.
// capacity sizing methods (HeatingDesignCapacity, CapacityPerFloorArea, FractionOfAutosizedCoolingCapacity, and | ||
// FractionOfAutosizedHeatingCapacity ) | ||
int const CapSizingMethod = zoneHVACSizing.CoolingCapMethod; | ||
zoneEqSizing.SizingMethod(SizingMethod) = CapSizingMethod; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intent for FlowPerCoolingCapacity is:
- calc design capacity using zone design air flow rate
- set fraction that will adjust zone design air flow when air flow is sized (which will also adjust capacity when capacity is sized)
@@ -1011,7 +1011,7 @@ namespace WindowAC { | |||
zoneEqSizing.SizingMethod(SizingMethod) = CapSizingMethod; | |||
if (CapSizingMethod == CoolingDesignCapacity || CapSizingMethod == CapacityPerFloorArea || | |||
CapSizingMethod == FractionOfAutosizedCoolingCapacity) { | |||
if (CapSizingMethod == HeatingDesignCapacity) { | |||
if (CapSizingMethod == CoolingDesignCapacity) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Without example files or unit tests these are hard to find.
// Make the Fan be sized by this | ||
zoneEqSizing.CoolingAirFlow = true; | ||
zoneEqSizing.CoolingAirVolFlow = windowAC.MaxAirVolFlow; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In SystemAirFlowSizing, if CoolingAirVolFlow = windowAC.MaxAirVolFlow (which should be already adjusted?) and dataFracOfAutosizedCoolingAirflow = the target fraction, then how does this get the right answer? I would think this would either be 1 * adjusted air flow
or fraction * design air flow
. This looks like it's fraction * adjusted air flow
. Maybe windowAC.MaxAirVolFlow is the design air flow which gets multiplied by the fraction. In any event you are getting the right answer now.
double constexpr expected_full_airflow = 0.041484382187390034; | ||
double constexpr scaler_cooling_saf = 0.5; | ||
double constexpr scaler_cooling_cap = 0.5; | ||
double constexpr expected_full_cap = 622.50474573886743; | ||
|
||
auto const &windowAC = state->dataWindowAC->WindAC(compIndex); | ||
|
||
ASSERT_GT(windowAC.FanIndex, 0); | ||
auto const &fan = state->dataFans->fans(windowAC.FanIndex); | ||
|
||
auto const &finalZoneSizing = state->dataSize->FinalZoneSizing(1); | ||
|
||
EXPECT_NEAR(expected_full_airflow, finalZoneSizing.DesCoolVolFlow, 0.0001); | ||
|
||
ASSERT_GT(windowAC.HVACSizingIndex, 0); | ||
auto const &zoneHVACSizing = state->dataSize->ZoneHVACSizing(windowAC.HVACSizingIndex); | ||
|
||
EXPECT_EQ(DataSizing::FractionOfAutosizedCoolingAirflow, zoneHVACSizing.CoolingSAFMethod); | ||
EXPECT_EQ(scaler_cooling_saf, zoneHVACSizing.MaxCoolAirVolFlow); | ||
EXPECT_EQ(DataSizing::FractionOfAutosizedCoolingCapacity, zoneHVACSizing.CoolingCapMethod); | ||
EXPECT_EQ(scaler_cooling_cap, zoneHVACSizing.ScaledCoolingCapacity); | ||
|
||
EXPECT_EQ(windowAC.DXCoilType_Num, HVAC::Coil_CoolingAirToAirVariableSpeed); | ||
ASSERT_GT(windowAC.DXCoilIndex, 0); | ||
auto const &varSpeedCoil = state->dataVariableSpeedCoils->VarSpeedCoil(windowAC.DXCoilIndex); | ||
|
||
// check Sizing | ||
EXPECT_NEAR(expected_full_airflow * scaler_cooling_saf, windowAC.MaxAirVolFlow, 0.0001); | ||
EXPECT_NEAR(expected_full_airflow * scaler_cooling_saf, fan->maxAirFlowRate, 0.0001); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my previous comment about SystemAirFlowSizing, windowAC.MaxAirVolFlow is already adjusted. Now I'm confused.
EXPECT_NEAR(expected_full_airflow * scaler_cooling_saf, windowAC.MaxAirVolFlow, 0.0001); | ||
EXPECT_NEAR(expected_full_airflow * scaler_cooling_saf, fan->maxAirFlowRate, 0.0001); | ||
|
||
EXPECT_NEAR(622.505, varSpeedCoil.RatedCapCoolTotal, 0.001); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The VS coil should be adjusted but it was never converted to use the sizing routines. That begs an open issue to adjust VS coil sizing when DesignSpecification:ZoneHVAC:Sizing is used.
Thanks for the review on this @rraustad, seems like this is very close, just maybe one remaining issue. |
This data shows that the single speed DX coil air flow was adjusted so it follows that the VS DX coil air flow should also be adjusted. Also in this data, why is the window AC scaled-ori and scaled-new values both equal to 0.02832 and the single speed DX coil air flow rate different? |
} | ||
bool errorsFound = false; | ||
CoolingCapacitySizer sizerCoolingCapacity; | ||
sizerCoolingCapacity.overrideSizingString(SizingString); | ||
sizerCoolingCapacity.overrideSizingString(""); | ||
sizerCoolingCapacity.initializeWithinEP(state, CompType, CompName, PrintFlag, RoutineName); | ||
state.dataSize->DataCapacityUsedForSizing = sizerCoolingCapacity.size(state, TempSize, errorsFound); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this does look correct. Call the cooling capacity sizer which uses zone design air flow.
sizerCoolingCapacity.initializeWithinEP(state, CompType, CompName, PrintFlag, RoutineName); | ||
state.dataSize->DataCapacityUsedForSizing = sizerCoolingCapacity.size(state, TempSize, errorsFound); | ||
state.dataSize->DataFlowPerCoolingCapacity = state.dataSize->ZoneHVACSizing(zoneHVACIndex).MaxCoolAirVolFlow; | ||
state.dataSize->DataFlowPerCoolingCapacity = zoneHVACSizing.MaxCoolAirVolFlow; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set the fraction of air flow here
sizingCoolingAirFlow.initializeWithinEP(state, CompType, CompName, PrintFlag, RoutineName); | ||
state.dataWindowAC->WindAC(WindACNum).MaxAirVolFlow = sizingCoolingAirFlow.size(state, TempSize, errorsFound); | ||
windowAC.MaxAirVolFlow = sizingCoolingAirFlow.size(state, TempSize, errorsFound); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And then size that new air flow rate here. You should also reset state.dataSize->DataFlowPerCoolingCapacity here to be a flow rate instead of a fraction.
@jmarrec these global fraction variables are hard to get right. It may be correct now, just check to make sure the final scaled values are as expected. |
For some reason, probably lack of understanding, I'm severely confused about what's being asked here. Code linting and unit test aside, I haven't touched the sections that are being commented. The actual changes are https://github.com/NREL/EnergyPlus/pull/10673/files/5dbc7b13c8597788540474a075a5eab678a3ce12..a5d32dea2e5a7ea6c68c38b6f073176a9cdc9ce1 I've been looking at other files to see if I could figure out what may be different here compared to other objects EnergyPlus/src/EnergyPlus/WindowAC.cc Lines 977 to 1003 in a5d32de
eg taking FanCoilUnits, EnergyPlus/src/EnergyPlus/FanCoilUnits.cc Lines 1294 to 1314 in a5d32de
Rewritting a bit the code to miminize diffs: --- WindowAC.blob.cc 2024-09-12 12:17:47.094657049 +0200
+++ FanCoilUnits.blob.cc 2024-09-12 12:18:03.988792981 +0200
@@ -2,22 +2,21 @@
SizingMethod = HVAC::CoolingCapacitySizing;
TempSize = DataSizing::AutoSize;
PrintFlag = false;
- state.dataSize->DataScalableSizingON = true;
- state.dataSize->DataFlowUsedForSizing = state.dataSize->FinalZoneSizing(state.dataSize->CurZoneEqNum).DesCoolVolFlow;
- if (zoneHVACSizing.CoolingCapMethod == FractionOfAutosizedCoolingCapacity) {
- state.dataSize->DataFracOfAutosizedCoolingCapacity = zoneHVACSizing.ScaledCoolingCapacity;
- }
bool errorsFound = false;
CoolingCapacitySizer sizerCoolingCapacity;
sizerCoolingCapacity.overrideSizingString("");
sizerCoolingCapacity.initializeWithinEP(state, CompType, CompName, PrintFlag, RoutineName);
- state.dataSize->DataCapacityUsedForSizing = sizerCoolingCapacity.size(state, TempSize, errorsFound);
+ state.dataSize->DataAutosizedCoolingCapacity = sizerCoolingCapacity.size(state, TempSize, errorsFound);
+ if (zoneHVACSizing.CoolingCapMethod == DataSizing::FractionOfAutosizedCoolingCapacity) {
+ state.dataSize->DataFracOfAutosizedCoolingCapacity = zoneHVACSizing.ScaledCoolingCapacity;
+ }
state.dataSize->DataFlowPerCoolingCapacity = zoneHVACSizing.MaxCoolAirVolFlow;
PrintFlag = true;
TempSize = DataSizing::AutoSize;
errorsFound = false;
+ state.dataSize->DataScalableSizingON = true;
CoolingAirFlowSizer sizingCoolingAirFlow;
sizingCoolingAirFlow.overrideSizingString(SizingString);
sizingCoolingAirFlow.initializeWithinEP(state, CompType, CompName, PrintFlag, RoutineName);
- windowAC.MaxAirVolFlow = sizingCoolingAirFlow.size(state, TempSize, errorsFound);
+ CoolingAirVolFlowDes = sizingCoolingAirFlow.size(state, TempSize, errorsFound);
} |
@jmarrec @Myoldmopar it has been 28 days since this pull request was last updated. |
1 similar comment
@jmarrec @Myoldmopar it has been 28 days since this pull request was last updated. |
Pull request overview
DesignSpecification:ZoneHVAC:Sizing
#10663Pull 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.