-
Notifications
You must be signed in to change notification settings - Fork 2
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
More robust 'addSkylights' (+ dependencies) #20
Conversation
@@ -2054,7 +2054,7 @@ def setpoints(space = nil) | |||
res[:heating] = nil | |||
res[:cooling] = nil | |||
elsif cnd.downcase == "semiheated" | |||
res[:heating] = 15.0 if res[:heating].nil? | |||
res[:heating] = 14.0 if res[:heating].nil? |
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.
A first of a handful of changes unrelated to skylights.
The Canadian NECBs use a "< 15°C" threshold (not "<= 15°C") to determine whether a space/zone is considered SEMIHEATED.
return mismatch("space", space, cl, mth, DBG, false) unless space.is_a?(cl) | ||
|
||
# 1. First check OSut's REFRIGERATED status. | ||
status = space.additionalProperties.getFeatureAsString(tg0) |
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.
Similar to plenum? and vestibule?, refrigerated? gives priority to an AdditionalProperties tag (useful e.g. when thermal zoning is in an incomplete state).
@@ -2945,9 +2999,16 @@ def getLineIntersection(s1 = [], s2 = []) | |||
a1b2 = b2 - a1 | |||
xa1b1 = a.cross(a1b1) | |||
xa1b2 = a.cross(a1b2) | |||
xa1b1.normalize | |||
xa1b2.normalize | |||
xab.normalize |
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.
Quite the oversight not to normalize, especially when dealing with one line segment possibly orders of magnitude longer than the other.
|
||
segments.each do |sg| | ||
intersect = getLineIntersection([p0, p1], sg) | ||
next unless intersect | ||
|
||
# One of the polygon vertices? | ||
# Skip test altogether if one of the polygon vertices. |
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.
These collisions are challenging to deal with - easier to skip them altogether.
# | ||
# @return [Bool] if considered a roof surface | ||
# @return [false] if invalid input (see logs) | ||
def roof?(pts = nil) |
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.
Single-span, prefab arched structures often end up with tilted side walls that share the same construction as their neighbouring roof segments. Quite common in single story industrial facilities (e.g. NorsemanStructures):
![norseman](https://private-user-images.githubusercontent.com/98104985/373017249-c1a5ab31-c0f6-4e7d-9a7a-3cd4497197be.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzg4MTE1NzUsIm5iZiI6MTczODgxMTI3NSwicGF0aCI6Ii85ODEwNDk4NS8zNzMwMTcyNDktYzFhNWFiMzEtYzBmNi00ZTdkLTlhN2EtM2NkNDQ5NzE5N2JlLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMDYlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjA2VDAzMDc1NVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTVjMmRjNDU0OWQzMzhlMDIxMGE2NmU0ZDUzODdhNDg3ZTIzMzIzN2YyYjhmMmNkZjJlZmNhMWYyZDNkMzliODMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.slumX3qj9SHPSWRkTLfVU384YLjo53ME4tQRrPK21Oc)
Am sometimes encountering OpenStudio models where such side walls end up typed as roofs, possibly to facilitate default construction inheritance. Understandable, yet energy codes (90.1, NECBs) are pretty clear: once a roof's outward normal is tilted too far away from zenith, it's considered a wall (i.e. not a candidate surface for skylights).
getSegments(p1).each do |sg| | ||
mp = midpoint(sg.first, sg.last) | ||
return false unless pointWithinPolygon?(mp, p2) | ||
end |
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.
This is an expensive add-on, but essential when dealing with convoluted surfaces, namely holding multiple sequential concavities, like C-shaped or G-shaped corridors. Want to speed things up? Break up such a surface into multiple convex ones.
unless [true, false].include?(force) | ||
log(DBG, "Ignoring force input (#{mth})") | ||
force = false | ||
end |
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.
This option became key to systematize the treatment of roof surfaces, whether flat, low-sloped, etc.
# Roof vs ceiling may neither share alignment transformation nor space | ||
# site transformation identities. All subsequent calls to 'genAnchors' | ||
# shall recover the :out points, apply a succession of de/alignments and | ||
# transformations in sync , and overwrite tagged points. |
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.
This redesign (or rather a more systematic application of an original principle) became necessary when trying to manage multiple skylight arrays for a single roof surface (e.g. a large unconditioned attic roof sheltering a dozen conditioned spaces below).
next unless o[:set] | ||
|
||
st[:out] = o | ||
st[:bx ] = blc(o[:r] * (o[:t] * o[:set])) |
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.
Bounded boxes are systematically bottom-left-cornered (BLC) - no longer any need to post-process them all.
# @param bfr [#to_f] safety buffer, to maintain near other edges | ||
# | ||
# @return [Bool] whether addition is successful | ||
# @return [false] if invalid input (see logs) | ||
def addSubs(s = nil, subs = [], clear = false, bfr = 0.005) | ||
def addSubs(s = nil, subs = [], clear = false, bound = false, realign = false, bfr = 0.005) |
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.
Setting options bound and realign to true became key to the systematic treatment of roofs (as candidates for skylights), whether flat, low sloped, etc.
# @option opts [#to_f] :size (1.22m) template skylight width/depth (min 0.4m) | ||
# | ||
# @return [Array<OpenStudio::Model::Space>] candidates (see logs if empty) | ||
def toToplit(spaces = [], opts = {}) |
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.
An option to addSkylights, yet called upon by default. This is useful when treating multiple 3rd-party models (e.g. a specific market segment), with undiagnosed geometry (i.e. flying blind). Through trial and error, these handful of sliders and rules have yielded satisfactory results during recent stress testing.
else | ||
return mismatch("area", opts[:area], Numeric, mth, DBG, 0) | ||
end | ||
elsif opts.key?(:srr) |
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.
It is often preferable to precalculate a desired total skylight :area (from a building's grossRoofArea x a code-required SRR%) than relying on the :srr option directly. Once :area is determined, users may (wisely) choose to filter out technical or ancillary spaces (e.g. washrooms, corridors) as potential candidates for skylights. The value of :area would remain valid - not the :srr.
|
||
toitures.each { |surf| h = [h, surf.vertices.max_by(&:z).z].max } | ||
plafonds.each { |surf| h = [h, surf.vertices.max_by(&:z).z].max } | ||
h = hMAX - hMIN |
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.
Yikes - quite the oversight!
# smaller than that of its roof. A set is 'thin' if the depth of its | ||
# bounded box is (too) narrow. If either is true, some geometry rules | ||
# may be relaxed to maximize allocated skylight area. Neither apply to | ||
# cases with skylight wells. |
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.
A roof surface's tightness and/or thinness trigger accommodations to maximize skylight allocation in challenging cases.
set[:roof ] = roof | ||
set[:space ] = space | ||
set[:m ] = space.multiplier |
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.
A number of necessary changes have been introduced to correct the use of space multipliers (and elsewhere existing subsurface multipliers). Unfortunately no RSpec tests yet ... @todo.
end | ||
end | ||
|
||
# Size contraction: round 2: prioritize larger sets. |
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.
Multiple rounds of skylight area contractions (instead of a single one), towards strictly meeting a requested :srr or :area.
|
||
expect(s.setSurfaceType("Floor")).to be true | ||
expect(s.setVertices(s.vertices.reverse)).to be true | ||
end |
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.
The antiquated plenum floor vertex sequencing starts to be an issue with more recent OpenStudio versions.
vtx << OpenStudio::Point3d.new(-14.9024000, 2.7700, 3.658) | ||
vtx << OpenStudio::Point3d.new(-14.9024000, 0.0000, 3.658) | ||
|
||
bbx = mod1.boundedBox(vtx) |
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.
# - a new method identifying surface cutouts amongst surface vertices | ||
# - a method to purge both leader lines and cutouts from surface vertices | ||
# - have 'getRoofs' & 'grossRoofArea' rely on the remaining outer vertices | ||
# ... @todo? |
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.
Future considerations towards better harmonizing OSut's cutouts and leader lines vs native OpenStudio/EnergyPlus methods ... @todo.
expect(zn.volume).to be_empty | ||
end | ||
end | ||
|
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.
INF = OSlg::INFO.dup # see github.com/rd2/oslg | ||
WRN = OSlg::WARN.dup # see github.com/rd2/oslg | ||
ERR = OSlg::ERROR.dup # see github.com/rd2/oslg | ||
FTL = OSlg::FATAL.dup # see github.com/rd2/oslg |
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.
This is an attempt to circumvent (often numerous) Ruby warnings related to duplication of constants. These pop up when building/using parent applications relying on gems (like TBD), which have OSut as a dependency. That's possibly 3 to 4 gems harmonizing constants, while trying to keep things concise (e.g. not having to drag module namespaces if extended). It will take a while to update/build/test larger, multi-gem applications before making a definitive decision.
@@ -8,6 +8,8 @@ | |||
# Disable RSpec exposing methods globally on `Module` and `main` | |||
config.disable_monkey_patching! | |||
|
|||
# config.warnings = true |
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.
Sometimes useful to switch on during development.
expect(cls1.clean!).to eq(DBG) | ||
|
||
# The v1.11.5 (2016) seb.osm, shipped with OpenStudio, holds (what would now | ||
# be considered as deprecated) a definition of plenum floors (i.e. ceiling | ||
# tiles) generating several warnings with more recent OpenStudio versions. |
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.
Harmonized with (more detailed) fixes introduced in TBD Tests.
Significant changes to addSkylights, including quite a few dependencies!
Most of the tests included in this repository will offer little (to no) change when comparing outcomes/results (vs current release), following this merge. However, recent testing (of several very large 3rd-party models) have underlined a lack of robustness when it comes to challenging conditions (e.g. very convoluted roof surfaces). Unfortunately, it is not possible to include these 3rd-party models here - a separate private fork will be maintained to ensure harmonization.
Comments + visuals to come ...