-
Notifications
You must be signed in to change notification settings - Fork 390
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
Coil:Cooling:DX updates #8558
Coil:Cooling:DX updates #8558
Conversation
Coil refactor feb2020 rich
…us into CoilRefactorFeb2020
…us into CoilRefactorFeb2020
…EnergyPlus into CoilRefactorFeb2020
…us into CoilRefactorFeb2020
…us into CoilRefactorFeb2020
…EnergyPlus into CoilRefactorFeb2020
…EnergyPlus into CoilRefactorFeb2020
…EnergyPlus into CoilRefactorFeb2020
bool SubcoolReheatFlag = false; // Subcool reheat coil control | ||
|
||
CoilCoolingDXCurveFitSpeed &normModeNomSpeed(); | ||
CoilCoolingDXCurveFitSpeed &altModeNomSpeed(); |
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 is beyond me. And also not in develop. Maybe it will make sense later.
CoilCoolingDXCurveFitSpeed & CoilCoolingDX::normModeNomSpeed() | ||
{ | ||
return this->performance.normalMode.speeds[this->performance.normalMode.nominalSpeedIndex]; | ||
} |
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.
Ah, these are functions. This syntax is just confusing to me. See what you learn reviewing new code.
This:
this->normModeNomSpeed().condenser_air_flow_rate;
is the same as:
this->performance.normalMode.speeds[this->performance.normalMode.nominalSpeedIndex].condenser_air_flow_rate;
} else { | ||
return this->normModeNomSpeed().condenser_air_flow_rate; | ||
} | ||
} |
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 is volume flow rate, at least I didn't see it converted to mass, and the function name is condMassFlowRate.
Real64 const coolingRate = dummyEvapInlet.MassFlowRate * (dummyEvapInlet.Enthalpy - dummyEvapOutlet.Enthalpy); | ||
Real64 const thisMinAirHumRat = min(dummyEvapInlet.HumRat, dummyEvapOutlet.HumRat); | ||
Real64 const sensCoolingRate = dummyEvapInlet.MassFlowRate * (Psychrometrics::PsyHFnTdbW(dummyEvapInlet.Temp, thisMinAirHumRat) - | ||
Psychrometrics::PsyHFnTdbW(dummyEvapOutlet.Temp, thisMinAirHumRat)); |
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.
Didn't we have a different calculation for sens cap? A new @Nigusse function somewhere?
src/EnergyPlus/UnitarySystem.cc
Outdated
@@ -11467,6 +11532,22 @@ namespace UnitarySystems { | |||
DXCoils::SimDXCoilMultiMode( | |||
state, CompName, state.dataUnitarySystems->On, FirstHVACIteration, PartLoadFrac, DehumidMode, this->m_CoolingCoilIndex, FanOpMode); | |||
this->m_CompPartLoadRatio = PartLoadFrac; | |||
} else if (CoilType_Num == DataHVACGlobals::CoilDX_Cooling) { // CoilCoolingDX |
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.
Hmm, the new coil model isn't included in SP control. I thought I added that a while back.
autosize, !- Minimum Outdoor Air Flow Rate {m3/s} | ||
autosize, !- Maximum Outdoor Air Flow Rate {m3/s} | ||
0.0, !- Minimum Outdoor Air Flow Rate {m3/s} | ||
0.0, !- Maximum Outdoor Air Flow Rate {m3/s} |
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.
I assume you are trying to target another configuration that we don't have yet with these type of changes to idfs.
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.
No, these changes were supposed to remain local. Will revert this and a couple of others.
EXPECT_NEAR(thisSys->m_CycRatio, 1.000, 0.001); | ||
// EXPECT_NEAR(thisSys->m_SpeedRatio, 0.228062, 0.001); | ||
EXPECT_NEAR(thisSys->m_SpeedRatio, 0.12, 0.001); | ||
EXPECT_NEAR(sensOut, -11998.0, 210.0); |
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.
OK, made it through to the end. This is a lot to take in but nothing else jumped out at me.
@Myoldmopar @mitchute This is as far as I'll get for now, maybe more next week if it's still open. This isn't perfect yet, but it's good progress. Someone should look at the annual energy diffs on these files to make sure this doesn't break them. |
@mjwitte can you remind me if we have a 1:1 list of baseline vs new coil object IDF files? This was (almost) an entire pandemic ago after all... 🙃 I'll plot some of the results from these files and compare the results. |
Ah, OK. Thanks for the reminder. |
…ilRefactorFeb2020Feb2021
…ilRefactorFeb2020Feb2021
…ilRefactorFeb2020Feb2021
…ilRefactorFeb2020Feb2021
…ilRefactorFeb2020Feb2021
…ilRefactorFeb2020Feb2021
Not waiting any longer on CI, thank @Myoldmopar for a local quick regression test. Merging. See top comment for discussion of diffs. |
Pull request overview
This supersedes #8292. This is a group effort by @mjwitte @Myoldmopar @rraustad and @mitchute .
Fill in feature gaps and clean up Coil:Cooling:DX, including:
Still needs work:
Diffs
As of 03 Mar 2021 (sunset in Chicagoland)
develop at a12bdf0 vs CoilBaseline has small or no diffs for:
UnitarySystem_HeatPumpAuto
UnitarySystem_SingleSpeedDX
All of the other files have big diffs.
This branch at add89b6 vs CoilBaseline has small or no diffs for:
UnitarySystem_HeatPumpAuto
UnitarySystem_SingleSpeedDX
UnitarySystem_DXCoilSystemAuto
UnitarySystem_FurnaceWithDXSystemRHcontrol
UnitarySystem_MultiSpeedDX
All of the other files still have big diffs.
So, this is progress.
Comparing annual simulations for this branch vs develop (new coil vs new coil) today, 26 Mar 2021
Most files have annual energy diffs on the order of 5% or less.
Files with larger diffs:
UnitarySystem_FurnaceWithDXSystemRHcontrol: +5% cooling elec, +20% heating gas, -44% Time Not Comfortable Based on Simple ASHRAE 55-2004 - but now it agrees with the baseline model - so OK.
UnitarySystem_DXCoilSystemAuto: +100% cooling electricity - wasn't even working before - not it agrees with baseline - OK.
UnitarySystem_MultiSpeedDX: +10% cooling elec, agrees with baseline now - OK.
UnitarySystem_MultiSpeedCoils_SingleMode: +25% cooling elec, still doesn't agree with baseline, so will need more work. Not holding this up for that.
UnitarySystem_SubcoolReheatDX: +25% cooling elec, no baseline to compare against, but the eso has small diffs, only the energy use has big diffs.
Biggest changes in HVAC elec are in Jan and Jul. Needs more investigation.
Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
If any defect files are updated to a more recent version, upload new versions here or on DevSupportIf IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange labelIf structural output changes, add to output rules file and add OutputChange labelIf adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependenciesReviewer
This will not be exhaustively relevant to every PR.