Skip to content
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.

Sign up for GitHub

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 #5313 - AirLoopHVACUnitaryHeatPump(Multispeed) fixes #5322

Merged
merged 4 commits into from
Jan 15, 2025
Merged

Fix #5313 - AirLoopHVACUnitaryHeatPump(Multispeed) fixes #5322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c5bfaca
Add tests for #5313 + ALL tests (none exists) for AirLoopHVACUnitaryH…
jmarrec Dec 18, 2024
33fd849
Fixup crash when ctor fails to assign one of the child component: rem…
jmarrec Dec 18, 2024
93c150f
AirLoopHVACUnitaryHeatPumpAirToAirMultiSpeed: restrict addToNode + fi…
jmarrec Dec 18, 2024
1ede1bd
#5313 - Accept HeatingCoil:Water for supplemental HC for AirLoopHVACU…
jmarrec Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions resources/model/OpenStudio.idd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11569,6 +11569,7 @@ OS:AirLoopHVAC:UnitaryHeatPump:AirToAir,
\type object-list
\required-field
\object-list HeatingCoilsGasElec
\object-list HeatingCoilsWater
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specific for #5313 - Suppl HC should allow Coil:Heating:Water (Coil:Heating:Steam isn't wrapped in OS)

N4, \field Maximum Supply Air Temperature from Supplemental Heater
\type real
\required-field
Expand Down
204 changes: 50 additions & 154 deletions src/model/AirLoopHVACUnitaryHeatPumpAirToAir.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
#include "CoilHeatingElectric_Impl.hpp"
#include "CoilHeatingGas.hpp"
#include "CoilHeatingGas_Impl.hpp"
#include "CoilHeatingWater.hpp"
#include "CoilHeatingWater_Impl.hpp"
#include "FanConstantVolume.hpp"
#include "FanConstantVolume_Impl.hpp"
#include "FanOnOff.hpp"
Expand Down Expand Up @@ -114,10 +116,19 @@ namespace model {
std::vector<ModelObject> AirLoopHVACUnitaryHeatPumpAirToAir_Impl::children() const {
std::vector<ModelObject> result;

result.push_back(supplyAirFan());
result.push_back(coolingCoil());
result.push_back(heatingCoil());
result.push_back(supplementalHeatingCoil());
// Avoid crashing when calling remove() in Ctor when failing to set one of the child component by calling the optional one
if (boost::optional<HVACComponent> supplyFan = this->optionalSupplyAirFan()) {
result.push_back(std::move(*supplyFan));
}
if (boost::optional<HVACComponent> coolingCoil = this->optionalCoolingCoil()) {
result.push_back(std::move(*coolingCoil));
}
if (boost::optional<HVACComponent> heatingCoil = this->optionalHeatingCoil()) {
result.push_back(std::move(*heatingCoil));
}
if (boost::optional<HVACComponent> supplementalHeatingCoil = this->optionalSupplementalHeatingCoil()) {
result.push_back(std::move(*supplementalHeatingCoil));
}
Comment on lines +119 to +131
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix for the crash is here


return result;
}
Expand Down Expand Up @@ -206,44 +217,51 @@ namespace model {
}

HVACComponent AirLoopHVACUnitaryHeatPumpAirToAir_Impl::supplyAirFan() const {
boost::optional<HVACComponent> result;

result = getObject<ModelObject>().getModelObjectTarget<HVACComponent>(OS_AirLoopHVAC_UnitaryHeatPump_AirToAirFields::SupplyAirFanName);

OS_ASSERT(result);

return result.get();
boost::optional<HVACComponent> value = optionalSupplyAirFan();
if (!value) {
LOG_AND_THROW(briefDescription() << " does not have an Supply Air Fan attached.");
}
return value.get();
}

HVACComponent AirLoopHVACUnitaryHeatPumpAirToAir_Impl::heatingCoil() const {
boost::optional<HVACComponent> result;

result = getObject<ModelObject>().getModelObjectTarget<HVACComponent>(OS_AirLoopHVAC_UnitaryHeatPump_AirToAirFields::HeatingCoilName);

OS_ASSERT(result);

return result.get();
boost::optional<HVACComponent> value = optionalHeatingCoil();
if (!value) {
LOG_AND_THROW(briefDescription() << " does not have an Heating Coil attached.");
}
return value.get();
}

HVACComponent AirLoopHVACUnitaryHeatPumpAirToAir_Impl::coolingCoil() const {
boost::optional<HVACComponent> result;

result = getObject<ModelObject>().getModelObjectTarget<HVACComponent>(OS_AirLoopHVAC_UnitaryHeatPump_AirToAirFields::CoolingCoilName);

OS_ASSERT(result);

return result.get();
boost::optional<HVACComponent> value = optionalCoolingCoil();
if (!value) {
LOG_AND_THROW(briefDescription() << " does not have an Cooling Coil attached.");
}
return value.get();
}

HVACComponent AirLoopHVACUnitaryHeatPumpAirToAir_Impl::supplementalHeatingCoil() const {
boost::optional<HVACComponent> result;
auto value = optionalSupplementalHeatingCoil();
if (!value) {
LOG_AND_THROW(briefDescription() << " does not have a Supplemental Heating Coil attached.");
}
return value.get();
}

boost::optional<HVACComponent> AirLoopHVACUnitaryHeatPumpAirToAir_Impl::optionalSupplyAirFan() const {
return getObject<ModelObject>().getModelObjectTarget<HVACComponent>(OS_AirLoopHVAC_UnitaryHeatPump_AirToAirFields::SupplyAirFanName);
}

result =
getObject<ModelObject>().getModelObjectTarget<HVACComponent>(OS_AirLoopHVAC_UnitaryHeatPump_AirToAirFields::SupplementalHeatingCoilName);
boost::optional<HVACComponent> AirLoopHVACUnitaryHeatPumpAirToAir_Impl::optionalHeatingCoil() const {
return getObject<ModelObject>().getModelObjectTarget<HVACComponent>(OS_AirLoopHVAC_UnitaryHeatPump_AirToAirFields::HeatingCoilName);
}

OS_ASSERT(result);
boost::optional<HVACComponent> AirLoopHVACUnitaryHeatPumpAirToAir_Impl::optionalCoolingCoil() const {
return getObject<ModelObject>().getModelObjectTarget<HVACComponent>(OS_AirLoopHVAC_UnitaryHeatPump_AirToAirFields::CoolingCoilName);
}

return result.get();
boost::optional<HVACComponent> AirLoopHVACUnitaryHeatPumpAirToAir_Impl::optionalSupplementalHeatingCoil() const {
return getObject<ModelObject>().getModelObjectTarget<HVACComponent>(OS_AirLoopHVAC_UnitaryHeatPump_AirToAirFields::SupplementalHeatingCoilName);
}

boost::optional<double> AirLoopHVACUnitaryHeatPumpAirToAir_Impl::maximumSupplyAirTemperaturefromSupplementalHeater() const {
Expand Down Expand Up @@ -419,6 +437,8 @@ namespace model {
isTypeOK = true;
} else if (hvacComponent.optionalCast<CoilHeatingElectric>()) {
isTypeOK = true;
} else if (hvacComponent.optionalCast<CoilHeatingWater>()) {
isTypeOK = true;
Comment on lines +440 to +441
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here too

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we check type on Multispeed?

Copy link
Collaborator Author

@jmarrec jmarrec Jan 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

OS:AirLoopHVAC:UnitaryHeatPump:AirToAir,
  [...]
  A10, \field Supplemental Heating Coil Name
       \note Needs to match in the supplemental heating coil object
       \type object-list
       \required-field
+      \object-list HeatingCoilsGasElec
OS:AirLoopHVAC:UnitaryHeatPump:AirToAir:MultiSpeed,
   A12, \field Supplemental Heating Coil
        \type object-list
+       \object-list HeatingCoilName

Technically speaking, checking specifically on the AirLoopHVACUnitaryHeatPumpAirToAir::setSupplementalHeatingCoil is probably pointless unless the HeatingCoilsGasElec englobes more coils than are actually accepted, since setPointer will enforce the references. But 1) it was existing, so I'm just adding to it, and 2) it is probably safer?

The Multispeed accepts all coils.

}

if (isTypeOK) {
Expand Down Expand Up @@ -488,130 +508,6 @@ namespace model {
OS_ASSERT(result);
}

boost::optional<ModelObject> AirLoopHVACUnitaryHeatPumpAirToAir_Impl::availabilityScheduleAsModelObject() const {
OptionalModelObject result = availabilitySchedule();
return result;
}

boost::optional<ModelObject> AirLoopHVACUnitaryHeatPumpAirToAir_Impl::controllingZoneAsModelObject() const {
OptionalModelObject result;
OptionalThermalZone intermediate = controllingZone();
if (intermediate) {
result = *intermediate;
}
return result;
}

boost::optional<ModelObject> AirLoopHVACUnitaryHeatPumpAirToAir_Impl::supplyAirFanAsModelObject() const {
OptionalModelObject result = supplyAirFan();
return result;
}

boost::optional<ModelObject> AirLoopHVACUnitaryHeatPumpAirToAir_Impl::heatingCoilAsModelObject() const {
OptionalModelObject result = heatingCoil();
return result;
}

boost::optional<ModelObject> AirLoopHVACUnitaryHeatPumpAirToAir_Impl::coolingCoilAsModelObject() const {
OptionalModelObject result = coolingCoil();
return result;
}

boost::optional<ModelObject> AirLoopHVACUnitaryHeatPumpAirToAir_Impl::supplementalHeatingCoilAsModelObject() const {
OptionalModelObject result = supplementalHeatingCoil();
return result;
}

boost::optional<ModelObject> AirLoopHVACUnitaryHeatPumpAirToAir_Impl::supplyAirFanOperatingModeScheduleAsModelObject() const {
OptionalModelObject result;
OptionalSchedule intermediate = supplyAirFanOperatingModeSchedule();
if (intermediate) {
result = *intermediate;
}
return result;
}

bool AirLoopHVACUnitaryHeatPumpAirToAir_Impl::setAvailabilityScheduleAsModelObject(const boost::optional<ModelObject>& modelObject) {
if (modelObject) {
OptionalSchedule intermediate = modelObject->optionalCast<Schedule>();
if (intermediate) {
Schedule schedule(*intermediate);
return setAvailabilitySchedule(schedule);
}
}
return false;
}

bool AirLoopHVACUnitaryHeatPumpAirToAir_Impl::setControllingZoneAsModelObject(const boost::optional<ModelObject>& modelObject) {
if (modelObject) {
OptionalThermalZone intermediate = modelObject->optionalCast<ThermalZone>();
if (intermediate) {
setControllingZone(*intermediate);
return true;
}
}
return false;
}

bool AirLoopHVACUnitaryHeatPumpAirToAir_Impl::setSupplyAirFanAsModelObject(const boost::optional<ModelObject>& modelObject) {
if (modelObject) {
OptionalHVACComponent intermediate = modelObject->optionalCast<HVACComponent>();
if (intermediate) {
setSupplyAirFan(*intermediate);
return true;
}
}
return false;
}

bool AirLoopHVACUnitaryHeatPumpAirToAir_Impl::setHeatingCoilAsModelObject(const boost::optional<ModelObject>& modelObject) {
if (modelObject) {
OptionalHVACComponent intermediate = modelObject->optionalCast<HVACComponent>();
if (intermediate) {
setHeatingCoil(*intermediate);
return true;
}
}
return false;
}

bool AirLoopHVACUnitaryHeatPumpAirToAir_Impl::setCoolingCoilAsModelObject(const boost::optional<ModelObject>& modelObject) {
if (modelObject) {
OptionalHVACComponent intermediate = modelObject->optionalCast<HVACComponent>();
if (intermediate) {
setCoolingCoil(*intermediate);
return true;
}
}
return false;
}

bool AirLoopHVACUnitaryHeatPumpAirToAir_Impl::setSupplementalHeatingCoilAsModelObject(const boost::optional<ModelObject>& modelObject) {
if (modelObject) {
OptionalHVACComponent intermediate = modelObject->optionalCast<HVACComponent>();
if (intermediate) {
setSupplementalHeatingCoil(*intermediate);
return true;
}
}
return false;
}

bool AirLoopHVACUnitaryHeatPumpAirToAir_Impl::setSupplyAirFanOperatingModeScheduleAsModelObject(const boost::optional<ModelObject>& modelObject) {
if (modelObject) {
OptionalSchedule intermediate = modelObject->optionalCast<Schedule>();
if (intermediate) {
Schedule schedule(*intermediate);
return setSupplyAirFanOperatingModeSchedule(schedule);
} else {
return false;
}
} else {
resetSupplyAirFanOperatingModeSchedule();
}
return true;
}

boost::optional<double> AirLoopHVACUnitaryHeatPumpAirToAir_Impl::autosizedSupplyAirFlowRateDuringCoolingOperation() const {
return getAutosizedValue("Supply Air Flow Rate During Cooling Operation", "m3/s");
}
Expand Down
43 changes: 30 additions & 13 deletions src/model/AirLoopHVACUnitaryHeatPumpAirToAirMultiSpeed.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@
#include "Model_Impl.hpp"
#include "AirLoopHVACUnitaryHeatPumpAirToAirMultiSpeed.hpp"
#include "AirLoopHVACUnitaryHeatPumpAirToAirMultiSpeed_Impl.hpp"
#include "AirLoopHVAC.hpp"
#include "Node.hpp"
#include "Schedule.hpp"
#include "Schedule_Impl.hpp"
#include "ThermalZone.hpp"
#include "ThermalZone_Impl.hpp"
#include "ScheduleTypeLimits.hpp"
#include "ScheduleTypeRegistry.hpp"
#include "ThermalZone.hpp"
#include "ThermalZone_Impl.hpp"

#include "../utilities/core/Assert.hpp"
#include "../utilities/data/DataEnums.hpp"
Expand Down Expand Up @@ -121,8 +123,7 @@ namespace model {
}

HVACComponent AirLoopHVACUnitaryHeatPumpAirToAirMultiSpeed_Impl::supplementalHeatingCoil() const {
auto value = getObject<ModelObject>().getModelObjectTarget<HVACComponent>(
OS_AirLoopHVAC_UnitaryHeatPump_AirToAir_MultiSpeedFields::SupplementalHeatingCoil);
auto value = optionalSupplementalHeatingCoil();
if (!value) {
LOG_AND_THROW(briefDescription() << " does not have a Supplemental Heating Coil attached.");
}
Expand Down Expand Up @@ -583,6 +584,11 @@ namespace model {
return getObject<ModelObject>().getModelObjectTarget<HVACComponent>(OS_AirLoopHVAC_UnitaryHeatPump_AirToAir_MultiSpeedFields::CoolingCoil);
}

boost::optional<HVACComponent> AirLoopHVACUnitaryHeatPumpAirToAirMultiSpeed_Impl::optionalSupplementalHeatingCoil() const {
return getObject<ModelObject>().getModelObjectTarget<HVACComponent>(
OS_AirLoopHVAC_UnitaryHeatPump_AirToAir_MultiSpeedFields::SupplementalHeatingCoil);
}

unsigned AirLoopHVACUnitaryHeatPumpAirToAirMultiSpeed_Impl::inletPort() const {
return OS_AirLoopHVAC_UnitaryHeatPump_AirToAir_MultiSpeedFields::AirInletNode;
}
Expand All @@ -591,8 +597,18 @@ namespace model {
return OS_AirLoopHVAC_UnitaryHeatPump_AirToAir_MultiSpeedFields::AirOutletNode;
}

bool AirLoopHVACUnitaryHeatPumpAirToAirMultiSpeed_Impl::addToNode(Node& node) {
if (boost::optional<AirLoopHVAC> airLoop = node.airLoopHVAC()) {
if (airLoop->supplyComponent(node.handle())) {
return StraightComponent_Impl::addToNode(node);
}
}

return false;
}
Comment on lines +600 to +608
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restrict addToNode.

Note that ideally we should also allow demand side of a plant loop and connect the Heat Recovery Water Nodes there, but outside the scope and no one ever asked for it.


ModelObject AirLoopHVACUnitaryHeatPumpAirToAirMultiSpeed_Impl::clone(Model model) const {
auto modelObjectClone = ModelObject_Impl::clone(model).cast<AirLoopHVACUnitaryHeatPumpAirToAirMultiSpeed>();
auto modelObjectClone = StraightComponent_Impl::clone(model).cast<AirLoopHVACUnitaryHeatPumpAirToAirMultiSpeed>();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixup clone here


if (boost::optional<HVACComponent> supplyFan = this->supplyAirFan()) {
modelObjectClone.setSupplyAirFan(supplyFan->clone(model).cast<HVACComponent>());
Expand All @@ -613,17 +629,18 @@ namespace model {
std::vector<ModelObject> AirLoopHVACUnitaryHeatPumpAirToAirMultiSpeed_Impl::children() const {
std::vector<ModelObject> result;

if (boost::optional<HVACComponent> supplyFan = this->supplyAirFan()) {
result.push_back(*supplyFan);
// Avoid crashing when calling remove() in Ctor when failing to set one of the child component by calling the optional one
if (boost::optional<HVACComponent> supplyFan = this->optionalSupplyAirFan()) {
result.push_back(std::move(*supplyFan));
}
if (boost::optional<HVACComponent> coolingCoil = this->coolingCoil()) {
result.push_back(*coolingCoil);
if (boost::optional<HVACComponent> coolingCoil = this->optionalCoolingCoil()) {
result.push_back(std::move(*coolingCoil));
}
if (boost::optional<HVACComponent> heatingCoil = this->heatingCoil()) {
result.push_back(*heatingCoil);
if (boost::optional<HVACComponent> heatingCoil = this->optionalHeatingCoil()) {
result.push_back(std::move(*heatingCoil));
}
if (boost::optional<HVACComponent> supplementalHeatingCoil = this->supplementalHeatingCoil()) {
result.push_back(*supplementalHeatingCoil);
if (boost::optional<HVACComponent> supplementalHeatingCoil = this->optionalSupplementalHeatingCoil()) {
result.push_back(std::move(*supplementalHeatingCoil));
}

return result;
Expand Down
14 changes: 9 additions & 5 deletions src/model/AirLoopHVACUnitaryHeatPumpAirToAirMultiSpeed_Impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ namespace model {

virtual std::vector<ScheduleTypeKey> getScheduleTypeKeys(const Schedule& schedule) const override;

virtual std::vector<ModelObject> children() const override;

virtual unsigned inletPort() const override;
virtual unsigned outletPort() const override;
virtual bool addToNode(Node& node) override;

virtual ModelObject clone(Model model) const override;

virtual ComponentType componentType() const override;
virtual std::vector<FuelType> coolingFuelTypes() const override;
virtual std::vector<FuelType> heatingFuelTypes() const override;
Expand Down Expand Up @@ -236,11 +244,6 @@ namespace model {
/** @name Other */
//@{

virtual unsigned inletPort() const override;
virtual unsigned outletPort() const override;
std::vector<ModelObject> children() const override;
ModelObject clone(Model model) const override;

//@}
protected:
private:
Expand All @@ -249,6 +252,7 @@ namespace model {
boost::optional<HVACComponent> optionalSupplyAirFan() const;
boost::optional<HVACComponent> optionalHeatingCoil() const;
boost::optional<HVACComponent> optionalCoolingCoil() const;
boost::optional<HVACComponent> optionalSupplementalHeatingCoil() const;
};

} // namespace detail
Expand Down
Loading
Loading