OA Limiting Factor enum and Related Cleanup #9947
Conversation
mjwitte
added
the
Refactoring
| @@ -3924,6 +3906,15 @@ void VentilationMechanicalProps::CalcMechVentController( | |||
|
|
|||
| // Calculate zone maximum target CO2 concentration in PPM | |||
| if (this->SystemOAMethod == DataSizing::SysOAMethod::ProportionalControlDesOcc) { | |||
| // Accumulate CO2 generation from people at design occupancy and current activity level | |||
| Real64 CO2PeopleGeneration = 0.0; | |||
| for (int PeopleNum = 1; PeopleNum <= state.dataHeatBal->TotPeople; ++PeopleNum) { | |||
There was a problem hiding this comment.
Can probably do for (auto const &people : state.dataHeatBal->People).
| // Accumulate CO2 generation from people at design occupancy and current activity level | ||
| Real64 CO2PeopleGeneration = 0.0; | ||
| for (int PeopleNum = 1; PeopleNum <= state.dataHeatBal->TotPeople; ++PeopleNum) { | ||
| if (state.dataHeatBal->People(PeopleNum).ZonePtr != ZoneNum) continue; |
There was a problem hiding this comment.
There are no per-zone PeopleNum lists I guess.
There was a problem hiding this comment.
There are no per-zone PeopleNum lists I guess.
It does seem wrong to loop through all People statements for all zones. Options include making CO2PeopleGeneration an array and looping though once, storing the values, but this is a system-level thing here, so it only needs the zones on this systems, and a different system might use a different control option. So, per-zone PeopleNum lists are probably the best way to solve this, esp, because 99% of the time, it's a list of one element.
There was a problem hiding this comment.
Will address this with #9969
| enum class OALimitFactor | ||
| { | ||
| Invalid = -1, | ||
| None = 0, // No limit other than fixed OA amount |
There was a problem hiding this comment.
Don't need the numbers.
Yes, but these get used as output variable values which are documented in the I/O Ref, so the numbers are here to emphasize that these shouldn't be rearranged or have anything inserted in the middle of the list.
| @@ -3648,6 +3617,7 @@ void VentilationMechanicalProps::CalcMechVentController( | |||
| // new local variables for DCV | |||
| // Zone OA flow rate based on each calculation method [m3/s] | |||
| std::array<Real64, static_cast<int>(DataSizing::OAFlowCalcMethod::Num)> ZoneOACalc{0.0}; | |||
| Real64 CO2PeopleGeneration = 0; // CO2 generation from people at design level | |||
There was a problem hiding this comment.
Keeping this here at the top, will move while addressing #9969
| if (this->SystemOAMethod == DataSizing::SysOAMethod::ProportionalControlDesOcc) { | ||
| // Accumulate CO2 generation from people at design occupancy and current activity level | ||
| for (int PeopleNum = 1; PeopleNum <= state.dataHeatBal->TotPeople; ++PeopleNum) { |
There was a problem hiding this comment.
This was moved lower, but left unchanged for now.
| // Accumulate CO2 generation from people at design occupancy and current activity level | ||
| Real64 CO2PeopleGeneration = 0.0; | ||
| for (int PeopleNum = 1; PeopleNum <= state.dataHeatBal->TotPeople; ++PeopleNum) { | ||
| if (state.dataHeatBal->People(PeopleNum).ZonePtr != ZoneNum) continue; |
There was a problem hiding this comment.
Will address this with #9969
| @@ -874,8 +874,7 @@ void GetStandAloneERV(EnergyPlusData &state) | |||
| auto &thisOAController = state.dataMixedAir->OAController(OutAirNum); | |||
|
|
|||
| thisOAController.Name = Alphas(1); | |||
| thisOAController.ControllerType = CurrentModuleObject; | |||
| thisOAController.ControllerType_Num = MixedAir::MixedAirControllerType::ControllerStandAloneERV; | |||
| thisOAController.ControllerType = MixedAir::MixedAirControllerType::ControllerStandAloneERV; | |||
There was a problem hiding this comment.
Combine type and num into one variable.
| @@ -140,6 +140,9 @@ using namespace FaultsManager; | |||
|
|
|||
| constexpr std::array<std::string_view, static_cast<int>(ControllerKind::Num)> ControllerKindNamesUC{"CONTROLLER:WATERCOIL", "CONTROLLER:OUTDOORAIR"}; | |||
|
|
|||
| constexpr std::array<std::string_view, static_cast<int>(MixedAirControllerType::Num)> MixedAirControllerTypeNames{ | |||
| "Controller:OutdoorAir", "ZoneHVAC:EnergyRecoveryVentilator:Controller"}; | |||
|
|
|||
There was a problem hiding this comment.
and either is available with the new enum and type array
| thisVentilationMechanical.ZoneSecondaryRecirculation.dimension(MechVentZoneCount, 0.0); | ||
| thisVentilationMechanical.ZoneDesignSpecADObjName.allocate(MechVentZoneCount); | ||
| thisVentilationMechanical.ZoneDesignSpecADObjIndex.dimension(MechVentZoneCount, 0); | ||
| thisVentilationMechanical.VentMechZone.allocate(MechVentZoneCount); |
There was a problem hiding this comment.
move all these mech vent arrays to a new class
src/EnergyPlus/MixedAir.cc
Outdated
| break; | ||
| } | ||
| } | ||
| if (found) { |
There was a problem hiding this comment.
CppCheck says this could be done with any_of but will address that when the time comes.
[src/EnergyPlus/MixedAir.cc:1414]:(style),[useStlAlgorithm],Consider using std::any_of algorithm instead of a raw loop.
| thisVentilationMechanical.VentMechZoneName(MechVentZoneCount) = state.dataHeatBal->Zone(ZoneNum).Name; | ||
| auto &thisMechVentZone = thisVentilationMechanical.VentMechZone(MechVentZoneCount); | ||
| thisMechVentZone.zoneNum = ZoneNum; | ||
| thisMechVentZone.name = state.dataHeatBal->Zone(ZoneNum).Name; |
There was a problem hiding this comment.
begin saving data into new struct
|
|
||
| GetZoneData(*state, ErrorsFound); | ||
| GetOARequirements(*state); | ||
| GetZoneEquipment(*state); | ||
| GetOAControllerInputs(*state); | ||
| ZoneAirLoopEquipmentManager::GetZoneAirLoopEquipment(*state); | ||
| SingleDuct::GetSysInput(*state); |
There was a problem hiding this comment.
I see, so you could call single duct for both TUs.
There was a problem hiding this comment.
I see, so you could call single duct for both TUs.
And keep the connections simple, and not need a fan object, etc.
tst/EnergyPlus/unit/MixedAir.unit.cc
Outdated
| OAMassFlow = state->dataMixedAir->VentilationMechanical(1).CalcMechVentController(*state, SysMassFlow); | ||
|
|
||
| EXPECT_TRUE(OAMassFlow > DesignOAMassFlow); // Expect that the system OA is greater than the design OA air flow | ||
| EXPECT_TRUE(OAMassFlow == SysMassFlow); // Expect that the system OA is less than the system air flow |
There was a problem hiding this comment.
Well, comment says OA is less than system flow, but they are equal. Copy-paste.
| EXPECT_EQ(SysOAMethod::IAQP, state->dataMixedAir->VentilationMechanical(1).SystemOAMethod); | ||
| state->dataMixedAir->VentilationMechanical(1).CalcMechVentController(*state, SysMassFlow, OAMassFlow); | ||
| OAMassFlow = state->dataMixedAir->VentilationMechanical(1).CalcMechVentController(*state, SysMassFlow); |
There was a problem hiding this comment.
now OA flow is returned from the function
| EXPECT_EQ(0.0, OAMassFlow); | ||
|
|
||
| state->dataContaminantBalance->ZoneSysContDemand.deallocate(); | ||
| state->dataScheduleMgr->Schedule.deallocate(); |
There was a problem hiding this comment.
clean up unnecessary deallocate at end of unit test
| state->dataLoopNodes->Node(primaryInletNode2).MassFlowRate = SupplyMassFlow2; | ||
| state->dataLoopNodes->Node(primaryOutletNode1).MassFlowRate = SupplyMassFlow1; | ||
| state->dataLoopNodes->Node(primaryOutletNode2).MassFlowRate = SupplyMassFlow2; | ||
| SysMassFlow = SupplyMassFlow1 + SupplyMassFlow2; // System supply mass flow rate [kg/s] |
There was a problem hiding this comment.
Not sure of the need for code duplication here, lines 5565-5572 and 5580-5585. I don't think those would have changed from previous test. You lost me on the "Starve one zone" comment
There was a problem hiding this comment.
Oh, I see, before SupplyMassFlow1 was /0.5 and now it's *0.5. That makes sense. So the "1" vars changed.
|
Only diff is SolarShading, with no warnings. |
|
It must be the any_of that caused these new diffs? |
| @@ -1471,15 +1466,9 @@ void GetOAControllerInputs(EnergyPlusData &state) | |||
| if (ZoneListNum > 0) { | |||
| for (int ScanZoneListNum = 1; ScanZoneListNum <= state.dataHeatBal->ZoneList(ZoneListNum).NumOfZones; ++ScanZoneListNum) { | |||
| // check to make sure zone name is unique (not listed more than once)... | |||
| int ZoneNum = state.dataHeatBal->ZoneList(ZoneListNum).Zone(ScanZoneListNum); | |||
There was a problem hiding this comment.
So, I mistakenly deleted this line. How did it build, then? Is there another ZoneNum that's still in scope here?
There was a problem hiding this comment.
Wise choice to change the var name. Same name in the outer loop.
|
@Myoldmopar this appears to be clean now so should be ready for merge soon. |
|
@mjwitte it has been 8 days since this pull request was last updated. |
|
Wow, this is great! I'll get on this one now |
Pull request overview
The original defect file for this was a bit of a red herring, because it is an airflownetwork MultizoneWithDistribution model. So, the AFN connection causes slight changes in the mixed air mass flow rate which leads to the supposed anomalies in the OA limiting factor output. Even through the mass flow differences are tiny, the OA controller logic is doing what it is supposed to. So, 8802 can be closed as-designed.
CalcMechVentControllerPull 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.