Tweaks 'genConstruction' (tested) by brgix · Pull Request #22 · rd2/osut

brgix · 2025-08-05T15:20:47Z

Related to this pyOSut PR.

Around 40-odd typos (and other oddities) were identified in this Ruby implementation of OSut, while developing a Python counterpart, pyOSut. In some cases, these are simple fixes (no impact on the Python implementation). In other cases (like this first commit), they reveal weaknesses (to fix), such as not adequately catching bad user input. Over the next day or so, a number of such fixes are expected to be introduced, before re-releasing an upcoming OSut v0.6.1.

brgix · 2025-08-05T19:18:43Z

lib/osut/utils.rb

      a[:compo  ][:id  ] = "OSut|#{mt}|#{format('%03d', d*1000)[-3..-1]}"
    when :window
-      a[:glazing][:u   ]  = specs[:uo  ]
+      a[:glazing][:u   ]  = u ? u : @@uo[:window]


A user could set a glazing assembly's specs["uo"] to nil. This is caught and reset here.

brgix · 2025-08-05T19:20:28Z

lib/osut/utils.rb

+    if u and unglazed
+      ro = 1 / u - film
+
+      if ro > 0


For opaque construction, adjusting insulating layer thickness (to meet a requested assembly Uo factor) is skipped entirely if the requested Uo isn't:

Numeric

within postulated (acceptable) range

brgix · 2025-08-05T19:21:58Z

spec/osut_tests_spec.rb

+    expect(surface).to be_a(OpenStudio::Model::LayeredConstruction)
+    expect(surface.layers.size).to eq(1)
+    u = 1 / cls1::rsi(surface)
+    expect(u).to be_within(TOL).of(uo[:skylight])


In the case of fenestrated assemblies, an invalid Uo request is ignored and the glazing assembly inherits a default OSut Uo (here 3.5 W/m2.K for a skylight).

brgix · 2025-08-06T14:04:49Z

lib/osut/utils.rb

    m1  = "#{id}:spandrel"
    m2  = "#{id}:spandrel:boolean"
-    return mismatch(id, s, cl, mth) unless s.is_a?(cl)
+    return mismatch(id, s, cl, mth, false) unless s.is_a?(cl)


brgix · 2025-08-06T14:05:08Z

lib/osut/utils.rb


    id = s.nameString
-    return mismatch(id, s, cl, mth, false) unless s.is_a?(cl)
+    return mismatch(id, s, cl, mth, DBG, false) unless s.is_a?(cl)


Also yikes.

brgix · 2025-08-06T14:06:29Z

lib/osut/utils.rb

+
+    values.each do |value|
+      if value.respond_to?(:to_f)
+        vals << value.to_f


Validate time series data for scheduled temperatures: Log + skip if invalid, then move on.

brgix · 2025-08-06T14:08:14Z

lib/osut/utils.rb

+          res[:spt] = max unless res[:spt]
+          res[:spt] = max     if res[:spt] < max
+        end
+      end


Safe (?) to now check for ScheduleInterval classes.

brgix · 2025-08-06T14:08:36Z

lib/osut/utils.rb

  # @return [nil] if invalid input (see logs)
  def trueNormal(s = nil, r = 0)
-    mth = "TBD::#{__callee__}"
+    mth = "OSut::#{__callee__}"


brgix · 2025-08-06T14:09:08Z

lib/osut/utils.rb


    # Once joined, re-adjust Z-axis coordinates.
-    unless z.zero?
+    unless z.round(2) == 0.00


z is a float - not an integer.

brgix · 2025-08-06T14:09:46Z

spec/osut_tests_spec.rb

+    start  = model.getYearDescription.makeDate(1, 1)
+    inter  = OpenStudio::Time.new(0, 1, 0, 0)
+    values = OpenStudio.createVector(Array.new(8760, 22.78))
+    series = OpenStudio::TimeSeries.new(start, inter, values, "")


OSut enabled ScheduleFixedInterval checks, e.g.:

min/max setpoint temperatures

is space UNCONDITIONED, etc.

Yet no unit test had been in place.

brgix · 2025-08-09T12:15:10Z

lib/osut/utils.rb

    return pts if pts.size < 3
    return mismatch("n collinears", n, Integer, mth, DBG, v) unless ok
+    return invalid("+n collinears", mth, 0, ERR, v) if n > pts.size
+    return invalid("-n collinears", mth, 0, ERR, v) if n < 0 && n.abs > pts.size


