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.

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

Create get_spaces_served_by_SWH_use.md #1407

Merged
merged 8 commits into from
Sep 6, 2024

Conversation

KarenWGard
Copy link
Collaborator

No description provided.

Logic:

- find the building_segment that contains this swh_use by looking through each building segment: `for building_segment in RMD.building_segments:`
- check if the swh_use is in this building segment: `if swh_use in? building_segment.service_water_heating_uses:`
Copy link
Collaborator

@claperle claperle Jul 12, 2024

Choose a reason for hiding this comment

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

Looks like there may be a typo on this line (i.e., not understanding why the ? is included after in)

- look eat each building segment: `for building_segment in RMD.building_segments`
- check if the swh_use is in this building_segment: `if swh_use in? building_segment.service_water_heating_uses:`
- look at each space in the building segment to see which of them reference the swh_use: `for space in building_segment.spaces:`
- if the swh_use ID is in the list of space ServiceWaterHeatingUses, then the space is one of the spaces and we add it to the list: `if swh_use.id in? space.service_water_heating_uses: spaces << space`
Copy link
Collaborator

@claperle claperle Jul 12, 2024

Choose a reason for hiding this comment

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

Looks like there may be a typo on this line (i.e., not understanding why the ? is included after in).

Need to update code to python instead od Ruby syntax.

changed function based on discussion had on call 7/18 - for swh_us with unit type power or volume, and it it's applied to no spaces, only count it once. If it's explicitly applied to any spaces, then count it once for each space.
- look at each space in the building segment to see which of them reference the swh_use: `for space in building_segment.spaces:`
- if the swh_use ID is in the list of space ServiceWaterHeatingUses, then the space is one of the spaces and we add it to the list: `if swh_use.id in space.service_water_heating_uses: spaces << space`
- here, we check whether there are any spaces applied to the swh_use. If no spaces are applied, and the service water heating use units are not POWER or VOLUME, then this use applies to all spaces in the building_segment: `if len(spaces) == 0:`
- check if the swh use use_units are not POWER or VOLUME. Service water heating uses with use units power or volume are only counted once, unless they are expicitly assigned to multiple spaces: `if swh_use.use_units not in ["POWER","VOLUME"]: spaces = building_segment.spaces`
Copy link
Collaborator

@JacksonJ-KC JacksonJ-KC Jul 24, 2024

Choose a reason for hiding this comment

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

If no spaces reference the service water heating use and it is specified using POWER or VOLUME it is still representative of all of the spaces right?

My preference would be to exclude logic related to POWER or VOLUME from this function. If no spaces reference the service water heating use, return an empty list. Where this function is called, if an empty list is returned then check there if the units are POWER or VOLUME and take the necessary steps.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is my understanding of how this is currently working:

  1. Spaces reference service water heating use, with units of POWER or VOLUME --> return list of spaces that reference the service water heating use
  2. Spaces reference service water heating use, with units of POWER_PER_AREA --> return list of spaces that reference the service water heating use
  3. Spaces do not reference the service water heating use, with units of POWER or VOLUME --> return empty list
  4. Spaces do not reference the service water heating use, with units of POWER_PER_AREA --> return list of all building segment spaces

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmmm… interesting. My knee-jerk reaction is that this would make it challenging to maintain the encapsulation of potential schema changes that this function and get_SWH_uses_associated_with_each_building_segment are intended to achieve, but I think if we always return an empty list when no spaces are served, this will work. Putting a pin /placeholder in this - I’ll keep this in mind as I go through the rest of your comments

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@JacksonJ-KC - making the change you suggest would break encapsulation. The function get_energy_required_to_heat_swh_use relies on get_spaces_served_by_SWH_use. This function doesn't know which building segment the swh_use is located in. We might be able to pass the building segment to that function, but that makes get_energy_required_to_heat_swh_use unnecessarily more complicated

Copy link
Collaborator

Choose a reason for hiding this comment

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

get_energy_required_to_heat_swh_use is only called from get_SWH_equipment_associated_with_each_SWH_bat. It is called from within a loop through building segments so it will be easy to pass the building segment. If the list returned from get_energy_required_to_heat_swh_use is empty, use all spaces in the building segment as the list. This does not add complication, it significantly simplifies get_spaces_served_by_SWH_use and keeps power and volume out of the function because they are not related to the function's own description:

This function determines the spaces served by a given SWH use. The convention is that if any spaces reference the swh_use, then the service water heating use applies to only those spaces. If no spaces reference the service water heating use, it applies to all spaces in the building segment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

what you're suggesting works much better given our proposed schema changes, and it will work if the schema doesn't change, but in that case it's a bit messier because if the schema stays the same as it is now, SWH_use is a child of space, so a SWH_use will never be applied to zero or multiple spaces, and get_energy_required_to_heat_swh_use has to know something about the structure of how swh_uses are stored.

I've changed it to do as you suggested because I think it is a better solution, but I wanted to highlight the above so you understand my reasoning and so we can make sure these functions will work no matter which way the schema goes.

… into RDS/KJW/function_get_spaces_served_by_SWH_use
@weilixu weilixu merged commit 4e9d0dd into develop Sep 6, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants