Add PlantLocation Shortcuts for Streamlined Access to Underlying Data#11428
Add PlantLocation Shortcuts for Streamlined Access to Underlying Data#11428
PlantLocation Shortcuts for Streamlined Access to Underlying Data#11428Conversation
amirroth
added
DoNotPublish
amirroth
left a comment
amirroth
left a comment
There was a problem hiding this comment.
Straightforward code review. Not much to this PR.
|
|
||
| auto &this_comp(branch.Comp(CompCounter)); | ||
| PlantLocation this_plantLoc = {this->plantLoc.loopNum, this->plantLoc.loopSideNum, BranchCounter, CompCounter}; | ||
| this_plantLoc.loop = &state.dataPlnt->PlantLoop(this_plantLoc.loopNum); |
There was a problem hiding this comment.
Update plantLoc struct to include direct pointers to loop, side, branch, and comp objects.
There was a problem hiding this comment.
I was curious of the white space failure in code integrity (1733 and 1734):
| WaterCoils::SimulateWaterCoilComponents(state, CompName, FirstHVACIteration, this->m_HeatCoilNum); | ||
| Real64 const CoilMaxVolFlowRate = WaterCoils::GetCoilMaxWaterFlowRate(state, "Coil:Heating:Water", CompName, ErrorsFound); | ||
| rho = state.dataPlnt->PlantLoop(this->HWPlantLoc.loopNum).glycol->getDensity(state, Constant::HWInitConvTemp, RoutineName); | ||
| rho = this->HWPlantLoc.loop->glycol->getDensity(state, Constant::HWInitConvTemp, RoutineName); |
There was a problem hiding this comment.
This is the standard simplification, just use the direct pointer rather than the lookup index. You will see this another 200 times.
| plantLoc.side = &plantLoc.loop->LoopSide(LoopSideNum); | ||
| plantLoc.branch = &plantLoc.side->Branch(BranchNum); | ||
| plantLoc.comp = &plantLoc.branch->Comp(CompNum); | ||
|
|
There was a problem hiding this comment.
Make sure these pointers are initialized.
| state->dataPlnt->PlantLoop(1).FluidIndex = 1; | ||
| state->dataPlnt->PlantLoop(1).glycol = Fluid::GetWater(*state); | ||
|
|
||
| state->dataWaterCoils->WaterCoil(1).WaterPlantLoc.loopNum = 1; |
There was a problem hiding this comment.
Unit tests have to be updated to to perform these initializations, especially if they do not use IDF snippets and init_state.
This one is on me. I'm working on a fix. |
| if (fanCoil.HCoilType_Num == HCoil::Water) { | ||
| Real64 rho = | ||
| state.dataPlnt->PlantLoop(fanCoil.HeatCoilPlantLoc.loopNum).glycol->getDensity(state, Constant::HWInitConvTemp, RoutineName); | ||
| Real64 rho = fanCoil.HeatCoilPlantLoc.loop->glycol->getDensity(state, Constant::HWInitConvTemp, RoutineName); |
There was a problem hiding this comment.
Hmmm...how did this happen?
|
This looks good. |
mitchute
left a comment
mitchute
left a comment
There was a problem hiding this comment.
Looks good. Let me know if you want me to handle the the code integrity failure.
mitchute
changed the title
PlantLocation Shortcuts for Streamlined Access to Underlying Data
|
This is ready. Merging. |
4 participants
Pull request overview
Description of the purpose of this PR
This PR propagates the pattern of using direct links in the
PlantLocationobject (i.e.,loop,side,branch,comp) instead of location indices (i.e.,LoopNum,LoopSideNum,BranchNum,CompNum) to about 220 places in the code, specifically those the use the plant location object to access a loop's FluidProperties object. A number of places in the code (and in a few unit tests) had to be updated to populate this link information.Pull Request Author
There is one failing unit test
SQLiteFixture.OutputReportTabularAnnual_CurlyBracesbut it fails for a completely independent reason (there is an arithmetic underflow when a number set to maxVal is subtracted from minVal in the table binning routine) and needs its own PR to resolve.