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

Initializes nrcan_446 branch: skylights & wells #1854

Open
wants to merge 1 commit into
base: nrcan
Choose a base branch
from
Open

Conversation

brgix
Copy link
Collaborator

@brgix brgix commented Nov 25, 2024

Pull request overview

Description

BTAP is currently limited in how/where it adds NECB-required skylights in OpenStudio models (e.g. SRR 5% for NECB2011). Outstanding limitations include sloped roofs, narrow/concave roof surfaces, plenum/attic cases (requiring skylight wells). These initial limitations are understandable, given the challenges of dealing with odd or finely-grained geometry - no walk in the park. Yet without a general solution, BTAP remains unable to quantify (to the full extent) energy/carbon savings (or more commonly penalties) from code-required skylights in NECB reference buildings.

Approach

The solution is part of the OSut gem, a TBD dependency (TBD is now a standard OpenStudio gem, and is distributed with BTAP/OpenStudio-Standards). OSut's addSkylights method tackles the above limitations. It can fail with invalid geometry (e.g. some of the DND models!), or if the model geometry is, let's say, unreasonable (e.g. just a bunch of closets, or where all roof surfaces are heavily tessellated or subdivided as strips the width of a desk). This unfortunately may happen with 3rd-party models! Easiest to consult related OSut PRs, here & here.

Testing Plan

OSut's addSkylights is already heavily tested: e.g. OSut RSpec, as well tests on ALL DND projects (private repo, some BTAP staff have access). Specific BTAP unit tests are nonetheless needed. Existing regression tests will need to be updated, as the implications of a general solution are somewhat far reaching (e.g. automatic photocell placement, heating/cooling sizing, resulting energy use + carbon emissions).

Related issues

This work may solve the concerns raised here & here - not sure.

Pull Request Author

  • Method changes or additions
  • Data changes or additions
  • Added tests for added methods
  • If methods have been deprecated, update rest of code to use the new methods
  • Documented new methods using yard syntax
  • Resolved yard documentation errors for new code (ran bundle exec rake doc)
  • Resolved rubocop syntax errors for new code (ran bundle exec rake rubocop)
  • All new and existing tests passes
  • If the code adds new require statements, ensure these are in core ruby or add to the gemspec

Review Checklist

This will not be exhaustively relevant to every PR.

  • Perform a code review on GitHub
  • All related changes have been implemented: method additions, changes, tests
  • If fixing a defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If a new feature, test the new feature and try creative ways to break it
  • CI status: all green or justified

@brgix brgix requested a review from ckirney November 25, 2024 17:03
@brgix brgix self-assigned this Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant