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

Roofing material restructure #426

Merged
merged 9 commits into from
Apr 1, 2020
Merged

Conversation

joseph-robertson
Copy link
Contributor

@joseph-robertson joseph-robertson commented Mar 17, 2020

Pull Request Description

Previously, we had:

  • No "None" option for Insulation Unfinished Attic.tsv
  • Roofing Material.tsv assigned roofing_material arguments for both ResidentialConstructionsUnfinishedAttic and ResidentialConstructionsFinishedRoof

The issue with the previous is that if we try to upgrade just roofing_material, measure ResidentialConstructionsFinishedRoof is applied without all measure arguments specified. By adding the building type dependency to Insulation Unfinished Attic.tsv, Insulation Finished Roof.tsv, and both Roofing Material.tsv files, we can avoid this issue.

Other possible solutions are:

  • Add "None" option for Insulation Unfinished Attic.tsv and have each "None" option correspond to dummy values
  • Remove the Roofing Material.tsv file and absorb its options into Insulation Unfinished Attic.tsv and Insulation Finished Roof.tsv

Checklist

Not all may apply:

  • Unit tests have been added or updated
  • All rake tasks have been run, and pass
  • Documentation has been modified appropriately
  • Any new options are added to project_testing
  • project_testing runs without any failures
  • No unexpected regression test changes
  • All tests are passing (green) on circleci
  • The changelog has been updated appropriately
  • This branch is up-to-date with master

For more information on how to perform these checklist items, see the documentation's Advanced Tutorial.

@joseph-robertson joseph-robertson self-assigned this Mar 17, 2020
@joseph-robertson joseph-robertson marked this pull request as ready for review March 18, 2020 00:00
Copy link
Contributor

@afontani afontani left a comment

Choose a reason for hiding this comment

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

@joseph-robertson I think it makes sense to split out the housing characteristics. Looks good to me.

Comment on lines 359 to 360
"weekday_sch": "0.035, 0.033, 0.032, 0.031, 0.032, 0.033, 0.037, 0.042, 0.043, 0.043, 0.043, 0.044, 0.045, 0.045, 0.044, 0.046, 0.048, 0.052, 0.053, 0.05, 0.047, 0.045, 0.04, 0.036",
"weekend_sch": "0.035, 0.033, 0.032, 0.031, 0.032, 0.033, 0.037, 0.042, 0.043, 0.043, 0.043, 0.044, 0.045, 0.045, 0.044, 0.046, 0.048, 0.052, 0.053, 0.05, 0.047, 0.045, 0.04, 0.036"
"weekday_sch": "0.046, 0.046, 0.046, 0.046, 0.046, 0.037, 0.035, 0.034, 0.033, 0.028, 0.022, 0.015, 0.012, 0.011, 0.011, 0.012, 0.019, 0.037, 0.049, 0.065, 0.091, 0.105, 0.091, 0.063",
"weekend_sch": "0.046, 0.046, 0.045, 0.045, 0.046, 0.045, 0.044, 0.041, 0.036, 0.03, 0.024, 0.016, 0.012, 0.011, 0.011, 0.012, 0.019, 0.038, 0.048, 0.06, 0.083, 0.098, 0.085, 0.059"
Copy link
Contributor

Choose a reason for hiding this comment

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

@joseph-robertson Did we miss these *.osw update with the lighting schedule Pull request #419?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

@codecov-io
Copy link

codecov-io commented Mar 18, 2020

Codecov Report

Merging #426 into master will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #426   +/-   ##
=======================================
  Coverage   93.42%   93.42%           
=======================================
  Files         168      168           
  Lines       49400    49400           
=======================================
+ Hits        46150    46151    +1     
+ Misses       3250     3249    -1     
Impacted Files Coverage Δ
project_testing/tests/test.rb
project_multifamily_beta/tests/test.rb 84.61% <0.00%> (ø)
...easures/HPXMLtoOpenStudio/resources/hvac_sizing.rb 81.52% <0.00%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d8f44c...8d6a9d8. Read the comment docs.

@joseph-robertson
Copy link
Contributor Author

joseph-robertson commented Mar 31, 2020