Checks range of n number of points - harmonized with pyOSut.

brgix · 2025-08-09T12:16:15Z

lib/osut/utils.rb

+    a2  = flatten(a2).to_a if flat
+    cw2 = clockwise?(a2)
+    a2  = a2.reverse if cw2
+    return invalid("points 2", mth, 2, DBG, face) unless xyz?(a2, :z)


Eliminates redundancies between aligned vs non-aligned cases, harmonized with pyOSut.

brgix · 2025-08-09T12:16:40Z

lib/osut/utils.rb

    return face if p1.empty?
    return face if p2.empty?
-    return mismatch("ray", ray, cl, mth) unless ray.is_a?(cl)
+    return mismatch("ray", ray, cl, mth, face) unless ray.is_a?(cl)


Fixes (wrong) return value.

brgix · 2025-08-09T12:17:07Z

lib/osut/utils.rb

  # @param [Set<OpenStudio::Point3d>] a triad (3D points)
  #
-  # @return [Set<OpenStudio::Point3D>] a rectangular ULC box (see logs if empty)
+  # @return [Set<OpenStudio::Point3D>] a rectangular BLC box (see logs if empty)


Fixes (incorrect) vertex sequencing (documentation issue).

brgix · 2025-08-09T12:17:56Z

lib/osut/utils.rb

        next if same?(p1, sg.last)
        next if same?(p2, sg.first)
-        next if same?(p2, sg.first)
+        next if same?(p2, sg.last)


Fixes redundant check - brings in correct check.

brgix · 2025-08-09T12:19:49Z

lib/osut/utils.rb

      id = "plate # #{i+1} (index #{i})"

-      return mismatch(id, plt, cl1, mth, DBG, slb) unless plt.is_a?(cl2)
+      return mismatch(id, plt, cl2, mth, DBG, slb) unless plt.is_a?(cl2)


Fixes incorrect class check. No unit test had been written to stress-test this, so never got caught.

brgix · 2025-08-09T12:21:21Z

lib/osut/utils.rb

          elsif fpm2.keys.include?("strips")
            pattern = "strips"
-          else fpm2.keys.include?("strip")
+          else # fpm2.keys.include?("strip")


Ruby ignores anything following an else, so the hash key check is ignored. Keeping as comment.

brgix · 2025-08-09T12:22:11Z

spec/osut_tests_spec.rb

+    # Original polygon remains unaltered.
+    # vtx2 = mod1.poly(vtx)
+    # expect(mod1.same?(vtx, vtx2)).to be true
+    # expect(mod1.status).to eq(0)


Needed updating.

brgix · 2025-08-09T12:22:59Z

spec/osut_tests_spec.rb

+    slab = mod1.genSlab(plates, z0)
+    expect(mod1.debug?).to be true
+    expect(mod1.logs.size).to eq(1)
+    expect(mod1.logs.first[:message]).to include("String? expecting Hash")


New stress-test when requested floor plates (input) aren't hashes.

brgix · 2025-08-10T18:53:29Z

lib/osut/utils.rb

+      return pts
+    elsif pts.is_a?(OpenStudio::Model::PlanarSurface)
+      pts.vertices.each { |pt| v << OpenStudio::Point3d.new(pt.x, pt.y, pt.z) }
+      return v


This would otherwise return an Array.

brgix · 2025-08-10T18:59:38Z

lib/osut/utils.rb

    return v if pts.empty?
-    return mismatch("n unique points", n, Integer, mth, DBG, v) unless ok
+
+    if n.is_a?(Numeric)


Safer to check for Numeric than respond_to?(:to_i). Non-numeric strings respond_to?(:to_i), for instance.

brgix · 2025-08-10T19:01:49Z

lib/osut/utils.rb

-    a   = sg.first
-    b   = sg.last
+    a   = sg[ 0]
+    b   = sg[-1]


