-
Notifications
You must be signed in to change notification settings - Fork 58
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
brgix
wants to merge
11
commits into
nrcan
Choose a base branch
from
nrcan_446
base: nrcan
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+612
−719
Open
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
2a4ed9b
Initializes nrcan_446 branch: skylights & wells
brgix 9dd78b6
Comments out (broken) 'setConstruction' call
brgix 4fad85d
Initial implementation + unit test
brgix 64536f3
BTAP pre-1980 tweaks + gem spec tweaks
brgix 32cb6f1
Fixes skylight gross area tally
brgix cfc5d71
Fixes SmallOffice roof overhang area bug
brgix 4f72316
Reduces gra0 vs graX equations
brgix e60c1e1
Merge pull request #1874 from NREL/nrcan
ckirney 68e3f30
Reintroduce uprated Uo assignment
brgix 6691d2c
Cleanup + full unit TBD tests
brgix 5b32f37
Merge pull request #1881 from NREL/nrcan
brgix File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reintroducing the
BTAP::Geometry::Surfaces.set_surfaces_construction_conductance
call. This had changed about a year ago with this commit. However, the change couldn't have worked given thatconstruction[:uo]
is simply a U-factor (in W/K.m2). Reverting to the original solution for now.TBD doesn't need to have uprated, clear-field U-factors reset for constructions this way. At the time, keeping track (within the BTAP-generated OSM) of the uprated construction U-factors was deemed useful for documentation/costing purposes. This can be achieved otherwise within BTAP. Other changes are expected over the next weeks, given the upcoming structure/construction changes to BTAP. So this may change.
Note that this is unrelated to skylight wells, and the change should be introduced under a new (or another existing) pull request. TODO.