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

fix bug for small window areas #452

Merged
merged 10 commits into from
Jun 10, 2020
Merged

fix bug for small window areas #452

merged 10 commits into from
Jun 10, 2020

Conversation

aspeake
Copy link
Contributor

@aspeake aspeake commented May 15, 2020

Fixes #450

Pull Request Description

@whiphi92 uncovered a bug that would throw an error when the calculated window area of a facade was less than the minimum allowable window area. This fix checks for that case and applies the invalid window area to the largest available surface.

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.

@aspeake aspeake requested review from shorowit and whiphi92 May 15, 2020 18:00
@aspeake
Copy link
Contributor Author

aspeake commented May 15, 2020

@whiphi92 Can you test this with the ResStock project you were running previously?

@codecov-io
Copy link

codecov-io commented May 15, 2020

Codecov Report

Merging #452 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #452      +/-   ##
==========================================
+ Coverage   93.43%   93.44%   +0.01%     
==========================================
  Files         168      168              
  Lines       49472    49506      +34     
==========================================
+ Hits        46223    46260      +37     
+ Misses       3249     3246       -3     
Impacted Files Coverage Δ
measures/TimeseriesCSVExport/measure.rb 93.58% <100.00%> (+0.14%) ⬆️
...eriesCSVExport/tests/timeseries_csv_export_test.rb 99.49% <100.00%> (+0.02%) ⬆️
resources/measures/ResidentialAirflow/measure.rb 99.18% <100.00%> (-0.02%) ⬇️
...sidentialAirflow/tests/residential_airflow_test.rb 100.00% <100.00%> (ø)
...s/ResidentialGeometryWindowSkylightArea/measure.rb 95.33% <100.00%> (+0.76%) ⬆️
...indowSkylightArea/tests/WindowSkylightArea_Test.rb 99.37% <100.00%> (+0.01%) ⬆️
...esources/measures/ResidentialHVACBoiler/measure.rb 97.61% <100.00%> (+0.11%) ⬆️
...entialHVACCentralSystemBoilerBaseboards/measure.rb 97.72% <100.00%> (+0.16%) ⬆️
...res/ResidentialHVACCentralSystemFanCoil/measure.rb 98.18% <100.00%> (+0.12%) ⬆️
...asures/ResidentialHVACCentralSystemPTAC/measure.rb 97.36% <100.00%> (+0.22%) ⬆️
... and 7 more

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 41ae698...b92eccc. Read the comment docs.

@whiphi92
Copy link
Contributor

I re-tried the run with the same buildstock.csv and YAML file and the 3 fails I had are now successes. So looks like this fixes the bug.

@whiphi92 whiphi92 closed this May 15, 2020
@whiphi92 whiphi92 reopened this May 15, 2020
@codecov-commenter
Copy link

codecov-commenter commented May 26, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@357a657). Click here to learn what that means.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #452   +/-   ##
=========================================
  Coverage          ?   93.44%           
=========================================
  Files             ?      168           
  Lines             ?    49518           
  Branches          ?        0           
=========================================
  Hits              ?    46271           
  Misses            ?     3247           
  Partials          ?        0           
Impacted Files Coverage Δ
...s/ResidentialGeometryWindowSkylightArea/measure.rb 95.41% <100.00%> (ø)
...indowSkylightArea/tests/WindowSkylightArea_Test.rb 99.37% <100.00%> (ø)

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 357a657...9f6335c. Read the comment docs.

@aspeake
Copy link
Contributor Author

aspeake commented Jun 10, 2020

@shorowit Can you take a quick look at this? We'd like to merge is asap.

@aspeake aspeake removed the request for review from shorowit June 10, 2020 19:41
Copy link
Contributor

@joseph-robertson joseph-robertson left a comment

Choose a reason for hiding this comment

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

👍

