Repository navigation
Allow MF locations for battery location - #2285
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The focused changes consistently cover location handling, validation, and regression tests, with no blocking issues identified in static review; tests were not executed.
No open issues 🎉
What changed in this PR
Adds multifamily battery locations to the HPXML workflow, addressing #2104 without assigning battery heat gains to the dwelling unit.
Changes:
- Expands accepted locations and builder options.
- Treats multifamily battery locations as outside for thermal modeling.
- Updates regression tests, samples, documentation, and measure metadata.
| File | Description |
|---|---|
| workflow/sample_files/base-bldgtype-mf-unit-shared-pv-battery.xml | Uses other heated space. |
| workflow/hpxml_inputs.json | Selects the matching battery option. |
| HPXMLtoOpenStudio/tests/test_validation.rb | Tests rejection for incompatible building types. |
| HPXMLtoOpenStudio/tests/test_battery.rb | Tests multifamily locations and thermal behavior. |
| HPXMLtoOpenStudio/resources/hpxml_schematron/EPvalidator.sch | Allows multifamily battery locations. |
| HPXMLtoOpenStudio/resources/battery.rb | Excludes these locations from dwelling heat gains. |
| HPXMLtoOpenStudio/measure.xml | Refreshes translator metadata and checksums. |
| docs/source/workflow_inputs.rst | Documents supported locations. |
| Changelog.md | Announces multifamily battery support. |
| BuildResidentialHPXML/resources/options/battery.tsv | Adds multifamily battery choices. |
| BuildResidentialHPXML/README.md | Lists new builder choices. |
| BuildResidentialHPXML/measure.xml | Updates choices, metadata, and checksums. |
Review effort: Balanced
…ss, fixes a bug where belly and wing timeseries temperatures were missing.)
There was a problem hiding this comment.
🟢 Approval recommended
The validation, modeling, options, and focused test changes consistently implement the requested behavior, with no blocking issues identified in static review; tests were not executed.
No open issues 🎉
Review effort: Balanced
shorowit
left a comment
shorowit
left a comment
There was a problem hiding this comment.
This looks great to me. While reviewing your code, it made me realize that some light refactoring would really clean things up, so I committed that change. CI passed and there were no diffs, so that looks good. I'm going to make one more minor change below, but otherwise this is good to go. Thanks, @joseph-robertson.
| assert_equal(1, batteries.size) | ||
| battery = batteries[0] | ||
| assert(!battery.thermalZone.is_initialized) | ||
| assert_equal(0, battery.radiativeFraction) |
| 'Expected only SystemIdentifier to be specified when sameas attribute used'], | ||
| 'multifamily-common-space-whole-sfa-or-mf-building-sim-false' => ['Expected ExteriorAdjacentTo'], | ||
| 'multifamily-reference-appliance' => ['There are references to "other housing unit" but ResidentialFacilityType is not "single-family attached" or "apartment unit".'], | ||
| 'multifamily-reference-battery' => ['There are references to "other heated space" but ResidentialFacilityType is not "single-family attached" or "apartment unit".'], |
Pull Request Description
Closes #2104.
Checklist
Not all may apply:
EPvalidator.sch) has been updatedopenstudio tasks.rb update_hpxmls)HPXMLtoOpenStudio/tests/test*.rband/orworkflow/tests/test*.rb)openstudio tasks.rb update_measureshas been run