Skip to content

Commit 457b469

Browse files
Lutherwavesclaude
andauthored
feat: let one classification serve both the drawings and the quantities (#33)
* feat: let one classification serve both the drawings and the quantities Totalling a facade and drawing it are the same measurement asked twice, and goifc made a caller pay for it twice. `Scene.Elevations` builds the facing classification internally and keeps it; a caller who also wants per-direction quantities had to call `BuildFacings` again. On a 1,922-element model that is 15.3s of duplicated work on top of a 17.1s call. `Scene.ElevationsWith` takes the classification the caller already holds. Same drawings, pinned by TestElevationsWith_MatchesElevations. On kb645 the pair goes from 32.3s to 17.5s — the quantities now cost +0.4s rather than +15.3s. A nil or empty map is taken at FACE VALUE, not as a request to build one. An empty classification is a positive claim, and silently classifying there would restore the exact cost the call exists to avoid — invisibly, because the drawing would still look right. Removing that normalization makes TestElevationsWith_TakesAnEmptyClassificationAtFaceValue fail with 2 entities drawn where 0 were wanted. `ImportNode.OpeningDeduction` and `ImportNode.ProjectedGross` publish the two halves `NetArea` is the difference of. `NetArea` alone cannot be aggregated: `NetAreas` OMITS hosts with no IfcRelVoidsElement, so summing nets over a facade silently drops every solid wall. On kb645's exterior EPS that is 17 of 83 hosts and 151.6 m2 of face — summing nets gives 440.3 against 891.5 gross, a 51% opening ratio. Netting a total means subtracting the DEDUCTION from the gross being totalled: 626.5, a 30% ratio. `ProjectedGross` travels with it because the two are only usable as a pair. Both are measured on the host's winning projection axis, which foreshortens gross and deduction by the same factor, while `Facing.FaceArea` does not foreshorten at all. A caller netting an on-face gross must scale the deduction by their ratio rather than subtracting it raw; on kb645 that is 626.5 against 611.2, a 2.4% spread. Without `ProjectedGross` that bias is not merely uncorrected, it is invisible. Both are present exactly when `NetArea` is, gated on Net rather than on Trusted so the fields cannot disagree. An untrusted host publishing a zero deduction would satisfy a presence check and read as "this wall has no openings" — a refused measurement turned into a confident claim that the whole gross is net. Also restores the `## v0.8.1` heading. That release tagged and shipped without renaming its section, so its entry sat under "Unreleased" on main and the next release would have promoted it under the wrong version. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: distinguish a host with no openings from one whose netting was refused The three area fields are absent for two OPPOSITE reasons, and nothing on ImportNode said which. A host with no IfcRelVoidsElement is not in the reconciliation at all, so its net IS its gross; a host whose reconciliation was refused is in it with a nil Net, so its net is unknown. Both reach a consumer as the same nils. Netting a facade total forces the choice. Reading absence as "no openings" reports a fully-glazed wall as solid — the over-subtraction gate exists precisely because those walls are wrong. Reading it as "unknown" drops every solid wall, which on kb645's exterior EPS is 17 of 83 hosts and 151.6 m2. HasOpenings is presence in the NetAreas map, which is exactly the question. It is always meaningful, so a plain bool rather than a third nil-able field. kb645 has zero untrusted hosts among those 17 today, so both readings happen to agree on the model in front of us. That is what makes this cheap now and expensive to discover later. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: state the obliquity correction as a formula, not a direction-free ratio "Scale the deduction by their ratio" does not say which way the ratio goes, and inverting it produces a biased net that still looks plausible. Spell it out: net = FaceArea - OpeningDeduction*(FaceArea/ProjectedGross). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 39c29af commit 457b469

5 files changed

Lines changed: 349 additions & 3 deletions

File tree

CHANGELOG.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,45 @@ minor versions, as the README states. Releases before v0.2.0 predate this file.
1616

1717
### Added
1818

19+
- `Scene.ElevationsWith(f, r, planes, facings)``Scene.Elevations` over a
20+
classification the caller already holds. `BuildFacings` dominates the cost of
21+
drawing a facade set (on a ~1,900-element model, roughly 15s of a 17s call),
22+
and its result is worth more than the drawing: `Facing.FaceArea` binned by
23+
`Facing.Azimuth` is the sound way to total a facade, because summing the
24+
sheets is not — an element with two exposed sides is drawn on two
25+
perpendicular sheets, so per-sheet totals double-count it. A caller wanting
26+
both the drawings and the quantities previously had to classify the same scene
27+
twice. A nil or empty map is taken at face value, not as a request to build
28+
one: silently classifying there would restore the exact cost the call exists
29+
to avoid, invisibly, because the drawing would still look right.
30+
- `ImportNode.OpeningDeduction` and `ImportNode.ProjectedGross` — the two halves
31+
`NetArea` is the difference of, on the import contract. `NetArea` alone cannot
32+
be aggregated: a host with no `IfcRelVoidsElement` is ABSENT from the
33+
reconciliation, so summing nets over a facade silently drops every solid wall.
34+
Netting a total means subtracting the DEDUCTION from the gross being totalled.
35+
`ProjectedGross` comes with it because both are measured on the host's winning
36+
projection axis, which foreshortens gross and deduction by the same factor — so
37+
a caller netting an unforeshortened gross (`Facing.FaceArea`) subtracts
38+
`OpeningDeduction * (FaceArea / ProjectedGross)`, not `OpeningDeduction` raw.
39+
Without `ProjectedGross` that bias is not merely uncorrected, it is invisible. Present exactly when `NetArea` is,
40+
so an untrusted host never publishes a zero deduction that would read as "this
41+
wall has no openings".
42+
- `ImportNode.HasOpenings` — whether the element carries `IfcRelVoidsElement`
43+
openings at all. This is the fact the nil-able area fields cannot express:
44+
they are absent for two OPPOSITE reasons — the host has none, so its net
45+
equals its gross; or its reconciliation was refused, so its net is unknown.
46+
Reading absence as "no openings" reports a fully-glazed wall as solid;
47+
reading it as "unknown" drops every solid wall from a facade total. Always
48+
meaningful, so a plain bool rather than a third nil-able field.
49+
50+
## v0.8.1 — 2026-08-18
51+
52+
<!-- Restored by hand: v0.8.1 tagged and shipped without renaming its section,
53+
so its entry sat under "Unreleased" on main and the next release would have
54+
promoted it under the wrong version heading. -->
55+
56+
### Added
57+
1958
- `ImportNode.OpeningPerimeter` — the opening union's boundary length (m) on the
2059
import contract, so the measurement v0.8.0 added is actually reachable by
2160
consumers, which read `ImportNode` rather than `NetArea`. Present exactly when

geometry/elevation.go

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,10 +158,47 @@ func (s *Scene) ElevationOn(f *step.File, r *model.Result, p Plane) ElevationVie
158158
//
159159
// Deterministic: identical input yields identical views.
160160
func (s *Scene) Elevations(f *step.File, r *model.Result, planes []Plane) []ElevationView {
161+
return s.elevations(f, r, planes, nil)
162+
}
163+
164+
// ElevationsWith is [Scene.Elevations] over a classification the caller already
165+
// holds, so the two never pay for it twice.
166+
//
167+
// [BuildFacings] dominates the cost of drawing a set of elevations — on a
168+
// ~1,900-element model it is roughly 15s of a 17s call — and its result is
169+
// worth more than the drawing alone: FaceArea binned by Azimuth is the sound
170+
// way to total a facade, precisely because summing the sheets is not. An
171+
// element with two exposed sides is drawn on two perpendicular sheets, so
172+
// per-sheet totals double-count it; Azimuth bins it to exactly one direction.
173+
// A caller wanting both the drawings and the quantities would otherwise
174+
// classify the same scene twice.
175+
//
176+
// facings must have been built from THIS scene's elements — pass
177+
// BuildFacings(s.Elements) to get exactly what [Scene.Elevations] computes.
178+
// An element absent from the map is unclassified, and a nil or empty map is
179+
// taken at face value rather than as a request to build one: silently doing
180+
// the 15s build here would defeat the only reason to call this instead of
181+
// [Scene.Elevations].
182+
//
183+
// Deterministic: identical input yields identical views.
184+
func (s *Scene) ElevationsWith(f *step.File, r *model.Result, planes []Plane, facings map[string]Facing) []ElevationView {
185+
if facings == nil {
186+
// A nil map reads the same as an empty one on lookup, but the shared
187+
// loop uses nil as its "build it lazily" signal. Normalize so a caller
188+
// passing nil gets the unclassified scene they asked for, not a
189+
// surprise classification.
190+
facings = map[string]Facing{}
191+
}
192+
return s.elevations(f, r, planes, facings)
193+
}
194+
195+
// elevations is the shared loop. A nil facings means "build it lazily on the
196+
// first valid plane" — the [Scene.Elevations] contract, under which a set of
197+
// planes that are all invalid never triggers BuildFacings at all.
198+
func (s *Scene) elevations(f *step.File, r *model.Result, planes []Plane, facings map[string]Facing) []ElevationView {
161199
if len(planes) == 0 {
162200
return nil
163201
}
164-
var facings map[string]Facing
165202
views := make([]ElevationView, len(planes))
166203
for i, p := range planes {
167204
if !p.Valid() {

geometry/elevation_with_test.go

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
package geometry
2+
3+
import (
4+
"testing"
5+
)
6+
7+
// elevationWithPlanes is the north/east pair the sharing tests project onto.
8+
func elevationWithPlanes(t *testing.T) []Plane {
9+
t.Helper()
10+
return []Plane{
11+
mustElevationPlane(t, [3]float64{0, 1, 0}),
12+
mustElevationPlane(t, [3]float64{1, 0, 0}),
13+
}
14+
}
15+
16+
// TestElevationsWith_MatchesElevations pins the equivalence that makes sharing
17+
// safe: handed the classification Elevations would have built for itself,
18+
// ElevationsWith must draw exactly the same thing. If the two ever diverge, a
19+
// caller that shares one pass to also total the facade gets sheets that
20+
// disagree with the sheets every other caller sees, and nothing in the output
21+
// says so.
22+
func TestElevationsWith_MatchesElevations(t *testing.T) {
23+
s, f, r, _ := buildElevation(t, twoWalls, [3]float64{0, 1, 0})
24+
planes := elevationWithPlanes(t)
25+
26+
want := s.Elevations(f, r, planes)
27+
got := s.ElevationsWith(f, r, planes, BuildFacings(s.Elements))
28+
29+
if len(got) != len(want) {
30+
t.Fatalf("ElevationsWith = %d views, Elevations = %d", len(got), len(want))
31+
}
32+
for i := range want {
33+
if len(got[i].Entities) != len(want[i].Entities) {
34+
t.Fatalf("view %d: %d entities, want %d", i, len(got[i].Entities), len(want[i].Entities))
35+
}
36+
for j := range want[i].Entities {
37+
if got[i].Entities[j].GlobalID != want[i].Entities[j].GlobalID {
38+
t.Fatalf("view %d entity %d: GlobalID %q, want %q",
39+
i, j, got[i].Entities[j].GlobalID, want[i].Entities[j].GlobalID)
40+
}
41+
if got[i].Entities[j].Depth != want[i].Entities[j].Depth {
42+
t.Fatalf("view %d entity %d: Depth %v, want %v",
43+
i, j, got[i].Entities[j].Depth, want[i].Entities[j].Depth)
44+
}
45+
}
46+
if got[i].Bounds != want[i].Bounds {
47+
t.Fatalf("view %d: Bounds %v, want %v", i, got[i].Bounds, want[i].Bounds)
48+
}
49+
}
50+
}
51+
52+
// TestElevationsWith_TakesAnEmptyClassificationAtFaceValue guards the one way
53+
// this API can silently cost exactly what it exists to save. An empty map is a
54+
// positive claim — "nothing is classified" — and treating it as "classify it
55+
// for me" would restore the BuildFacings pass the caller came here to skip,
56+
// invisibly, because the drawing would still look right.
57+
//
58+
// An unclassified scene has no outward-facing elements to draw, so any entity
59+
// here means the map was rebuilt behind the caller's back. nil is covered
60+
// alongside empty: the two read identically on lookup, and only the shared
61+
// loop's lazy-build signal distinguishes them.
62+
func TestElevationsWith_TakesAnEmptyClassificationAtFaceValue(t *testing.T) {
63+
s, f, r, _ := buildElevation(t, twoWalls, [3]float64{0, 1, 0})
64+
planes := elevationWithPlanes(t)
65+
66+
if n := len(s.Elevations(f, r, planes)[0].Entities); n == 0 {
67+
t.Fatal("fixture draws nothing even when classified; the assertions below cannot discriminate")
68+
}
69+
70+
for _, tc := range []struct {
71+
name string
72+
facings map[string]Facing
73+
}{
74+
{"empty", map[string]Facing{}},
75+
{"nil", nil},
76+
} {
77+
for i, v := range s.ElevationsWith(f, r, planes, tc.facings) {
78+
if len(v.Entities) != 0 {
79+
t.Errorf("%s classification, view %d: drew %d entities, want 0 — "+
80+
"the map was rebuilt rather than taken at face value",
81+
tc.name, i, len(v.Entities))
82+
}
83+
}
84+
}
85+
}

import.go

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,46 @@ type ImportNode struct {
3131
// reconciliation, so a consumer can never read a confident perimeter beside
3232
// an absent net.
3333
OpeningPerimeter *float64
34+
// OpeningDeduction is the area (m²) of that same opening union, and
35+
// ProjectedGross is the host's own silhouette on the same plane — the two
36+
// halves NetArea is the difference of. Both are measured on the host's
37+
// winning projection axis, which is what makes them a matched pair.
38+
//
39+
// They are published because NetArea alone cannot be aggregated: hosts with
40+
// no IfcRelVoidsElement are ABSENT from the reconciliation entirely, so
41+
// summing NetArea over a facade silently drops every solid wall. Netting a
42+
// total therefore means subtracting the DEDUCTION from whatever gross the
43+
// caller is totalling, not summing the nets.
44+
//
45+
// That matters most when the gross being netted is measured differently.
46+
// [geometry.Facing].FaceArea is the true on-face area and does not
47+
// foreshorten, while these two are a projection and do. The projection
48+
// foreshortens gross and deduction by the SAME factor, so that factor is
49+
// recoverable as FaceArea/ProjectedGross and a caller netting an on-face
50+
// gross computes:
51+
//
52+
// net = FaceArea - OpeningDeduction*(FaceArea/ProjectedGross)
53+
//
54+
// NOT FaceArea - OpeningDeduction, which under-deducts by that factor.
55+
// Without ProjectedGross the bias is not merely uncorrected, it is
56+
// invisible.
57+
//
58+
// Present exactly when NetArea is, for the reason OpeningPerimeter is: all
59+
// four come from one trusted reconciliation. A zero deduction here means a
60+
// host whose openings measured zero, never an untrusted one.
61+
OpeningDeduction *float64
62+
ProjectedGross *float64
63+
// HasOpenings reports whether this element carries IfcRelVoidsElement
64+
// openings at all, which is the fact the three nil-able fields above CANNOT
65+
// express. They are absent for two opposite reasons — the host has no
66+
// openings, so its net equals its gross; or its reconciliation was refused,
67+
// so its net is unknown — and a consumer netting a total must tell those
68+
// apart. Reading absence as "no openings" reports a fully-glazed wall as
69+
// solid; reading it as "unknown" drops every solid wall from the total.
70+
//
71+
// Unlike the others this is always meaningful, so it is a plain bool: there
72+
// is no third state to encode.
73+
HasOpenings bool
3474

3575
// TypeGlobalID / TypeName / TypeClass identify the element's IfcTypeObject.
3676
// Empty when the element carries no IfcRelDefinesByType — most elements do.
@@ -236,12 +276,19 @@ func BuildImportFrom(f *step.File, a *Assembled) (*ImportModel, error) {
236276
n.BBoxMax = ge.BBoxMax
237277
}
238278
if na, ok := nets[e.GlobalID]; ok {
279+
// Presence in the map IS the openings fact: NetAreas keys only the
280+
// hosts that have them.
281+
n.HasOpenings = true
239282
n.NetArea = na.Net // already nil when untrusted
240283
if na.Net != nil {
241-
// Gated on Net rather than on Trusted directly, so the two
242-
// fields cannot disagree: an untrusted host publishes neither.
284+
// Gated on Net rather than on Trusted directly, so the fields
285+
// cannot disagree: an untrusted host publishes none of them.
243286
p := na.OpeningPerimeter
244287
n.OpeningPerimeter = &p
288+
d := na.OpeningDeduction
289+
n.OpeningDeduction = &d
290+
g := na.Gross
291+
n.ProjectedGross = &g
245292
}
246293
}
247294
nodes[i] = n

import_deduction_test.go

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
package ifc
2+
3+
import (
4+
"math"
5+
"testing"
6+
)
7+
8+
// TestBuildImport_DeductionAndGrossReachNetArea pins the reachability of the
9+
// two halves NetArea is the difference of. NetArea alone cannot be aggregated:
10+
// a host with no IfcRelVoidsElement is absent from the reconciliation, so
11+
// summing nets over a facade silently drops every solid wall. Netting a total
12+
// means subtracting the DEDUCTION from the gross being totalled, which is only
13+
// possible if both cross the import contract.
14+
//
15+
// The identity is the assertion that matters. ProjectedGross - OpeningDeduction
16+
// must equal NetArea exactly, because all three are read from one
17+
// reconciliation on one plane; if they ever disagree, the pair cannot be used
18+
// to net anything and a consumer would be quietly inventing area.
19+
func TestBuildImport_DeductionAndGrossReachNetArea(t *testing.T) {
20+
f := parseFixture(t, "geometry/testdata/synthetic/netarea_rect_window.ifc")
21+
m, err := BuildImport(f)
22+
if err != nil {
23+
t.Fatalf("BuildImport: %v", err)
24+
}
25+
26+
var withNet int
27+
for _, n := range m.Nodes {
28+
if (n.NetArea == nil) != (n.OpeningDeduction == nil) {
29+
t.Errorf("node %q: NetArea and OpeningDeduction disagree about presence "+
30+
"(net=%v deduction=%v)", n.Name, n.NetArea, n.OpeningDeduction)
31+
}
32+
if (n.NetArea == nil) != (n.ProjectedGross == nil) {
33+
t.Errorf("node %q: NetArea and ProjectedGross disagree about presence "+
34+
"(net=%v gross=%v)", n.Name, n.NetArea, n.ProjectedGross)
35+
}
36+
if n.NetArea == nil {
37+
continue
38+
}
39+
// Report the missing field and move on rather than dereferencing it: a
40+
// test that panics on the very regression it exists to catch reports a
41+
// stack trace instead of a diagnosis, and takes the remaining nodes'
42+
// assertions down with it.
43+
if n.OpeningDeduction == nil || n.ProjectedGross == nil {
44+
continue
45+
}
46+
withNet++
47+
48+
if got := math.Abs(*n.ProjectedGross - *n.OpeningDeduction - *n.NetArea); got > 1e-9 {
49+
t.Errorf("node %q: gross(%v) - deduction(%v) != net(%v), off by %v",
50+
n.Name, *n.ProjectedGross, *n.OpeningDeduction, *n.NetArea, got)
51+
}
52+
// The fixture is a 4x3 wall with a 1x1 void: 12 gross, 1 deducted.
53+
// Pins units and transport; the union semantics live in geometry's
54+
// own tests.
55+
if got := *n.ProjectedGross; got < 11.999 || got > 12.001 {
56+
t.Errorf("node %q: ProjectedGross = %v, want 12 (4x3 wall)", n.Name, got)
57+
}
58+
if got := *n.OpeningDeduction; got < 0.999 || got > 1.001 {
59+
t.Errorf("node %q: OpeningDeduction = %v, want 1 (1x1 void)", n.Name, got)
60+
}
61+
}
62+
if withNet == 0 {
63+
t.Fatal("fixture produced no node with a trusted NetArea; the assertions above never ran")
64+
}
65+
}
66+
67+
// TestBuildImport_UntrustedHostPublishesNoDeduction confirms the untrusted path
68+
// stays silent in the new fields too. NetArea.OpeningDeduction is documented as
69+
// 0 when untrusted, and a published 0 is the failure mode worth guarding: it
70+
// satisfies a presence check and reads as "this wall has no openings", turning
71+
// a refused measurement into a confident claim that the whole gross is net.
72+
func TestBuildImport_UntrustedHostPublishesNoDeduction(t *testing.T) {
73+
f := parseFixture(t, "geometry/testdata/synthetic/netarea_mostly_glass.ifc")
74+
m, err := BuildImport(f)
75+
if err != nil {
76+
t.Fatalf("BuildImport: %v", err)
77+
}
78+
var sawUntrusted int
79+
for _, n := range m.Nodes {
80+
if n.OpeningDeduction != nil || n.ProjectedGross != nil {
81+
t.Errorf("node %q: untrusted host published deduction=%v gross=%v, want both absent",
82+
n.Name, n.OpeningDeduction, n.ProjectedGross)
83+
}
84+
if n.HasOpenings {
85+
sawUntrusted++
86+
// The discriminator that makes the absence above readable. Without
87+
// it a consumer sees the same nils a solid wall produces and nets
88+
// this fully-glazed host at its whole gross.
89+
if n.NetArea != nil {
90+
t.Errorf("node %q: HasOpenings with a trusted net; fixture is meant to be refused", n.Name)
91+
}
92+
}
93+
}
94+
if sawUntrusted == 0 {
95+
t.Fatal("fixture produced no host with openings; the untrusted path was never exercised")
96+
}
97+
}
98+
99+
// TestBuildImport_UnvoidedHostIsDistinguishableFromUntrusted is the whole point
100+
// of HasOpenings. Both a solid wall and a refused reconciliation publish nil in
101+
// all three area fields, and they are OPPOSITE answers: the solid wall's net
102+
// equals its gross, the refused one's net is unknown. A consumer totalling a
103+
// facade has to branch on something, and this is the only thing to branch on.
104+
func TestBuildImport_UnvoidedHostIsDistinguishableFromUntrusted(t *testing.T) {
105+
// A wall WITH a trusted window: HasOpenings, and every area field present.
106+
var voided int
107+
for _, n := range buildImport(t, "geometry/testdata/synthetic/netarea_rect_window.ifc").Nodes {
108+
if !n.HasOpenings {
109+
continue
110+
}
111+
voided++
112+
if n.NetArea == nil {
113+
t.Errorf("node %q: HasOpenings but no net; this fixture's window is trusted", n.Name)
114+
}
115+
}
116+
if voided == 0 {
117+
t.Error("rect_window produced no host with openings; the true branch went untested")
118+
}
119+
120+
// A wall with NO openings: not in the reconciliation at all, so every area
121+
// field is nil — the same nils the untrusted fixture produces above. Only
122+
// HasOpenings separates "its net IS its gross" from "its net is unknown".
123+
var unvoided int
124+
for _, n := range buildImport(t, "model/testdata/synthetic/wall_no_openings.ifc").Nodes {
125+
if n.HasOpenings {
126+
t.Errorf("node %q: HasOpenings on a fixture with no voids", n.Name)
127+
continue
128+
}
129+
unvoided++
130+
if n.NetArea != nil || n.OpeningDeduction != nil || n.ProjectedGross != nil {
131+
t.Errorf("node %q: !HasOpenings yet published net=%v deduction=%v gross=%v",
132+
n.Name, n.NetArea, n.OpeningDeduction, n.ProjectedGross)
133+
}
134+
}
135+
if unvoided == 0 {
136+
t.Error("wall_no_openings produced no nodes; the false branch went untested")
137+
}
138+
}

0 commit comments

Comments
 (0)