diff --git a/examples/harbor-house/patterns.config b/examples/harbor-house/patterns.config index b29a4f8..6701040 100644 --- a/examples/harbor-house/patterns.config +++ b/examples/harbor-house/patterns.config @@ -284,4 +284,9 @@ staircase_max: 2 value_inside: 300.0 value_circulation: 50.0 value_outside: 100.0 -value_supported: 300.0 +# homemaker-py-ecx (DESIGN.md §39.19): an upper-level terrace is worth what +# outdoor space is worth (value_outside); it is the COST that differs by level +# (outside 10.0 vs outside_supported 110.0), not the value. At 300.0 -- the +# `value_inside` rate -- a terrace was worth more per square metre than a real +# internal room, which the owner ruled out. +value_supported: 100.0 diff --git a/examples/health-centre/patterns.config b/examples/health-centre/patterns.config index 59afba4..dd8f680 100644 --- a/examples/health-centre/patterns.config +++ b/examples/health-centre/patterns.config @@ -365,4 +365,9 @@ staircase_max: 2 value_inside: 300.0 value_circulation: 50.0 value_outside: 100.0 -value_supported: 300.0 +# homemaker-py-ecx (DESIGN.md §39.19): an upper-level terrace is worth what +# outdoor space is worth (value_outside); it is the COST that differs by level +# (outside 10.0 vs outside_supported 110.0), not the value. At 300.0 -- the +# `value_inside` rate -- a terrace was worth more per square metre than a real +# internal room, which the owner ruled out. +value_supported: 100.0 diff --git a/examples/maple-court/patterns.config b/examples/maple-court/patterns.config index 4f45234..059dd78 100644 --- a/examples/maple-court/patterns.config +++ b/examples/maple-court/patterns.config @@ -423,4 +423,9 @@ staircase_max: 2 value_inside: 300.0 value_circulation: 50.0 value_outside: 100.0 -value_supported: 300.0 +# homemaker-py-ecx (DESIGN.md §39.19): an upper-level terrace is worth what +# outdoor space is worth (value_outside); it is the COST that differs by level +# (outside 10.0 vs outside_supported 110.0), not the value. At 300.0 -- the +# `value_inside` rate -- a terrace was worth more per square metre than a real +# internal room, which the owner ruled out. +value_supported: 100.0 diff --git a/examples/programme-house/patterns.config b/examples/programme-house/patterns.config index 749eabc..618da72 100644 --- a/examples/programme-house/patterns.config +++ b/examples/programme-house/patterns.config @@ -99,4 +99,9 @@ staircase_max: 1 value_inside: 300.0 value_circulation: 50.0 value_outside: 100.0 -value_supported: 300.0 +# homemaker-py-ecx (DESIGN.md §39.19): an upper-level terrace is worth what +# outdoor space is worth (value_outside); it is the COST that differs by level +# (outside 10.0 vs outside_supported 110.0), not the value. At 300.0 -- the +# `value_inside` rate -- a terrace was worth more per square metre than a real +# internal room, which the owner ruled out. +value_supported: 100.0 diff --git a/src/homemaker_layout/fitness.py b/src/homemaker_layout/fitness.py index 768c555..8e0b14a 100644 --- a/src/homemaker_layout/fitness.py +++ b/src/homemaker_layout/fitness.py @@ -190,7 +190,19 @@ CONF_DEFAULTS: dict = { "value_inside": 300.0, "value_circulation": 50.0, "value_outside": 100.0, - "value_supported": 300.0, + # homemaker-py-ecx (DESIGN.md §39.19). Was 300.0 -- the same rate as + # `value_inside`, which made a roof terrace worth as much per square metre + # as a real internal room, and (after quality, which barely touches an + # outside leaf) 4.4x as much in practice. The owner's ruling is that a + # terrace must not be worth more per area than a room. + # + # Set to `value_outside`, not to some number that makes an inequality come + # out: outdoor space is worth the same to an occupant whatever level it sits + # on, and the real difference between a ground garden and a roof terrace is + # what it takes to BUILD -- which `cost` already says, `outside` 10.0 + # against `outside_supported` 110.0. Value describes worth, cost describes + # structure; the level belongs in the second. + "value_supported": 100.0, "storey_limit": 4, "storey_minimum": 2, "latitude": 53.3814, @@ -502,15 +514,24 @@ class Fitness: # well-lit room costs is already charged by `exterior_wall` and # `boundary_wall` in the cost model, so penalising it again in value # bills the same wall twice. - # homemaker-py-ecx (DESIGN.md §39.18): how a leaf's quality factors are - # combined. "product" (default) is stock. "geometric_mean" divides out + # homemaker-py-ecx (DESIGN.md §39.18/§39.19): how a leaf's quality + # factors are combined. "geometric_mean" is now the DEFAULT and + # "product" is the old stock behaviour, kept for comparison. + # The geometric mean divides out # how many questions the leaf was ASKED, because quality is a product # and an outside leaf is exempt from size, crinkliness and access while # a room is judged on all three -- so exemption alone buys a higher # quality, and quality multiplies the value rate. Fails are emitted per # factor inside evaluate_leaf, before any combining, so the fail set # cannot move either way. - self._quality_aggregate = str(self.conf("quality_aggregate") or "product") + # + # Made default ON with the §39.19 value_supported change, because the + # owner's ruling -- a terrace must not be worth more per square metre + # than a real internal room -- needs BOTH. Measured on the corpus: + # stock 4.39x, rate alone 1.46x, aggregation alone 2.23x, together + # 98.9 against 132.9 per m2, which is the ruling satisfied. + self._quality_aggregate = str( + self.conf("quality_aggregate") or "geometric_mean") if self._quality_aggregate not in ("product", "geometric_mean"): raise ValueError( f"unknown quality_aggregate: {self._quality_aggregate!r}") diff --git a/tests/test_fitness_aggregate.py b/tests/test_fitness_aggregate.py index bdfddec..adb4657 100644 --- a/tests/test_fitness_aggregate.py +++ b/tests/test_fitness_aggregate.py @@ -46,7 +46,7 @@ def test_exempt_factors_really_are_one(): the corpus rather than assumed from reading the code.""" checked = 0 for d, p in _artefacts(): - conf, cost = load_config(d) + conf, cost = load_config(d, overrides={"quality_aggregate": "product"}) fit = Fitness(conf, cost) seen = [] orig = Fitness.evaluate_leaf @@ -75,7 +75,7 @@ def test_exempt_factors_really_are_one(): def test_fail_set_is_byte_identical(): for d, p in _artefacts(): root = dom_mod.load(str(p)) - c_prod, cost = load_config(d) + c_prod, cost = load_config(d, overrides={"quality_aggregate": "product"}) c_geo, _ = load_config(d, overrides={"quality_aggregate": "geometric_mean"}) _, f_prod = Fitness(c_prod, cost).score_with_fails(copy.deepcopy(root)) _, f_geo = Fitness(c_geo, cost).score_with_fails(copy.deepcopy(root)) diff --git a/tests/test_terrace_value_ruling.py b/tests/test_terrace_value_ruling.py new file mode 100644 index 0000000..01cae5d --- /dev/null +++ b/tests/test_terrace_value_ruling.py @@ -0,0 +1,77 @@ +"""A terrace must not be worth more per square metre than a real internal room. + +Owner's ruling, DESIGN.md §39.19. It is an invariant of the objective rather +than a property of any one layout, and it needs BOTH halves of §39.18/§39.19 to +hold — the rate alone leaves a terrace 1.46x a room, the aggregation alone +2.23x. This asserts it end to end on the corpus, so that flipping either back +fails loudly here rather than quietly in a six-hour run. +""" + +from __future__ import annotations + +from collections import defaultdict +from pathlib import Path + +import pytest + +from homemaker_layout import dom as dom_mod, geometry +from homemaker_layout.fitness import Fitness, load_config + +EXAMPLES = Path(__file__).resolve().parent.parent / "examples" +CORPUS = ["harbor-house", "maple-court", "health-centre", "programme-house"] +pytestmark = pytest.mark.skipif(not (EXAMPLES / "harbor-house").is_dir(), + reason="examples absent") + + +def _value_per_m2(overrides=None): + """(room, terrace) mean value per m2 over the committed baseline layouts.""" + acc = defaultdict(lambda: [0.0, 0.0]) + for name in CORPUS: + d = EXAMPLES / name + for p in sorted(d.glob("coldstart-500000-s*.dom")): + conf, cost = load_config(d, overrides=overrides) + fit = Fitness(conf, cost) + orig = Fitness.evaluate_leaf + + def ev(self, leaf, G, level_id, groups, fail, _o=orig): + q, f = _o(self, leaf, G, level_id, groups, fail) + if dom_mod.is_outside(leaf): + kind = "terrace" if dom_mod.level_of(leaf) else None + elif dom_mod.is_circulation(leaf): + kind = None + else: + kind = "room" + if kind: + a = geometry.area(leaf) + acc[kind][0] += a + acc[kind][1] += q * self.value_rate(leaf) * a + return q, f + + Fitness.evaluate_leaf = ev + try: + fit.score_with_fails(dom_mod.load(str(p))) + finally: + Fitness.evaluate_leaf = orig + return tuple(acc[k][1] / acc[k][0] for k in ("room", "terrace")) + + +def test_a_terrace_is_worth_less_per_m2_than_a_room(): + room, terrace = _value_per_m2() + assert terrace < room, ( + f"terrace {terrace:.1f}/m2 >= room {room:.1f}/m2 — §39.19 ruling broken") + + +def test_the_rate_alone_would_not_be_enough(): + """Documents why the aggregation default moved with the rate: revert the + aggregation and the ruling breaks again, so neither half is optional.""" + room, terrace = _value_per_m2({"quality_aggregate": "product"}) + assert terrace > room, ( + "the product aggregation no longer violates the ruling — if that is a " + "real improvement, §39.19's reasoning needs revisiting") + + +def test_value_supported_is_the_outdoor_rate_not_the_indoor_one(): + for name in CORPUS: + conf, _ = load_config(EXAMPLES / name) + assert conf["value_supported"] == conf["value_outside"], name + assert conf["value_supported"] < conf["value_inside"], name