@TobiAdekanye Were you able to confirm that this branch works? And is ready to merge?

@joseph-robertson Yes, this branch works and is ready to be merged. Thanks!

@joseph-robertson joseph-robertson merged commit 6674d15 into master Apr 1, 2020
@joseph-robertson joseph-robertson deleted the roof-mat-restructure branch April 1, 2020 15:25
@afontani afontani mentioned this pull request Apr 8, 2020
9 tasks
afontani pushed a commit that referenced this pull request Apr 8, 2020
Cherry-pick commit to revert the Insulation Unfinished Attic.tsv to changes made in #426.
afontani added a commit that referenced this pull request Apr 9, 2020
Bugfix: Revert Insulation Unfinished Attic to #426
joseph-robertson added a commit that referenced this pull request Jun 26, 2020
947481a7c Merge branch 'master' into build-res-hpxml-v3
a0464a7f1 Merge pull request #424 from NREL/ideal-system
1758bc7ed Clean up some argument descriptions.
092322600 Switch to a single heating system efficiency arg.
b1d2db658 Update docs. [ci skip]
db048d552 Merge branch 'ideal-system' of https://github.com/NREL/OpenStudio-HPXML into ideal-system
8cc7249f5 resolve report test issue
294bb45b8 Updates for coal fuel type.
bfec5cdc9 Found a few more places to use the new hpxml methods.
284a6e25a Merge branch 'master' into build-res-hpxml-v3
650d71d63 update_measures
b0dee0c59 Merge branch 'master' of https://github.com/NREL/OpenStudio-HPXML into ideal-system
ce8f5d66e total check no longer applicable
8e8a864b5 Merge pull request #428 from NREL/skylight-ufactor-fix
cb8299a29 Merge branch 'master' of https://github.com/NREL/OpenStudio-HPXML into skylight-ufactor-fix
22425cb7c Merge pull request #431 from NREL/more-fuel-types
3e20fcae8 Rename FuelTypeWood to FuelTypeWoodCord and some small code refactoring in the reporting measure.
949fd9caa ideal system unmet load test not applicable
d76ba3d5d report only residual ideal system unmet load
1631ac67e Add energyplus.rb resource w/ a few constants and methods to simplify code.
a5fb4c4c2 Merge branch 'master' of https://github.com/NREL/OpenStudio-HPXML into ideal-system
ac119d2af Prevent coal from being an option for some heating systems in EPvalidator, rather than catching it in hvac.rb.
cfcb1644e Oops.
ef423877c Merge pull request #429 from NREL/load-warnings
82937fdfb Merge branch 'master' of https://github.com/NREL/OpenStudio-HPXML into load-warnings
326ddd0a2 Allow additional HPXML fuel types for HVAC systems, water heating systems, and appliances. (Coal and similar fuel types are only allowed for boilers, not other heating system types, due to a limitation in the E+ Coil:Heating:Fuel object.)
bc842481a Revert some unexpected changes, and bugfix..
9b9833a92 bugfix
96564091f Merge branch 'master' of https://github.com/NREL/OpenStudio-HPXML into ideal-system
d132da8c9 Update docs and minor updates to defaults.
c3d4362f1 Add warnings for plug/fuel loads that are not modeled.
f5b85eb49 Merge pull request #427 from NREL/other-heated-space-is-thermal-boundary
2244b940c Converts skylight NFRC U-factor (at 20-degree slope) to SimpleGlazingSystem model input (vertical position).
6ac3f6446 Treat HPXML::LocationOtherHeatedSpace as a conditioned space for HPXML::is_thermal_boundary methods.
96e2a4040 Merge pull request #426 from NREL/eri-end-uses
5fce3a8e3 Merge branch 'master' of https://github.com/NREL/OpenStudio-HPXML into ideal-system
e5db62006 Exclude unused end uses for ERI outputs.
5391b8814 Merge branch 'master' of https://github.com/NREL/OpenStudio-HPXML into ideal-system
77ef1e190 bugfix
863ee94a9 two ideal systems

git-subtree-dir: resources/hpxml-measures
git-subtree-split: 947481a7c3447c2712015848354ca943b6288b10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants