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 options and tsvs updates #485

Merged
merged 5 commits into from
Sep 23, 2020
Merged

Conversation

joseph-robertson
Copy link
Contributor

@joseph-robertson joseph-robertson commented Sep 16, 2020

Addresses #[issue number here].

Pull Request Description

Expand roof material options and update related tsv files.

TODO:

  • Update the changelog

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 Sep 16, 2020
@joseph-robertson joseph-robertson added this to the ResStock v2.4.0 milestone Sep 16, 2020
@joseph-robertson joseph-robertson marked this pull request as ready for review September 16, 2020 21:37
@trynthink
Copy link

A couple questions:

  • Do we need to include any modifications to Radiant Barrier.tsv?
  • Do we need to update any OSWs and regenerate the corresponding OSMs?

@TobiAdekanye
Copy link
Contributor

These commits look fine. However, like @trynthink said, some extra changes were implemented in the Grid Benefit Analysis branch that arent implemented here:
d2377e8
9e11e93
c31c84e

@joseph-robertson
Copy link
Contributor Author

joseph-robertson commented Sep 17, 2020

These commits look fine. However, like @trynthink said, some extra changes were implemented in the Grid Benefit Analysis branch that arent implemented here:
d2377e8
9e11e93
c31c84e

@TobiAdekanye So I'm a little confused. Are you implying we should be carrying thru changes related to Heating/Cooling Setpoint Apply Offset? And weather? And vacancy? From @trynthink I got the impression we only wanted changes related to roof material.

@joseph-robertson. There were some file changes that I thought didn't apply to weather or vacancy but I might be wrong e.g.
project_multifamily_beta/housing_characteristics/Radiant Barrier.tsv
measures/BuildExistingModel/measure.xml
resources/measures/HPXMLtoOpenStudio/measure.xml

@joseph-robertson
Copy link
Contributor Author

A couple questions:

  • Do we need to include any modifications to Radiant Barrier.tsv?
  • Do we need to update any OSWs and regenerate the corresponding OSMs?

I don't believe these changes to possible roofing materials require any modifications to Radiant Barrier.tsv.
I added some unit tests for thermal, solar, and visible absorptances. We don't need to regenerate test osms because no test osws were added/updated.

@joseph-robertson
Copy link
Contributor Author

@trynthink @TobiAdekanye Is this ready to go?

joseph-robertson added a commit that referenced this pull request Sep 18, 2020
…3c52e

8f87813c52e Refine testing for multiple heating or cooling.
94159e65dfd Leave out pv system false.
fd77d1546a8 Merge branch 'master' into build-res-hpxml-v3
1ed004a8fe1 Merge pull request #490 from NREL/fix_ci_tests
ad65c79ed03 Some tests weren't running on the CI.
1c5c8f58365 Merge pull request #488 from NREL/shared-pv-system
ff1166ecfdd Change IsSharedSystem to be optional for PV, default to false. No longer a breaking change.
cff0b12b8f2 Merge pull request #487 from NREL/unconditioned-surface-adjacent-to-other-housing-unit
5f32ac58043 Small cleanup.
53218ca1bcf Merge branch 'master' of https://github.com/NREL/OpenStudio-HPXML into unconditioned-surface-adjacent-to-other-housing-unit
d96ecaff135 Require materials and psychros in case we need to weather cache.
30936180fef Merge pull request #485 from NREL/hvac-sizing-controls
359b2354edb Small docs update. [ci skip]
b8c31283a24 Bugfix for a surface between an unconditioned space and "other housing unit" being incorrectly set as adiabatic. Now uses SurfacePropertyOtherSideCoefficients to appropriately set the adjacent temperature.
8379cf1b2f0 update_measures
0a6fecde90b Merge branch 'master' of https://github.com/NREL/OpenStudio-HPXML into hvac-sizing-controls
693f2063426 Merge pull request #486 from NREL/duct-lto-testing-exemption
ccfd83d316f Renames DuctLeakageToOutsideTestingExemption to DuctLeakageToOutsideTestingExemption, to clarify that it is different from the total duct leakage testing exemption in ANSI/RESNET/ACCA 310.
6e298b9ca82 Merge branch 'master' of https://github.com/NREL/OpenStudio-HPXML into hvac-sizing-controls
345ec0d6ff3 Update test.
9ba85000862 Merge pull request #484 from NREL/test_validation_cli
66fb7a83979 Bugfix.
681cb17138d Adds high-level HVAC sizing controls.
183d993cbbb Allow running test_validation.rb via the OS CLI (only ruby tests will run). The CI will continue to run all tests (i.e., including schematron-nokogiri).

git-subtree-dir: resources/hpxml-measures
git-subtree-split: 8f87813c52ee35e754cbac304f61f2c80b878e3d
@trynthink
Copy link

@joseph-robertson It looks good to me, though it would be good to get concurrence from @TobiAdekanye as well. Is it standard practice on this repo to merge in changes on branches as-is or to merge/rebase on master and/or squash commits before merging? I'll leave it to you to clean up or revise these commits as needed (if at all) to be consistent with expectations or standard practice on this repo.

@joseph-robertson
Copy link
Contributor Author

joseph-robertson commented Sep 21, 2020

@joseph-robertson It looks good to me, though it would be good to get concurrence from @TobiAdekanye as well. Is it standard practice on this repo to merge in changes on branches as-is or to merge/rebase on master and/or squash commits before merging? I'll leave it to you to clean up or revise these commits as needed (if at all) to be consistent with expectations or standard practice on this repo.

So far on this repo we've just merged in changes on branches as-is.

@TobiAdekanye does this PR look ready to go?
@joseph-robertson this looks good to me!!

@joseph-robertson joseph-robertson merged commit f74d92c into master Sep 23, 2020
@joseph-robertson joseph-robertson deleted the roof-mat-updates branch September 23, 2020 16:21
@ejhw
Copy link
Contributor

ejhw commented Nov 19, 2020

@trynthink What is the data source for the updated roof material options? RECS? Were there any assumptions made?

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.

4 participants