AWHP followup fix output table #11276
Conversation
yujiex
added
Defect
|
|
|
|
|
|
src/EnergyPlus/Plant/PlantManager.cc
Outdated
| state, PlantEquipmentType::HeatPumpAirToWaterHeating, CompNames(CompNum)); | ||
| this_comp.Type = PlantEquipmentType::HeatPumpAirToWaterHeating; | ||
| this_comp.TypeOf = "HeatPumpAirToWaterHeating"; | ||
| this_comp.TypeOf = "HeatPumpAirToWater"; |
There was a problem hiding this comment.
Just delete this line. this_comp.TypeOf is set above (line 892) to the string used in the branch input, which is "HeatPump:AirToWater".
There was a problem hiding this comment.
indeed. I just removed this.
src/EnergyPlus/Plant/PlantManager.cc
Outdated
| state, PlantEquipmentType::HeatPumpAirToWaterCooling, CompNames(CompNum)); | ||
| this_comp.Type = PlantEquipmentType::HeatPumpAirToWaterCooling; | ||
| this_comp.TypeOf = "HeatPumpAirToWaterCooling"; | ||
| this_comp.TypeOf = "HeatPumpAirToWater"; |
| break; | ||
| } | ||
| case PlantEquipmentType::HeatPumpAirToWater: { | ||
| if (state.dataPlnt->PlantLoop(LoopNum).TypeOfWaterLoop == DataPlant::WaterLoopType::HotWater) { |
There was a problem hiding this comment.
This check for TypeOfWaterLoop will fail is the user does not specify the "Water Loop Type" in the PlantLoop object. Given it's the last field, it's likely most users/interfaces will let it default to "None". Is there a check somewhere to throw a severe error and inform the user?
PlantLoop,
A19; \field Water Loop Type
\type choice
\key HotWater
\key ChilledWater
\key None
\default None
I tested one of the AHWP files with the Water Loop Type field removed, this happens:
** Fatal ** Plant component "HEATPUMP:AIRTOWATER" was not assigned a pointer.
It should be possible to check the TypeOfWaterLoop in HeatPumpAirToWater::oneTimeInit after PlantUtilities::ScanPlantLoopsForObject returns this->loadSidePlantLoc. But I don't see ScanPlantLoopsForObject there, like the other oneTimeInit functions in PlantLoopHeatPumpEIR.
Oh, I see, HeatPumpAirToWater::oneTimeInit calls EIRPlantLoopHeatPump::oneTimeInit. So you should be able to check the loop types there and throw error messages. Ideally you shouldn't need that to be set on the PlantLoop, but I don't see a way around it with the current object structure.
There was a problem hiding this comment.
make sense, I added the error check in the EIRPlantLoopHeatPump::oneTimeInit function
There was a problem hiding this comment.
@mjwitte @yujiex
Is this new AWHP intended to be able to provide both heating and cooling on a condenser loop? If so, shouldn't the new "Water Loop Type" field have a "CondenserWater" or "HeatingAndCooling" option? That would align with the Sizing:Plant object type offering a "Condenser" option for its Loop Type input field.
There was a problem hiding this comment.
For now it only allows either heating or cooling on the plantloop, as it need to use this field to identify if the internal component is the heating one or the cooling one. Note that the object internally still has a heating component and a cooling component, using the same type of data structure as the plantloop heat pumps.
If later on the object is refactored into separate heating/cooling component, it should be able to allow this. But currently not yet. @aaron-boranian
I can try to see if there's a way to alter the internal logic to tell the internal ones apart (will create a separate PR for this) when they're on the same loop, but I don't think we can add another option like "HeatingAndCooling" as idd can't change right now. We might need to use the "None" option for now.
| modeKeyWord = "Cooling"; | ||
| } | ||
| objectName = format("{}:{}", this->name, modeKeyWord); | ||
| objectName = format("{}\n{} Component", this->name, modeKeyWord); |
There was a problem hiding this comment.
Oh, I see, for this table:
This will have to be ok for now, but take out the \n. It's fine for humans but may trip up software that's reading the tables.
There was a problem hiding this comment.
is replacing "\n" with a space be okay? If I don't put anything there, the AWHPHeating will stick together.
|
|
|
|
|
It looks like everyone is pretty much done here. Are we expecting anything else? |
|
|
Pull request overview
Fixes comments in #11251
Description of the purpose of this PR
Pull Request Author
Reviewer