@aspeake aspeake merged commit 2a880aa into master Jun 10, 2020
@aspeake aspeake deleted the issue-450 branch June 10, 2020 22:15
@joseph-robertson joseph-robertson added this to the ResStock v2.3.0 milestone Jun 17, 2020
joseph-robertson added a commit that referenced this pull request Oct 6, 2020
b61d28400 Merge pull request #452 from NREL/build-res-hpxml-v3-schedule-files
bd2138e86 Use string and not array for cf monthly mults.
822c3ec2a Fix assignment of ceiling fan mults.
444eca4ac Merge branch 'build-res-hpxml-v3' into build-res-hpxml-v3-schedule-files
809abccbe Update measure xmls.
ce2d4f814 Merge branch 'master' into build-res-hpxml-v3
4e4ade81e Merge pull request #504 from NREL/workflow-tests-on-windows
b3265fa8c Use GC.start to prevent possible error on Windows where the sql file can't be deleted when running batch simulations as part of the workflow tests.
a0314c2b6 Remove temporary debug statement.
d2a70ba3e Merge branch 'build-res-hpxml-v3' into build-res-hpxml-v3-schedule-files
c0aa96c74 Merge branch 'build-res-hpxml-v3' into build-res-hpxml-v3-schedule-files
049353cf9 Avoid sub folders in resources directory.
28fac80e0 Merge branch 'build-res-hpxml-v3' into build-res-hpxml-v3-schedule-files
c2d0905fd Merge branch 'build-res-hpxml-v3' into build-res-hpxml-v3-schedule-files
8912f2713 Merge branch 'build-res-hpxml-v3' into build-res-hpxml-v3-schedule-files
037c18477 Merge branch 'build-res-hpxml-v3' into build-res-hpxml-v3-schedule-files
bd1589886 Revert changes to lighting schedule.
bf4a7fa21 Include readme for new schedules directory.
61f0ff08d Refactor so as not to copy code.
98b682527 Update description of end uses that vacancy affects.
716ffcc17 Small fix for default schedules type.
170b96dd4 Add descriptions for vacancy related arguments.
38344588f Move a comment for exterior lighting weekday fractions.
0c62766af Change average to default and improve schedules type arg description.
e23cd8efe Point to sample csv instead of listing column names in docs.
38046241c Reduce exported schedules csv file size.
dead8c29d Modify the csv path hpxml element name.
05db6a119 Remove the currently unused set_outage method.
e81bc223d Replace month/day units with # for run period, dst, and vacancy arguments.
be36291c9 Remove debugs and fix an issue.
0f115e408 More debug.
581234b50 Print a debug line.
b1323eeeb Cleaning up and fixing.
f79f809a8 Merge branch 'build-res-hpxml-v3' into build-res-hpxml-v3-schedule-files
7995f3c61 Attempt to diagnose test failure.
7ccecddb8 Fix typos and remove unnecessary test.
0b2322ce4 Merge branch 'build-res-hpxml-v3' into build-res-hpxml-v3-schedule-files
2af1deafc Fix things in hot water resource.
b9b8c28a9 More merge conflicts, 2.
b9dd4d2f5 More merge conflicts.
97cee8416 Weird merge stuff going on.
70969705e Merge branch 'build-res-hpxml-v3' into build-res-hpxml-v3-schedule-files
4067761c4 Merge branch 'build-res-hpxml-v3' into build-res-hpxml-v3-schedule-files
39134750d Merge branch 'build-res-hpxml-v3' into build-res-hpxml-v3-schedule-files
3276e9bfd Call schedules file class once at top of measure.
1ad02b163 Use average schedules by defaulting, not schedule csv.
cc0891fe0 Update the docs.
4e9d0afd3 Clean up a few typos.
c5b036b74 Move schedules into shared resource file.
f6ffbf581 Change average cd schedule to wkdy, wked, mnthly.
edc8fd34c Add average hot water schedules.
35bd54421 Bit of rearranging.
db46af353 Update epvalidator to have either schedules or schedules path.
80f1103eb Remove schedule column string from epvalidator.
293f09e15 Add schedules require back in.
5192f0390 Merge branch 'build-res-hpxml-v3' into build-res-hpxml-v3-schedule-files
462e3f471 Few fixes.
da805b4a1 Merge branch 'build-res-hpxml-v3' into build-res-hpxml-v3-schedule-files
d02f18e9b Change variable name.
55880c0e9 Progress on setting average schedules.
3b8f8af06 Optionally point to rf, fz, fl, pool, hot tub schedules.
a9784e4fa Continue creating average vs stochastic schedules.
79233ddcb Get things running.
82c127a6d Stub test osws with schedule types.
c5e613205 Remove schedule fields from hpxml.
bf970868e Remove fraction and multiplier arguments.
888bd046b Merge branch 'build-res-hpxml-v3' into build-res-hpxml-v3-schedule-files
972105e6a Clean up vacancy method.
3e8152aa0 Typo.
6be817138 Update epvalidator for schedule fields.
5d065c62a Add fixtures column to exported schedules csv.
3540c7d30 Move fixture schedule names to water heating level.
77e77f0b5 Set vacancy only if schedules are generated.
a35e27334 Set vacancy after calculating design levels.
b24256457 Another typo in ceiling fan schedule file.
bbbeab6c5 Typo in ceiling fan schedule file.
a9cc2aac4 Connect clothes dryer to schedule file.
88f3cdb3b Connect ceiling fan to schedule file.
9b61aa57c Merge branch 'build-res-hpxml-v3' into build-res-hpxml-v3-schedule-files
d44a1af2e Set vacancy on generation instead of afterwards.
f0c7a123c Update vacancy osw to be 6 months.
2f684b43e Set calendar year before schedule generation.
f92ef9b33 Add osw test for vacancy period.
2e5197b64 Add vacancy args to build measure.
6ddce1e8a Avoid regenerating schedule when it exists.
0e4cf0103 Merge branch 'build-res-hpxml-v3' into build-res-hpxml-v3-schedule-files
5c0d553a2 Connect cw and dw to schedule file.
5d03a5535 Connect lighting to schedule file object.
2a161f90a Connect other plug loads to schedule file.
f04f3922f Start connecting translator measure to schedule file object.
5440e6b12 Set schedule path in header.
1d92af07e Move schedule generation into build measure.

git-subtree-dir: resources/hpxml-measures
git-subtree-split: b61d284004b090ca5a0a109b78967b83fc74300d
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.

ResidentialGeometryWindowSkylightArea Error - possibly that a window is larger than a facade
5 participants