-
Notifications
You must be signed in to change notification settings - Fork 5
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
Rds/kjw/rule 11 3 #1337
base: master
Are you sure you want to change the base?
Rds/kjw/rule 11 3 #1337
Conversation
I see commits in this pull request "Update 11-1.md". Were any of those changes intended to be incorporated into this branch/PR? |
no - I was using PyCharm & that was what was filled in from the previous
commit on the other branch.
…On Wed, May 8, 2024 at 1:58 PM Jackson Jarboe ***@***.***> wrote:
I see commits in this pull request "Update 11-1.md". Were any of those
changes intended to be incorporated into this branch/PR
<#843>?
—
Reply to this email directly, view it on GitHub
<#1337 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A3G3CLCQN7TQTD7AJR34SM3ZBJRWDAVCNFSM6AAAAABHARNRW2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBRGEYTIMZRGA>
.
You are receiving this because you were assigned.Message ID:
***@***.***>
|
I also use PyCharm. Let me know if you would like any tips to avoid these things. |
|
||
**Rule Assertion:** Options are PASS/FAIL | ||
|
||
**Appendix G Section Reference:** Table G3.1 #11, proposed column, c |
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.
Table G3.1(11) Proposed Building Performance (c)
docs/section11/11-3.md
Outdated
|
||
**Appendix G Section Reference:** Table G3.1 #11, proposed column, c | ||
|
||
**Evaluation Context:** P-RMD each SHW type |
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.
It seems weird to evaluate this for each SWH Building Area Type. I would expect to either a single evaluation for the whole project or have it be evaluated per SWH system. If there is a requirement for 1 baseline SWH system per BAT then that seems like it would be its own rule, not incorporated here.
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.
It may be better to do by ServiceWaterHeatingDistributionSystem since this would encompass a SHW system as opposed to SHWHeatingEq where there could be mulitple SHWHeatingEq IDs per system. So it would be {"ServiceWaterHeatingDistributionSystem":{"SHWHeatingEq":["shwe1","shwe2"], "Building area types served":["Dormitory","Multifamlily"], "Pumps":["p1"], "Tanks":["t1"], "Piping":["piping1"], "SolarThermal":[], "Service_water_heating_uses":["swhuse1","shwuse2"]}} so all this information would be returned for each SHWHeatingEq ID.
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.
this is now done by SWH distribution system
Unintended commit to .gitignore? |
docs/section11/11-3.md
Outdated
- create the compare context string: `compare_context_str = "AppG 11-1 P_RMD Equals B_RMD"` | ||
- get a dictionary of all SHW equipment connected to the SHW space type in the proposed model: `p_shw_equipment_dict = get_SHW_equipment_connected_to_use_type(P_RMD)[shw_space_type]` | ||
- get a dictionary of all SHW equipment connected to the SHW space type in the user model: `b_shw_equipment_dict = get_SHW_equipment_connected_to_use_type(B_RMD)[shw_space_type]` | ||
- create a list of all of the SHW equipment types that exist in both the proposed and user models: `shw_equipment_types = set(p_shw_equipment_dict.keys()) | set(b_shw_equipment_dict.keys())` |
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 am not sure what the | is doing exactly. Can you confirm this is creating a list of the the unique keys included across the two dictionaries?
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.
Need to correct the description
docs/section11/11-3.md
Outdated
- create an error string: `error_str = ""` | ||
- create the compare context string: `compare_context_str = "AppG 11-1 P_RMD Equals B_RMD"` | ||
- get a dictionary of all SHW equipment connected to the SHW space type in the proposed model: `p_shw_equipment_dict = get_SHW_equipment_connected_to_use_type(P_RMD)[shw_space_type]` | ||
- get a dictionary of all SHW equipment connected to the SHW space type in the user model: `b_shw_equipment_dict = get_SHW_equipment_connected_to_use_type(B_RMD)[shw_space_type]` |
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.
says user model, correct to baseline
docs/section11/11-3.md
Outdated
- create a list of all of the SHW equipment types that exist in both the proposed and user models: `shw_equipment_types = set(p_shw_equipment_dict.keys()) | set(b_shw_equipment_dict.keys())` | ||
- loop through each of the equipment types: `for shw_equipment_type in shw_equipment_types:` | ||
- continue if all_match is still true: `if all_match:` | ||
- check if this equipment type matches between the proposed and user models. First check whether there are the same number equipment: `if len(p_shw_equipment_dict[shw_equipment_type]) == len(b_shw_equipment_dict[shw_equipment_type]):` |
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.
Need to correct the description
docs/section11/11-3.md
Outdated
- loop through each of the equipment types: `for shw_equipment_type in shw_equipment_types:` | ||
- continue if all_match is still true: `if all_match:` | ||
- check if this equipment type matches between the proposed and user models. First check whether there are the same number equipment: `if len(p_shw_equipment_dict[shw_equipment_type]) == len(b_shw_equipment_dict[shw_equipment_type]):` | ||
- look at each equipment type system in the proposed model and see if there is one that is the same in the user model. This check relies on the understanding that the RCT team has a method for comparing all elements within an object match (compare_context_pair): `for p_SHW_equip_id in p_shw_equipment_dict[shw_equipment_type]:` |
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.
Need to correct the description
docs/section11/11-3.md
Outdated
- continue if all_match is still true: `if all_match:` | ||
- check if this equipment type matches between the proposed and user models. First check whether there are the same number equipment: `if len(p_shw_equipment_dict[shw_equipment_type]) == len(b_shw_equipment_dict[shw_equipment_type]):` | ||
- look at each equipment type system in the proposed model and see if there is one that is the same in the user model. This check relies on the understanding that the RCT team has a method for comparing all elements within an object match (compare_context_pair): `for p_SHW_equip_id in p_shw_equipment_dict[shw_equipment_type]:` | ||
- if this system is in U_RMD.service_water_heating_distribution_systems, compare the systems: `if p_SHW_equip_id in b_shw_equipment_dict[shw_equipment_type]:` |
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.
Need to correct the description
docs/section11/11-3.md
Outdated
- check if this equipment type matches between the proposed and user models. First check whether there are the same number equipment: `if len(p_shw_equipment_dict[shw_equipment_type]) == len(b_shw_equipment_dict[shw_equipment_type]):` | ||
- look at each equipment type system in the proposed model and see if there is one that is the same in the user model. This check relies on the understanding that the RCT team has a method for comparing all elements within an object match (compare_context_pair): `for p_SHW_equip_id in p_shw_equipment_dict[shw_equipment_type]:` | ||
- if this system is in U_RMD.service_water_heating_distribution_systems, compare the systems: `if p_SHW_equip_id in b_shw_equipment_dict[shw_equipment_type]:` | ||
- use compare_context_pair to compare systems and set all_match to FALSE if the systems don't compare: `if !compare_context_pair(p_SHW_equip_id, p_SHW_equip_id, $, extra_schema_for_SHW_comparison.json, true, compare_context_str, error_str): all_match = FALSE` |
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.
p_SHW_equip_id is input twice, second should be _b
docs/section11/11-3.md
Outdated
**Function Call:** | ||
- **get_component_by_id** | ||
- **compare_context_pair** - there is no RDS for this function, but it is a function developed for Rule 1-6 that compares two elements | ||
- **get_SHW_types_and_spaces** |
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 this funciton is now called get_spaces_associated_with_each_SWH_bat
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.
removed reference as the function is no longer used. Updated all function references so they're current.
docs/section11/11-3.md
Outdated
- **get_component_by_id** | ||
- **compare_context_pair** - there is no RDS for this function, but it is a function developed for Rule 1-6 that compares two elements | ||
- **get_SHW_types_and_spaces** | ||
- **get_SHW_equipment_connected_to_use_type** |
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 this function also has a different name now
docs/section11/11-3.md
Outdated
- **get_SHW_equipment_connected_to_use_type** | ||
|
||
**Applicability Checks:** | ||
- check that the SHW type in the P_RMD has SHW loads |
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.
Recommend modifying language here for clarity. I think you mean "check that the SHW BAT in the P_RMD has SHW loads"?
- get the distribution system: `p_distribution = get_component_by_id(P_RMD, distribution_id)` | ||
- for each swh use in the dictionary, check to see that the swh use exists in the user model: `for swh_use_id in p_swh_system_and_equip_dict[distribution_id]["SPACE_USES"]:` | ||
- get the user swh_use: `u_swh_use = get_component_by_id(U_RMD, swh_use_id)` | ||
- if the u_swh_use exists, this rule is not applicable and 11-1 applies: `if(u_swh_use): NOT_APPLICABLE` |
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 suggest that we check equipment instead of the uses in the user model. Because some could model the uses when there is no equipment designed, right? I think this rule applies when there will be SHW loads in the user model but no system has been designed. I am concerned by looking at uses instead of User model equipment we maybe skip something when it should not be skipped.
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 see your concern, however the architecture of the schema makes a ServiceWaterHeatingUse without a distribution_system problematic. On the last call (5/29/2024), we discussed (and agreed to) making ServiceWaterHeatingUse.served_by_distribution_system
a required field, which would allow this logic.
- **compare_context_pair** - there is no RDS for this function, but it is a function developed for Rule 1-6 that compares two elements | ||
- **get_SHW_types_and_spaces** | ||
- **get_SHW_equipment_connected_to_use_type** | ||
|
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.
add get_SWH_equipment_associated_with_each_swh_distribution_system
docs/section11/11-3.md
Outdated
- look at each equipment type system in the proposed model and see if there is one that is the same in the baseline model. This check relies on the understanding that the RCT team has a method for comparing all elements within an object match (compare_context_pair): `for p_SHW_equip_id in p_shw_equipment_dict[shw_equipment_type]:` | ||
- if this system is in B_RMD.service_water_heating_distribution_systems, compare the systems: `if p_SHW_equip_id in b_shw_equipment_dict[shw_equipment_type]:` | ||
- use compare_context_pair to compare systems and set all_match to FALSE if the systems don't compare: `if !compare_context_pair(p_SHW_equip_id, b_SHW_equip_id, $, extra_schema_for_SHW_comparison.json, true, compare_context_str, error_str): all_match = FALSE` | ||
- otherwise, set all_match to false: `else: all_match = FALSE` |
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 do not know where to see this function but trust that this works. Maybe Weili should check out this line.
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, would like @weilixu to confirm that the recursive nature of the compare_context_pair function has been communicated properly in this RDS
docs/section11/11-3.md
Outdated
|
||
**Appendix G Section Reference:** Table G3.1 #11, proposed column, c | ||
|
||
**Evaluation Context:** P-RMD each SHW bat |
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.
Evaluated for each ServiceWaterHeatingDistributionSystem?
- get the distribution system: `p_distribution = get_component_by_id(P_RMD, distribution_id)` | ||
- for each swh use in the dictionary, check to see that the swh use exists in the user model: `for swh_use_id in p_swh_system_and_equip_dict[distribution_id]["SPACE_USES"]:` | ||
- get the user swh_use: `u_swh_use = get_component_by_id(U_RMD, swh_use_id)` | ||
- if the u_swh_use exists, this rule is not applicable and 11-1 applies: `if(u_swh_use): NOT_APPLICABLE` |
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.
This makes the rule not applicable if even 1 ServiceWaterHeatingUse is defined in the User model. What is supposed to happen if there are just certain spaces where the ServiceWaterHeatingUse is not defined?
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.
if this rule is not applicable, rule 11-1 is applicable. This logic just checks whether the SWH use that exists in the proposed model exists in the user model. It allows for spaces without SWH uses
docs/section11/11-3.md
Outdated
|
||
## Rule Assertion: | ||
- Case1: all elements are equal: PASS: `if all_match: PASS` | ||
- Case2: all elements don't match, FAIL: if !all_match: FAIL` |
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.
missing ` prior to pseudo code
docs/section11/11-3.md
Outdated
- create the compare context string: `compare_context_str = "AppG 11-1 P_RMD Equals B_RMD"` | ||
- get a dictionary of all SHW equipment connected to the SHW distribution system in the proposed model: `p_shw_equipment_dict = get_SWH_equipment_associated_with_each_swh_distribution_system(P_RMD)[distribution_id]` | ||
- get a dictionary of all SHW equipment connected to the SHW distribution system in the baseline model: `b_shw_equipment_dict = get_SWH_equipment_associated_with_each_swh_distribution_system(B_RMD)[distribution_id]` | ||
- check whether b_shw_equipment_dict is equal to null. If it is, then set all_match is false and give the user an error indictating that the system doesn't exist in the baseline: `if b_shw_equipment_dict == NULL: all_match = FALSE; error_str = "The SHW Distribution system " + distribution_id + " is not found in the baseline model." |
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.
missing ` after pseudo code
docs/section11/11-3.md
Outdated
@@ -0,0 +1,61 @@ | |||
# Service_Water_Heating - Rule 11-3 | |||
**Schema Version:** 0.0.36 |
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.
Schema is currently on 0.0.37. Not sure if any of the updates impact this RDS.
- next, check Pumps - this will also recursively check PumpOutputValidationPointPumpOutputValidationPoint: `if len(p_swh_equipment_dict["Pumps"]) == len(b_swh_equipment_dict["Pumps"]):` | ||
- look at each SWHEquipment in the proposed model: `for pump_id in p_swh_equipment_dict["Pumps"]:` | ||
- compare the two pumps using compare_context_pair, if the result is false, set all_match equal to false. We won't exit early if all_match is false as we allow the function to keep running so errors is fully populated and available to the user: `if !compare_context_pair(pump_id, pump_id, $, extra_schema_for_SWH_comparison.json, true, compare_context_str, error_str): all_match = false` | ||
|
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.
Defer to Weili due to complex, novel structure.
No description provided.