-
Notifications
You must be signed in to change notification settings - Fork 81
Add SDR options analysis and minimal buildstock.csv through CI #1384
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
Conversation
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.
Pull Request Overview
This PR adds a new CI job to run the SDR options analysis and generate a minimal buildstock.csv while ensuring that every option in the sdr YAML applies to at least one building.
- Adds Git configuration settings for commits triggered by GitHub Actions.
- Introduces a new workflow job ("sdr_options_analysis") to install required dependencies, generate a 550K sample and minimal buildstock CSV, and commit analysis results.
@rajeee: I guess we could pull out the sdr tests and put them in the new job. I like this new job split, because the integration and analysis tests are more about the baseline and code changes, where the sdr job is about upgrades. These jobs are also parallel to each other which is also good. |
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.
Can you remind me about the usefulness of this file? The applicability is in sdr_option_application_report.csv
file. What additional information is in this file? It is long, and I would like it to be useful if we keep it. Is there another form that this could take that is more readable?
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 provides a more detailed x-ray into the apply logic. For example, the .csv file in this line tells you that the option applied to 16.7% of the buildings, which can seem correct. But if you look at the detailed report txt file for this option, you will find that that the "Water Heater Efficiency|Electric Heat Pump, 66 gal, 3.35 UEF" logic had no bearing on this logic since there is not water heater with that specification in the 550K sample. This may or may not be correct/expected, but the report file provides you the information to dig further.
Regarding alternative forms, few ideas come to mind.
-
Split them into one file per upgrade.
Although the file is large, it is purely text so github seems to be able to handle it okay. When a PR is made with changes/addition to just 1 upgrade, the diff view will only show section relevant to that upgrade (for example the diff after you pulled in the EV DF) so it shouldn't be that verbose in such cases. So, I am not too sure if splitting is worthwhile. -
This file would actually be of about the same length as the sdr yaml (minus the comments in the yaml, plus extra filler description in the report txt) if not for a detailed options-interplay-report. This is a more confusing part, and we could maybe remove this, but i think it is useful. For example, from this report, we can tell that 74% of buildings got exactly 3 options applied and 22% got exactly 9 options applied, which serves as a nice verification that options are being applied as intended (like no buildings that got 6 options applied etc).
I understand that this report file feels overwhelming, but once we start getting PR with one upgrade at a time, it should be manageable.
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.
Overall, I think this will work. Since you are pip installing buildstock-query into the CI environment, this is cleaner than I expected. I added some comments, but we can move forward with the development.
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 a changelog entry.
Co-authored-by: Anthony Fontanini <[email protected]>
Pull Request Description
Run the upgrades analyzer to generate the options apply logic report for the sdr yaml. These report helps catch and correct apply_logic errors, and also can be used in documentation to explain exactly how many building models the various pieces of the logic applies to.
In addition, it also generates a minimal version of buildstock.csv file. This file contains the minimal number of buildings but with enough diversity such that all of the options in the sdr yaml will apply to at least one building (when possible*).
All these files are commited to project_national/resources/ folder.
*If the logic applies to at least one building in 550K sample.
Related Pull Requests
[related PRs from different repositories]
Related Issues
[What issue(s) is the PR addressing]
Checklist
Required:
Optional (not all items may apply):
openstudio tasks.rb update_measures
has been run