RS/YJ/Rule 5-1 #1257
RS/YJ/Rule 5-1 #1257
Conversation
yunjoonjung-PNNL
requested review from
weilixu,
charliepnnl,
JacksonJ-KC and
Jiarongx-Xie
|
@yunjoonjung-PNNL This draft was helpful. I ran the tests with the built-in validation and found a couple issues with the previous test json that I did not catch with the local validation that I was running previously. This test is not in the TCD spreadsheet because it requires multiple RMDs and the spreadsheet method would have added complications. This json was created by copying a snippet of another test json and altering manually. |
Thank you for your quick response and No worries! Please refer to this PR for 5-1 TCD creation and let me know if this rule has any issues. |
|
See #1171 |
| "baseline_list": baseline_list, | ||
| "no_of_rmds": no_of_rmds, | ||
| "no_of_output_instance": no_of_output_instance, | ||
| } |
There was a problem hiding this comment.
After review this rule, I feel that you do not need this create_data function. All info can be retrieved inside BuildingRule
There was a problem hiding this comment.
I think I made a mistake doing find_one("$.output.output_instance", building_p, False) in the BuildingRule level. I should've done this in the RMDRule level. For this reason, I'll keep the RMDRule.
… into RS/YJ/Rule_5-1
Thanks for your hard work throughout this rule! |
… into RS/YJ/Rule_5-1
… into RS/YJ/Rule_5-1
| @@ -82,7 +82,7 @@ | |||
| ] | |||
| } | |||
| ], | |||
| "type": "USER" | |||
| "type": "BASELINE_0" | |||
There was a problem hiding this comment.
@JasonGlazer @JacksonJ-KC Could you one of you reflect this change in the section 5 spreadsheet? I believe when section 5 was renumbered, this part was changed unexpectedly somehow.
There was a problem hiding this comment.
This test json for rule 5-31 does not align with the section 5 spreadsheet - the test_description belong to a different rule as well. The spreadsheet appears to be correct so I just recreated the json file from the spreadsheet.
|
Approved but wait for the json test updates. |
@yunjoonjung-PNNL @weilixu I created a pull request to merge the recreated 5-31 test json into this branch |
recreate 5-31 test json
|
@weilixu Jackson's work was merged into this branch. I believe this PR is ready to be merged into the |
This Rule hasn't been tested yet. I'll work with Jackson to make this rule pass. Thanks!