Skip to content

Fix #1722 - CoilSystemCooling Water/DX HeatExchangerAssisted problems#3449

Merged
macumber merged 25 commits intoNatLabRockies:developfrom
jmarrec:PR_opened/Fix_1722_CoilSystemWaterHX
Mar 21, 2019
Merged

Fix #1722 - CoilSystemCooling Water/DX HeatExchangerAssisted problems#3449
macumber merged 25 commits intoNatLabRockies:developfrom
jmarrec:PR_opened/Fix_1722_CoilSystemWaterHX

Conversation

@jmarrec
Copy link
Copy Markdown
Collaborator

@jmarrec jmarrec commented Mar 12, 2019

Fix #1722 - CoilSystemCooling Water/DX HeatExchangerAssisted problems

Changelog:

  • Caught a mistake in \object-list for the CoilSystem:Cooling:DX:HeatExchangerAssisted cooling coil: should allow both CoilCoolingDXSingleSpeed (already the case) AND CoilCoolingDXVariableSpeed
  • Add SPM:MixedAir in FT for the CoilSystemCoolingWaterHeatExchangerAssisted's cooling coil outlet
  • Implement children() and clone() for CoilSystemCoolingWaterHeatExchangerAssisted
  • Implement containingHVACComponent() for all potential children for these two CoilSystemCoolingXXX objects, and modify their addToNode to disallow connection if part of a CoilSystemCoolingXXX
  • Disable addToNode altogether for CoilSystem DX, since it CANNOT be placed directly on an airloop hvac
  • Implemented containingHVACComponent and containingZoneHVACComponent for both CoilSystem objects.
  • Updated relevant unitary/zoneHVAC setCoolingCoil methods to allow these two CoilSystem as appropriate
  • Added a bunch of unit testing for getters, setters, addToNode and clone, and containingHVAC/ZoneHVAC COmponent
  • Added a test in OpenStudio-resources: Add CoilSystemCooling DX/Water HeatExchangerAssisted tests OpenStudio-resources#68

@jmarrec jmarrec marked this pull request as ready for review March 13, 2019 15:34
@macumber macumber added this to the Version 2.8.0 milestone Mar 20, 2019
Copy link
Copy Markdown
Contributor

@macumber macumber left a comment

Choose a reason for hiding this comment

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

Added some minor comments. Is there a regression test that covers the configurations of equipment in this PR? This seems like we need to test the specific configurations of equipment to get at interactions between them, basically we'd like to test all the control paths with a regression test

Comment thread openstudiocore/src/model/AirLoopHVACUnitaryHeatPumpAirToAir.cpp Outdated
Comment thread openstudiocore/src/model/AirLoopHVACUnitaryHeatPumpAirToAir.cpp Outdated
Comment thread openstudiocore/src/model/AirLoopHVACUnitaryHeatPumpAirToAir.cpp Outdated
Comment thread openstudiocore/src/model/AirLoopHVACUnitaryHeatPumpAirToAirMultiSpeed.cpp Outdated
@jmarrec jmarrec dismissed macumber’s stale review March 21, 2019 09:01

Addressed in last two commits

@jmarrec jmarrec requested a review from macumber March 21, 2019 09:01
@jmarrec
Copy link
Copy Markdown
Collaborator Author

jmarrec commented Mar 21, 2019

Added some minor comments. Is there a regression test that covers the configurations of equipment in this PR? This seems like we need to test the specific configurations of equipment to get at interactions between them, basically we'd like to test all the control paths with a regression test

As linked above, I added one test for the water version and another for the dx version in NatLabRockies/OpenStudio-resources#68. Isn't that enough? I'm a bit unclear about what you mean by control path here, does this mean I should also test the DX version with a Cooling:Cooling:DX:VariableSpeed in addition to the SingleSpeed one? Do I also need to test the water one inside of a unitary sys? I don't see any potential other configuration but I might be missing the point

(For what it's worth, I don't think the vast majority of the openstudio-resources test currently check for every possible combination there is)

@macumber macumber merged commit aeaa596 into NatLabRockies:develop Mar 21, 2019
@jmarrec jmarrec deleted the PR_opened/Fix_1722_CoilSystemWaterHX branch October 21, 2019 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants