-
Notifications
You must be signed in to change notification settings - Fork 80
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.
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
Modeling VRFs and Heat Pumps #590
Conversation
Sync fork
Sync with upstream branch
Merge remote-tracking branch 'upstream/master'
Update master
bricksrc/equipment.py
Outdated
@@ -558,6 +565,42 @@ | |||
}, | |||
}, | |||
}, | |||
"Heat_Pump": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In some sense, a split system is more of a "Heat Pump System", where as a packaged unit is a "Heat Pump Equipment"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do have "Heat_Pump_Condensing_Unit", perhaps "Heat_Pump" class could be modified to "Heat_Pump_Packaged_Unit" for clarification.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the distinction between a packaged and split system just the location/relationships. Similar to how an RTU is just an AHU that hasLocation roof.
Package System > feeds > Zone
Split System Condenser Unit > feeds > Evaporator_Unit (do we have this?) > feeds > Zone
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MatthewSteen In the split-system example above, this would be achieved by using the Heat_Pump_Condensing_Unit > feeds > FCU
In this case, do you agree that Heat_Pump should be "Packaged Heat Pump"?
https://www.trane.com/residential/en/products/packaged-systems/heat-pump-packaged-systems/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed "Heat Pump" to "Packaged_Heat_Pump" and added air/water/ground source to all condensing unit classes. In doing so, we can represent the same level of detail for both Packaged and Split System variations.
cbcc5d4
@gtfierro @jbkoh While while "Expansion Valves" and "Metering Devices" are often used interchangeably, a clearer distinction is beneficial. Expansion valves, such as the Thermostatic Expansion Valve (TXV), encompass functionalities beyond traditional valves. A TXV, for instance, adjusts refrigerant flow automatically, guided by a sensing bulb that reacts to temperature changes. Similarly, capillary tubes, another form of metering devices, control refrigerant flow through a fixed orifice and do so without moving parts, relying on the pressure differential. With that being said, Electronic Expansion Valves (EXVs) are closer to traditional valves. Being as though these are designed specifically for metering refrigerant and often containing much more than just a valve, I would prefer to classify this outside of the Valve class, as "Metering Devices" more accurately reflects their unique functionalities in HVAC systems. Alternatively, we could define a Metering Device class in addition to Expansion Valves. Expansion Valve could be a subclass to both Valve and Metering Device. Capillary Tube could exclusively be a subclass to Metering Device, whereas TXV and EXV can be a subclass of both Metering Device and Expansion Valve. |
@@ -5,6 +5,10 @@ | |||
Defining Brick relationships | |||
""" | |||
relationships = { | |||
"connectedTo": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gtfierro Is it necessary to specify that 'connectedTo' is a superclass to 'feeds' and 'isFedBy'?
Something like:
rdfs:subPropertyOf :feeds, :isFedBy ;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my intended design, feeds
and isFedBy
would both be subproperties of connectedTo
, thinking about it more. I'm not sure this is what we actually want to do. If we were to follow this design, then any use of the feeds
relationship would also imply the existence of a connectedTo
relationship.
This raises the essential question, which is "does connectedTo
communicate a bidirectional relationship, or a relationship where the directionality is not known?" If its the former, then connectedTo
should be a sibling of feeds
and isFedBy
because it is altogether different. If its the latter, then connectedTo
should be a superproperty of feeds
and isFedBy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does connectedTo communicate a bidirectional relationship, or a relationship where the directionality is not known?
I intended to use connectedTo to imply a bidirectional relationship. However, considering your point, 'connectedTo' doesn't explicitly convey directionality.
Alternatively, what about FeedsAndFedBy
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably want both:
- "feedsAndFedBy" (we need a better name) for bidirectional. We want this to work for equipment as well as for spaces/locations
- "connectedTo" for a super property of feeds, isFedby, and feedsAndFedBy that means "we don't know the direction"
bricksrc/equipment.py
Outdated
@@ -742,19 +880,41 @@ | |||
}, | |||
"parents": [BRICK.Water_Heater], | |||
}, | |||
"Metering_Device": { | |||
"tags": [TAG.Refrigerant, TAG.Gas, TAG.Liquid, TAG.Metering, TAG.Expansion, TAG.Valve], | |||
"Throttling_Device": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though I do prefer to use Metering Device, I have renamed "Metering Device" to "Throttling Device" to avoid confusion with Brick's Meter class. Throttling device can refer to any component that restricts or regulates the flow of a fluid. It's not specific to refrigeration systems and can apply to various types of mechanical and fluid systems. Open to feedback on this, as I would prefer to use Metering Device if we didn't have a naming conflict.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with the area, but this substitution seems fine for me. We could also go "Metering Device" and then add shackle rules to either Meter class or this class that detect when it's being used incorrectly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not either, but in general I always recommend/want terms to be rooted in a standard (ASHRAE, ISO, etc.). Does Throttling Device
come from somewhere?
I see Throttling Valve
in ASHRAE terminology (although it's not a standard nor are their standards required to use these).
valve used to restrict (throttle) the flow of fluid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gtfierro Ok, great to hear. In that case, I would prefer to call it "Metering Device". @MatthewSteen I was careful not to use the name "valve", due to the various metering devices used in practice, though I do agree that Throttling Device is a bit broad. If you're curious to learn more about metering devices, this is an informative article:
https://www.skillcatapp.com/post/a-complete-guide-to-metering-devices-fixed-modulating.
With the addition of metering device, Brick now has the ability to represent a complete refrigeration cycle using the following classes:
- Compressor
- Reversing Valve (if Heat Pump)
- Metering Device
- Evaporator Coil
- Condenser Coil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I strongly suggest basing the terminology on existing definitions/terms that are as close to canonical as possible so we're not "inventing" our own or basing them on a website. There are a lot of interchangeable terms used in industry but I think Brick should be using "authoritative" sources when it comes to its class names whenever possible.
I may have missed it, but what's the problem with using Throttling_Valve
per ASHRAE terms instead of Metering_Device
? If we use Throttling_Valve
, then it fits nicely under the parent Valve
class like Reversing_Valve
.
If Throttling Device
is too broad, why isn't Metering Device
, which seems a bit general for this application (VRF)? I think "metering" implies measuring something like an electric or gas meter, which aligns with Brick's Meter
class and ASHRAE terms.
instruments that measure electric voltage, current, power, etc.
ASHRAE terms does have this entry, which seems to conflict a bit with its definition of metering.
device that controls the flow of liquid refrigerant to an evaporator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed that reusing ASHRAE terms is ideal. Between the two, I would prefer "Refrigerant Metering Device".
Throttling_Valve is also a good alternative. However, Throttling Valve would only be able to represent modulating metering devices and would not be able to represent fixed metering devices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should be basing Brick's class names on an authoritative/canonical source (such as AHRI, ANSI, ASHRAE, ISO, IEE, etc.), not https://www.skillcatapp.com/.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
100% agreed, I shared the article with you to explain the differences between fixed and modulating metering devices and how throttling valve does not encompass all metering devices.
I have renamed the class to refrigerant metering device per ASHRAE's definition:
https://terminology.ashrae.org/?entry=refrigerant%20metering%20device
A general comment here. Does it make sense for this PR to focus on just VRFs rather than trying to complete the larger/more complex Heat Pump issue? Also, the Haystack Labs Working Group spent quite a bit of time developing the concept of Systems, which for the most part come from existing standards. The VRF concepts came from a separate VRF Systems Working Group. |
Thanks @MatthewSteen,
Yes, I am addressing both VRFs and Heat Pumps in this PR. Regarding the alignment with Haystack's VRF concepts, here are the equivalent classes: vrf-outdoorUnit -> Condensing_Unit Within the Haystack thread, Leroy Simms stated the following:
I, along with others, agree with this perspective, and have chosen not to introduce Outdoor and Indoor for this reason. Please let me know if you have any concerns with the implementation/ alignment. Thanks! |
Improve representation of VRF systems and Heat Pumps, drawing from #581 and integrating concepts from previous PRs: