BuildResidentialHPXML: make conditional arguments optional #1867

joseph-robertson · 2024-10-23T20:52:30Z

Pull Request Description

Change any required arguments that are ignored based on other arguments, to optional.

Remove setDefaultValue from all OpenStudio arguments. Required arguments must now pass a value.

Create a make_argument wrapper for passing in argument name, type, required, etc and determining/creating appropriate OpenStudio arguments automatically:

if there is OS-HPXML default (must be optional argument):
- choice: append "auto" choice
- boolean: create a choice argument with choices "true", "false", "auto"
- double/integer: create a string argument ("auto" can be passed in)
arguments with OS-HPXML default gets description about OS-HPXML defaults and href appended

Create convertArgumentValues method called from beginning of run method:

delete arguments that passed in "auto"
"true" to true, "false" to false, "10" to 10, "10.5" to 10.5, etc

Checklist

Not all may apply:

~~Schematron validator (EPvalidator.xml) has been updated~~
~~Sample files have been added/updated (openstudio tasks.rb update_hpxmls)~~
Tests have been added/updated (e.g., HPXMLtoOpenStudio/tests/test*.rb and/or workflow/tests/test*.rb)
~~Documentation has been updated~~
Changelog has been updated
openstudio tasks.rb update_measures has been run
No unexpected changes to simulation results of sample files

…n up hpxml_inputs.json.

…o optional-build-args

…o the other window area arguments.

shorowit

LGTM. Will wait on ResStock testing before merging.

joseph-robertson · 2024-10-30T03:19:02Z

Should all optional arguments that use the OS-HPXML default when not provided have a default argument value set? For example, site_type, site_shielding_of_home, site_soil_and_moisture_type use OS-HPXML defaults but don't set a default argument value. Arguments site_iecc_zone, site_city don't set a default argument value either, but don't use OS-HPXML defaults.

The implication of not doing this, as I'm noticing in ResStock, is that to allow "auto" I'd need to check for presence of "OS-HPXML default" in the argument description vs. checking that a default argument value is set.

Edit: Including a default argument value for every argument with description "OS-HPXML default" probably wouldn't work. For example, it makes sense to have the clothes_dryer_fuel_type argument default to "natural gas", but "auto" shouldn't be a valid ResStock argument value.

joseph-robertson · 2024-10-30T04:25:33Z

Do arguments ducts_supply_buried_insulation_level and ducts_return_buried_insulation_level need updated argument descriptions using "OS-HPXML default" pointing to https://openstudio-hpxml.readthedocs.io/en/latest/workflow_inputs.html#air-distribution?

…L into optional-build-args

…efaults.

… default is used.

…n be comma-separated.

shorowit · 2024-11-19T16:01:49Z

HPXMLtoOpenStudio/resources/meta_measure.rb

+      begin
+        value = Float(value)
+        if value % 1 == 0
+          args[name] = Integer(value)
+        else
+          args[name] = value
+        end
+      rescue
+      end


This is a bad idea. You should only be converting to integer/float for arguments that we've declared as integer/float, not simply converting anytime it's possible. I assume this is the reason that you then had to change a bunch of code so that it converts back to string.

You'll need to refactor the code so that you have access to the original desired argument type and use it here.

Also, should this really be in meta_measure.rb? Do you anticipate using this anywhere else than the BuildResHPXML measure?

I put it here because I found that ResStock needs to use it (with delete_auto = false).

BuildResidentialHPXML/README.md

Changelog.md

shorowit · 2024-11-20T14:40:25Z

HPXMLtoOpenStudio/resources/meta_measure.rb

@@ -624,3 +624,38 @@ def is_integer?
    return true
  end
 end
+
+# TODO


Do this now (and anywhere else you added TODOs).

shorowit · 2024-11-20T14:40:57Z

HPXMLtoOpenStudio/resources/meta_measure.rb

+      begin
+        value = Float(value)
+        if value % 1 == 0
+          args[name] = Integer(value)
+        else
+          args[name] = value
+        end
+      rescue
+      end


Also, should this really be in meta_measure.rb? Do you anticipate using this anywhere else than the BuildResHPXML measure?

workflow/sample_files/base-auto.xml

shorowit · 2024-11-20T14:44:25Z

workflow/template-build-and-run-hpxml-with-stochastic-occupancy.osw

-        "cooling_system_cooling_capacity": 24000.0,
+        "cooling_system_cooling_capacity": "24000.0",


Ugh, really? This might be a deal breaker. This is totally confusing and no one will be able to keep track of when they need a number vs a string. I didn't realize OpenStudio was going to require this.

Maybe we can update OpenStudio so that it allows this?

shorowit · 2024-11-20T14:47:59Z

BuildResidentialHPXML/measure.rb

+    # these were previously required arguments with default values set (i.e., arg.setDefaultValue(xxx))
+    args[:floor_over_foundation_assembly_r] = 28.1 if args[:floor_over_foundation_assembly_r].nil?
+    args[:floor_over_garage_assembly_r] = 28.1 if args[:floor_over_garage_assembly_r].nil?


I was really hoping we wouldn't need these (totally arbitrary) defaults. Why?

But if for some reason we do need them, this is definitely a step back from what we had before. Previously, at least a user opening the measure in the OS App or PAT would see the default value. Now they happen completely behind the scenes.

Some, yes, are totally arbitrary. I can fix/refactor those.

Others, for example heat pump backup fuel, are not. We can report the default value in the argument description (there's already precedent for this) and set it in its respective set_xxx method.

Why does heat pump backup fuel need a default?

Change several arguments from required to optional, and start to clea…

719024e

…n up hpxml_inputs.json.

joseph-robertson self-assigned this Oct 23, 2024

joseph-robertson added 3 commits October 23, 2024 20:47

Merge branch 'master' into optional-build-args

db5a8d7

Remove ignored arguments from build measure test file.

ef2e09f

Remove ignored arguments from template osw file.

2cba857

joseph-robertson marked this pull request as ready for review October 24, 2024 18:10

joseph-robertson requested a review from shorowit October 24, 2024 18:10

shorowit added 2 commits October 25, 2024 09:41

Merge branch 'master' of https://github.com/NREL/OpenStudio-HPXML int…

c80ce93

…o optional-build-args

More optional arguments. Remove WWR arguments; fold functionality int…

30dcd87

…o the other window area arguments.

shorowit approved these changes Oct 25, 2024

View reviewed changes

shorowit added 2 commits October 25, 2024 11:12

Update argument descriptions [ci skip]

ce9220d

Unrelated minor code cleanup.

ac06a7c

joseph-robertson mentioned this pull request Oct 28, 2024

Assign <blank> instead of, e.g., 0 to mean unused NREL/resstock#1316

Draft

10 tasks

shorowit added this to the 1.9.0 milestone Oct 29, 2024

joseph-robertson and others added 14 commits October 30, 2024 10:44

Merge branch 'master' into optional-build-args

ef3dde2

Merge branch 'optional-build-args' of github.com:NREL/OpenStudio-HPXM…

06f6253

…L into optional-build-args

Typo in hvac_perf_data_capacity_type argument.

5075a60

Stub generic makeArgument method.

c07ebb2

Use named arguments and reformat, introduce default_href.

54c542e

Try logic for adding auto choice for bool and choice arguments with d…

62e6fc8

…efaults.

Merge branch 'master' into optional-build-args

7f136a2

Handle double and integer arguments to string arguments when OS-HPXML…

0b5478d

… default is used.

Reorganize a bit.

5efc333

Merge branch 'master' into optional-build-args

ec72127

Method for converting string arguments back to appropriate data type.

00129ab

Handle and test what happens when auto is provided.

bc1232c

Few more spots where a string is expected for argument values that ca…

18716e8

…n be comma-separated.

Latest results.

6a64ad1

joseph-robertson added 14 commits November 1, 2024 17:34

Progress.

5ac7252

Progress.

775eb16

Sweep through.

1b60e91

Clean up.

dba3891

Start method for defaulting optional argument values.

4790ba2

More entries to defaultOptionalArgumentValues.

14a4896

Try a few more auto arguments in sample file.

9fc0362

Defaults for optional garage arguments.

ca1885e

More default values for previously required arguments.

2f31f5b

Make convertArguments generic so we can use in resstock.

6e3bb51

More default values for previously required arguments.

2f4903e

Update template osw file.

b03fd83

Check zip code for lead zero.

1ae24ad

Fix integer conversion in new method.

103a820

joseph-robertson requested a review from shorowit November 8, 2024 21:50

shorowit modified the milestones: 1.9.0, 1.10.0 Nov 13, 2024

shorowit reviewed Nov 19, 2024

View reviewed changes

shorowit reviewed Nov 20, 2024

View reviewed changes

joseph-robertson added 6 commits November 20, 2024 13:06

Merge branch 'master' into optional-build-args

487ed0e

Respond to review comments.

20ab04f

Merge branch 'master' into optional-build-args

abd5407

Update argument descriptions, defaults, tests, etc.

a763f64

Change all build osw values to strings.

bbad3f7

Merge branch 'master' into optional-build-args

5d9edca

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BuildResidentialHPXML: make conditional arguments optional #1867

BuildResidentialHPXML: make conditional arguments optional #1867

joseph-robertson commented Oct 23, 2024 •

edited

Loading

shorowit left a comment

joseph-robertson commented Oct 30, 2024 •

edited

Loading

joseph-robertson commented Oct 30, 2024 •

edited

Loading

shorowit Nov 19, 2024

shorowit Nov 20, 2024

joseph-robertson Nov 20, 2024

shorowit Nov 20, 2024

shorowit Nov 20, 2024

shorowit Nov 20, 2024

shorowit Nov 20, 2024

shorowit Nov 20, 2024

joseph-robertson Nov 20, 2024

shorowit Nov 20, 2024

		"cooling_system_cooling_capacity": 24000.0,
		"cooling_system_cooling_capacity": "24000.0",

BuildResidentialHPXML: make conditional arguments optional #1867

Are you sure you want to change the base?

BuildResidentialHPXML: make conditional arguments optional #1867

Conversation

joseph-robertson commented Oct 23, 2024 • edited Loading

Pull Request Description

Checklist

shorowit left a comment

Choose a reason for hiding this comment

joseph-robertson commented Oct 30, 2024 • edited Loading

joseph-robertson commented Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joseph-robertson commented Oct 23, 2024 •

edited

Loading

joseph-robertson commented Oct 30, 2024 •

edited

Loading

joseph-robertson commented Oct 30, 2024 •

edited

Loading