Safer. An admissible input segment could be an OpenStudio::Point3dVector (which would crash with a call to .last.

brgix · 2025-08-10T19:02:25Z

lib/osut/utils.rb

    end

-    if holds?(a, pts[0])
+    if a.include?(pts[0])


Quicker, same object ID.

brgix · 2025-08-10T19:05:38Z

lib/osut/utils.rb

-            sg0 = sg.to_a
-            sgX = sX[j].to_a
+            sg0 = sg
+            sgX = sX[j]


As each returned item from an OpenStudio::Point3dVectorVector is an Array in Ruby (and not an OpenStudio::Point3dVector), there would be no point in further converting an Array to an Array.

brgix · 2025-08-10T19:07:03Z

spec/osut_tests_spec.rb

+
+    # Stress test 'to_p3Dv'. 4 valid input cases.
+    # Valid case #1: a single Point3d.
+    vtx = mod1.to_p3Dv(p0)


All 4 valid to_p3Dv use cases systematically return an OpenStudio::Point3dVector (and not occasionally an Array).

brgix · 2025-08-11T12:51:22Z

lib/osut/utils.rb

  # @return [false] if invalid input (see logs)
  def segment?(pts = nil)
    pts = to_p3Dv(pts)
-    return false     if pts.empty?


brgix · 2025-08-11T12:51:58Z

lib/osut/utils.rb

-    return empty("segments",         mth, DBG, false)     if sgs.empty?
-    return mismatch("point", p0, cl, mth, DBG, false) unless p0.is_a?(cl1)
+    return empty("segments",          mth, DBG, false)     if sgs.empty?
+    return mismatch("point", p0, cl1, mth, DBG, false) unless p0.is_a?(cl1)


Oops - no cl variable. Clearly untested.

brgix · 2025-08-11T12:52:58Z

lib/osut/utils.rb

      str2 = str1 + " #{tag.to_s}"
-      next if st.key?(:void) && st[:void]
      return mismatch(str1, st,  Hash, mth, DBG, a) unless st.respond_to?(:key?)
+      next if st.key?(:void) && st[:void]


Hmm. Better to test for a Hash before attempting to access a key:value pair.

brgix · 2025-08-11T12:53:41Z

lib/osut/utils.rb

-      return mismatch(str, ld,  Hash, mth, DBG, a) unless ld.is_a?(Hash)
-      return hashkey( str, ld,     s, mth, DBG, a) unless ld.key?(s)
-      return mismatch(str, ld[s], cl, mth, DBG, a) unless ld[s].is_a?(cl)
+      return mismatch(str2, ld,  Hash, mth, DBG, a) unless ld.is_a?(Hash)


No str variable!

brgix · 2025-08-11T12:54:03Z

lib/osut/utils.rb

          # previously-added leader lines.
          #
          # @todo: revise approach for attics ONCE skylight wells have been added.
-          olap = nil


brgix · 2025-08-11T12:54:27Z

lib/osut/utils.rb

        w  = opts[:size].to_f
        w2 = w * w
-        return invalid(size, mth, 0, ERR, []) if w.round(2) < gap4
+        return invalid("size", mth, 0, ERR, []) if w.round(2) < gap4


No variable size - clearly a typo.

brgix · 2025-08-11T12:54:41Z

lib/osut/utils.rb

      spaces = spaces.reject { |sp| sp.floorArea < 4 * w2 }
      spaces = spaces.sort_by(&:floorArea).reverse
-      return empty("spaces", mth, WRN, 0) if spaces.empty?
+      return empty("spaces", mth, WRN, []) if spaces.empty?


Wrong return variable.

brgix · 2025-08-11T12:55:39Z

lib/osut/utils.rb

-        frame = nil if f.frameWidth.round(2) < 0
-        frame = nil if f.frameWidth.round(2) > gap
+        frame = nil if frame.frameWidth.round(2) < 0
+        frame = nil if frame.frameWidth.round(2) > gap


Typo: clearly untested.

brgix · 2025-08-11T12:56:33Z

lib/osut/utils.rb

-      when :sloped  then filters.map! { |f| f.include?("c") ? f.delete("c") : f }
-      when :plenum  then filters.map! { |f| f.include?("d") ? f.delete("d") : f }
-      when :attic   then filters.map! { |f| f.include?("e") ? f.delete("e") : f }
+      when :sidelit then filters.map! { |fl| fl.include?("b") ? fl.delete("b") : fl }


f already used (designates frame width): use fl instead.

brgix · 2025-08-11T12:57:25Z

spec/osut_tests_spec.rb

+    expect(mod1.same?(collinears[1], p1)).to be true
+
+    # Ignore n request when n.abs > number of actual collinears.
+    collinears = mod1.getCollinears([p0, p1, p2, p3, p8], 6)


Harmonized tests with pyOSut.

brgix · 2025-08-11T15:56:38Z

lib/osut/utils.rb

  #
  # @return [OpenStudio::Point3dVector] unique points (see logs)
-  def getUniques(pts = nil, n = 0)
+  def uniques(pts = nil, n = 0)


As per pyOSut convention, getter functions drop the get prefix.

brgix · 2025-08-11T20:50:12Z

spec/osut_tests_spec.rb

+    # function can be time consuming for very convoluted spaces (e.g. long
+    # corridors with multiple concavities).
+    expect(mod1.spaceHeight(fine)).to be_within(TOL).of(8.53)
+    expect(mod1.spaceWidth(fine)).to be_within(TOL).of(21.33)


The value returned by spaceHeight (8.53 m) corresponds to the tallest floor-to-ceiling distance (along Z-axis).

The floor section circumscribed (and crisscrossed) with red dotted lines corresponds to the largest bounded box that fits in the (large) L-shaped floor surface. The length of the narrowest edge of this bounded box (21.33 m) is what spaceWidth returns.

brgix · 2025-08-12T10:44:21Z

lib/osut/utils.rb

+    res = realignedFace(polyg.to_a.reverse)
+    return 0 if res[:box].nil?
+
+    height(res[:box])


Forgot to add the key piece (bounding box from realignFace).

brgix · 2025-08-12T10:59:32Z

spec/osut_tests_spec.rb

    expect(mod1.alignedWidth(floor)).to be_within(TOL).of(8.22)
    expect(mod1.spaceHeight(openarea)).to be_within(TOL).of(3.96)
-    expect(mod1.spaceWidth(openarea)).to be_within(TOL).of(7.90)
+    expect(mod1.spaceWidth(openarea)).to be_within(TOL).of(3.77)


in red: full height of space, including skylight wells

in green: width of bounding box of space floor(s), outline is very approximate

in blue: width of largest bounded box that can fit within grouped floor surfaces

The latter is retained here as the most adequate measure of what could be considered a room's width, e.g. for LPD adjustment calculations.

Tweaks 'genConstruction' (tested)

0114e28

brgix self-assigned this Aug 5, 2025

brgix added the enhancement New feature or request label Aug 5, 2025

brgix added 2 commits August 5, 2025 11:23

Adds OS 3.10/Ubuntu-latest GH Action test

0c41d04

Adds OS 3.10/Ubuntu-2204 GH AT (redux)

4394d0c

brgix commented Aug 5, 2025

View reviewed changes

Fixes ScheduleInterval methods/tests

3f315a0

brgix commented Aug 6, 2025

View reviewed changes

Tweaks line intersection + tests

fa96a39

brgix commented Aug 9, 2025

View reviewed changes

Tweaks methods relying on OpenStudio::Point3dVector (vs Array)

5b9a052

brgix commented Aug 10, 2025

View reviewed changes

lib/osut/utils.rb

end

if holds?(a, pts[0])

if a.include?(pts[0])

Copy link

Member Author

brgix Aug 10, 2025

Choose a reason for hiding this comment

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

Quicker, same object ID.

brgix commented Aug 10, 2025

View reviewed changes

Fixes typos, invalid variables/access, etc.

7c289b3

brgix commented Aug 11, 2025

View reviewed changes

brgix added 2 commits August 11, 2025 09:00

Updates version

6ec6ce8

Renames getters - as per pyOSut

aab9a57

brgix commented Aug 11, 2025

View reviewed changes

Adds space width & height

c53fe28

brgix commented Aug 11, 2025

View reviewed changes

Adds 'realignFace' to 'spaceWidth'

a1a8612

brgix commented Aug 12, 2025

View reviewed changes

brgix added 3 commits August 12, 2025 08:18

Tweaks space width/height methods/tests

699d8aa

Tests candidate OSlg v040

b462885

Bumps version to v070

ba28585

brgix mentioned this pull request Aug 12, 2025

Integrates OSut/OSlg updates rd2/tbd#121

Merged

Pulls OSlg release v040

f2d5b55

brgix merged commit 4d49032 into develop Aug 14, 2025
7 checks passed

brgix deleted the v061 branch August 14, 2025 12:59

Conversation

brgix commented Aug 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brgix Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brgix Aug 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brgix Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

brgix Aug 6, 2025 •

edited

Loading

brgix Aug 9, 2025 •

edited

Loading

brgix Aug 11, 2025 •

edited

Loading

brgix Aug 12, 2025 •

edited

Loading