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

RS/YJ/Rule 11-12 (include only necessary codes) #1554

Merged
merged 11 commits into from
Nov 20, 2024

Conversation

yunjoonjung-PNNL
Copy link
Collaborator

Based on PR #1543 comments, this PR only includes the codes related to rule 11-12. Thanks!

Copy link
Collaborator

@weilixu weilixu left a comment

Choose a reason for hiding this comment

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

i am a little concern on changing the get_swh_uses_associated_with_each_building_segment returning the building_seg_id: sum_of_swh_use_value key value pair as oppose to the RDS description: building_seg_id: list of swh_use object
However, if no other rule requires returning a list of swh_use objects, I am fine with this implementation.

@Jiarongx-Xie
Copy link
Collaborator

Jiarongx-Xie commented Nov 12, 2024

i am a little concern on changing the get_swh_uses_associated_with_each_building_segment returning the building_seg_id: sum_of_swh_use_value key value pair as oppose to the RDS description: building_seg_id: list of swh_use object However, if no other rule requires returning a list of swh_use objects, I am fine with this implementation.

This function is required by multiple rules. I think we may need to return the swh_use objects or their ids, an example is 11-15
Screenshot 2024-11-12 132549

@yunjoonjung-PNNL
Copy link
Collaborator Author

i am a little concern on changing the get_swh_uses_associated_with_each_building_segment returning the building_seg_id: sum_of_swh_use_value key value pair as oppose to the RDS description: building_seg_id: list of swh_use object However, if no other rule requires returning a list of swh_use objects, I am fine with this implementation.

This function is required by multiple rules. I think we may need to return the swh_use objects or their ids, an example is 11-15 Screenshot 2024-11-12 132549

As @Jiarongx-Xie suggested, I changed the get_swh_uses_associated_with_each_building_segment to return the service_water_heating_uses object. Good point Jiarong!

Copy link
Collaborator

@weilixu weilixu left a comment

Choose a reason for hiding this comment

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

Great work Yun Joon

Copy link
Collaborator

@Jiarongx-Xie Jiarongx-Xie left a comment

Choose a reason for hiding this comment

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

Great work, thank you Yun Joon!

@weilixu weilixu merged commit 4668dd1 into develop Nov 20, 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.

3 participants