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

RTD: summarize lookup arguments #1146

Merged
merged 55 commits into from
Nov 27, 2023
Merged

RTD: summarize lookup arguments #1146

merged 55 commits into from
Nov 27, 2023

Conversation

joseph-robertson
Copy link
Contributor

@joseph-robertson joseph-robertson commented Oct 12, 2023

Pull Request Description

This PR:

  • Adds remaining arguments (i.e., <arg_name>=auto) to options_lookup.tsv so that every ResStockArguments argument is included.
  • Adds a unit test to ensure that all ResStock arguments are called out in options_lookup.tsv.
  • By housing characteristic parameter, automates summarizing Arguments sets used in options_lookup.tsv (including Name, Required, Units, Type, Choices, Description). See below for example.
  • Adds a README.md.erb to all ResStock measures so that README.md documentation is automatically generated.
  • Adds introduction in Housing Characteristics for describing subsections and the "auto" argument choice. See below.

image


image

Checklist

Not all may apply:

  • Tests (and test files) have been updated
  • Documentation has been updated
  • Changelog has been updated
  • openstudio tasks.rb update_measures has been run
  • No unexpected regression test changes on CI (checked comparison artifacts)

@joseph-robertson joseph-robertson self-assigned this Oct 12, 2023
@joseph-robertson joseph-robertson changed the title RTD: summary lookup arguments RTD: summarize lookup arguments Oct 12, 2023
Base automatically changed from minor-issues to develop October 13, 2023 20:23
@shorowit
Copy link
Contributor

shorowit commented Oct 18, 2023

I realize you've already gone down a certain path and I'm coming to it late, but I can't help but wonder if it would be simpler to just include every argument in the options_lookup.tsv somewhere. Could even write a test to make sure they are all covered.

@shorowit
Copy link
Contributor

Related: NREL/OpenStudio-HPXML#1518

@joseph-robertson joseph-robertson marked this pull request as ready for review October 26, 2023 23:58
@joseph-robertson
Copy link
Contributor Author

joseph-robertson commented Nov 6, 2023

We need to better orient readers of the Housing Characteristics page to the idea of "auto" and what that means, how things connect to OS-HPXML documentation and HPXML fields, etc.

We should have a paragraph up front on "auto", and then for each ResStock parameter we should have link(s) to the related OS-HPXML documentation sections (e.g., HVAC Heating Efficiency -> HPXML Heating Systems, HPXML Heat Pumps).

Edit: related to NREL/OpenStudio-HPXML#1252.

joseph-robertson and others added 16 commits November 6, 2023 19:38
…4f5ab

e4788c4f5ab Update build measure argument descriptions with links to docs.
02207f7a83c Merge pull request #1540 from NREL/deprecation-msgs
b251c34b737 Adds error messages for all extension elements that have been moved/deprecated since v1.0, so that the input is not silently ignored. No need to do this for non-extension elements since the XSD Schema will report errors for these situations.
5889de415f7 Update unit tests.
0f813a83fe1 Hotfix for recirc energy when multiple water heaters.
51f9d2fde58 Merge pull request #1537 from NREL/ground_temperatures2
c45fa9de0fb Merge branch 'master' of https://github.com/NREL/OpenStudio-HPXML into ground_temperatures2
6598ab6059d Merge pull request #1539 from NREL/fix_system_use
053e9130f24 Fix CI failures.
54280437497 Run update_measures.
fb2bf8fb803 Merge branch 'ground_temperatures2' of github.com:NREL/OpenStudio-HPXML into ground_temperatures2
4063887f5d0 Accept nearest station in xing lookup.
9d9ad3647d7 Add comment [ci skip]
1043ac2d1b9 Fix comment [ci skip]
2b0d89bf4ba Fix possible incorrect value for system use, bug introduced in #1456. Adds error-checking to prevent this going forward. Some other minor misc cleanup.
d4ed83b6688 Merge branch 'ground_temperatures2' of github.com:NREL/OpenStudio-HPXML into ground_temperatures2
54d28d2ce6b Skip deep ground temp calcs if no gshp.
b00bb924031 Add test for defaulting xing values.
3a783ad4a25 Respond to review.
23f6e542ea5 Latest results.
71ac84dff44 Merge branch 'master' into ground_temperatures2
98ace8d4ac0 Update the changelog. [ci skip]
9cd53e49d11 Merge pull request #1538 from NREL/misc_cleanup
e24568fd749 Latest results.
951dc933449 Misc cleanup.
092d4981415 Latest results.
3370a534320 Convert new csv to utf8.
cde1f9aaab2 Get the units right.
27148f692cf Missed a return.
533d2926d78 Update the changelog.
4a6174d571f Assign deep ground variables and use throughout.
1fbc7087565 Add deep temperature lookup based on xing model.
6c91ec33ac5 Revert shallow and deep ground temperature conflation.
4127a5dcb4a Merge pull request #1536 from NREL/split_workflow_tests
00b8d21b992 Remove old files. [ci skip]
fd88b9696ce Latest results.
004c05172b3 Fix CI failure
d2a87c7e3aa Update pull_request_template.md [ci skip]
ab259ce187a Split workflow tests, so they can be parallelized on the CI for faster turnaround time.

git-subtree-dir: resources/hpxml-measures
git-subtree-split: e4788c4f5ab76ae809a1a008a97d6fe85889396e
…ca1ee

2cd420ca1ee Few more href updates.
445d7a40a43 Replace all measure descriptions with href.
a2fcdf2c809 Update readme erb to convert href to markdown.
1c6fd3dce40 Try an argument description with href.

git-subtree-dir: resources/hpxml-measures
git-subtree-split: 2cd420ca1ee40655527889d048afa833f2be67d2
…f2692

111190f2692 Fix a few typos.

git-subtree-dir: resources/hpxml-measures
git-subtree-split: 111190f2692bca07a219d40e1e175e8365ec1aa8
- **Name**
- **Units**
- **Choices**
- **Description**
Copy link
Contributor Author

@joseph-robertson joseph-robertson Nov 9, 2023

Choose a reason for hiding this comment

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

Look at BuildResHPXML/measure.xml to get: either (A) Choices gets updated with "auto" and "type" or (B) Type, and then (C) Required (Yes or No, so that we can populate "auto" when it's not a choice argument). Optional here is to populate "true" and "false" in the Choices column for bool arguments.

@afontani

Copy link
Contributor Author

@joseph-robertson joseph-robertson Nov 10, 2023

Choose a reason for hiding this comment

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

I'm writing the following columns to the Arguments table, in the following order:

  • Name
  • Required (comes from BuildResidentialHPXML)
  • Units
  • Type (comes from BuildResidentialHPXML)
  • Choices
    • all optional choice arguments have "auto" in addition to all other choices
    • all optional boolean arguments have "auto", "true", "false"
    • add "auto" choice if "OS-HPXML default" string in the Description and Type is string/double/integer
  • Description

@@ -46,4 +112,46 @@ def to_underscore_case
write_subsection(f, row, 'Created by', '*')
write_subsection(f, row, 'Source', '*')
write_subsection(f, row, 'Assumption', '*')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Assumptions? Handle this on the resstock-estimation side. These should probably not be hard-coded here.

@joseph-robertson
Copy link
Contributor Author

Need to either (A) wait for source_report formatting updates, or (B) fix some of the list item indentation issues.

@joseph-robertson joseph-robertson merged commit 6f477c0 into develop Nov 27, 2023
7 checks passed
@joseph-robertson joseph-robertson deleted the arg-summary branch November 27, 2023 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants