Recover most of the cpsat test-suite cost; keep the part that is real
38.20's cap fix took the suite from ~4.5 to ~10 min because the assign_cpsat tests now solve to optimality. Recovered to ~6.8 min. The bigger win was not the threading. The secondary-adjacency test ran the cpsat arm THREE times and averaged, and its own comment says why: the cpsat path "is not yet bit-reproducible (homemaker-py-fdp)", so one 10-seed aggregate could straddle greedy's deterministic value and the test was flaky by construction. fdp is fixed (38.15), so one pass says exactly what three did -- that was work spent papering over a bug that no longer exists. constructive_topology and _assign_adjacency_aware now forward an optional cpsat_limits=(time_limit_s, deterministic_limit); default None keeps solve_room_labels' defaults, so production is unchanged -- verified 24/24 harbor solves still OPTIMAL at the defaults. It is not a tuning knob: it exists so a test whose claim does not depend on optimality can economise. test_construction_assign_cpsat_yields_valid_seed asserts invariants only and uses it, 91s -> 53s. That test now also guards a real trap: too small a budget makes solve_room_labels return None, _assign_adjacency_aware falls back to greedy, and the test would pass while exercising nothing. It counts fallbacks and fails if any occur. The two quality comparisons keep the full budget deliberately -- their claims are about the optimum, and cheapening them would weaken what they assert. That is why the suite does not return to 4.5 min; the residue is the honest price of optimal deterministic solves. Also corrected a stale claim in the secondary-adjacency comment: it measures only "not adjacent to" fails and is not a claim that cpsat seeds better overall, which 38.20 measured markedly worse. Closes homemaker-py-7t1. Lint at parity (46); tests 384 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MJ84Feep79Hhm3E4zZJmnB
This commit is contained in:
parent
ac5913151a
commit
57d8391c52
4 changed files with 96 additions and 29 deletions
|
|
@ -97,7 +97,7 @@
|
||||||
{"_type":"issue","id":"homemaker-py-nyb","title":"High-locality topology operators (mutation + subtree crossover)","description":"DESIGN.md §5, §7 Phase 2, §8.4. Mutation moves: divide/undivide leaf, swap children, rotate cut, retype leaf, per-floor delta edits, storey add/delete (cf. Urb Mutate.pm — but geometry sliding belongs to the inner loop, not the operator set). Crossover: area-matched subtree exchange (a subtree = a contiguous region, so crossover is meaningful — Crossover.pm). Operators must be high-locality: small genome change =\u003e small phenotype change, so warm-started inner loops stay cheap.","acceptance_criteria":"Each operator produces valid genomes (oracle scores them without error); locality measured (mean fitness/geometry perturbation per operator)","status":"closed","priority":2,"issue_type":"feature","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-06-11T23:37:27Z","created_by":"Bruno Postle","updated_at":"2026-06-12T13:07:37Z","started_at":"2026-06-12T12:54:23Z","closed_at":"2026-06-12T13:07:37Z","close_reason":"operators.py lands: 7 mutations + area-matched crossover, valid-by-construction via genome.encode repair. 115/115 oracle-valid children; locality measured: geom-pert 0.07-0.33 per op, fitness-pert 0.68-0.99 (0.5^n cliff flags raw moves — warm restart + penalty reshaping confirmed load-bearing). Also fixed dom._link stale below-links on structural mutation.","dependencies":[{"issue_id":"homemaker-py-nyb","depends_on_id":"homemaker-py-k2g","type":"blocks","created_at":"2026-06-12T00:39:36Z","created_by":"Bruno Postle","metadata":"{}"}],"dependency_count":1,"dependent_count":1,"comment_count":0}
|
{"_type":"issue","id":"homemaker-py-nyb","title":"High-locality topology operators (mutation + subtree crossover)","description":"DESIGN.md §5, §7 Phase 2, §8.4. Mutation moves: divide/undivide leaf, swap children, rotate cut, retype leaf, per-floor delta edits, storey add/delete (cf. Urb Mutate.pm — but geometry sliding belongs to the inner loop, not the operator set). Crossover: area-matched subtree exchange (a subtree = a contiguous region, so crossover is meaningful — Crossover.pm). Operators must be high-locality: small genome change =\u003e small phenotype change, so warm-started inner loops stay cheap.","acceptance_criteria":"Each operator produces valid genomes (oracle scores them without error); locality measured (mean fitness/geometry perturbation per operator)","status":"closed","priority":2,"issue_type":"feature","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-06-11T23:37:27Z","created_by":"Bruno Postle","updated_at":"2026-06-12T13:07:37Z","started_at":"2026-06-12T12:54:23Z","closed_at":"2026-06-12T13:07:37Z","close_reason":"operators.py lands: 7 mutations + area-matched crossover, valid-by-construction via genome.encode repair. 115/115 oracle-valid children; locality measured: geom-pert 0.07-0.33 per op, fitness-pert 0.68-0.99 (0.5^n cliff flags raw moves — warm restart + penalty reshaping confirmed load-bearing). Also fixed dom._link stale below-links on structural mutation.","dependencies":[{"issue_id":"homemaker-py-nyb","depends_on_id":"homemaker-py-k2g","type":"blocks","created_at":"2026-06-12T00:39:36Z","created_by":"Bruno Postle","metadata":"{}"}],"dependency_count":1,"dependent_count":1,"comment_count":0}
|
||||||
{"_type":"issue","id":"homemaker-py-k2g","title":"Topology genome: base-floor tree + per-floor deltas + type assignment","description":"DESIGN.md §5.2, §7 Phase 2. Genome = base-floor slicing topology (primary) + per-leaf type assignment + per-floor divide/undivide deltas (Below-inheritance as regulariser; cut owned by lowest storey where its path is divided — §10). Must round-trip to/from dom.py Node trees so the oracle and inner loop consume it directly. Includes storey count and per-floor type overrides.","acceptance_criteria":"Genome \u003c-\u003e .dom round-trip on all 35 corpus files preserves fitness; multi-storey wall stacking preserved","status":"closed","priority":2,"issue_type":"feature","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-06-11T23:37:26Z","created_by":"Bruno Postle","updated_at":"2026-06-12T12:52:34Z","started_at":"2026-06-12T10:55:21Z","closed_at":"2026-06-12T12:52:34Z","close_reason":"genome.py encode/decode lands. 35/35 oracle fitness parity after round-trip (flag-on); genome fixed-point + owned-projection tests. Dead-field discovery: corpus upper storeys carry drifted dead divisions (97) and rotations (187) — canonicalised by decode, validated fitness-neutral.","dependency_count":0,"dependent_count":1,"comment_count":0}
|
{"_type":"issue","id":"homemaker-py-k2g","title":"Topology genome: base-floor tree + per-floor deltas + type assignment","description":"DESIGN.md §5.2, §7 Phase 2. Genome = base-floor slicing topology (primary) + per-leaf type assignment + per-floor divide/undivide deltas (Below-inheritance as regulariser; cut owned by lowest storey where its path is divided — §10). Must round-trip to/from dom.py Node trees so the oracle and inner loop consume it directly. Includes storey count and per-floor type overrides.","acceptance_criteria":"Genome \u003c-\u003e .dom round-trip on all 35 corpus files preserves fitness; multi-storey wall stacking preserved","status":"closed","priority":2,"issue_type":"feature","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-06-11T23:37:26Z","created_by":"Bruno Postle","updated_at":"2026-06-12T12:52:34Z","started_at":"2026-06-12T10:55:21Z","closed_at":"2026-06-12T12:52:34Z","close_reason":"genome.py encode/decode lands. 35/35 oracle fitness parity after round-trip (flag-on); genome fixed-point + owned-projection tests. Dead-field discovery: corpus upper storeys carry drifted dead divisions (97) and rotations (187) — canonicalised by decode, validated fitness-neutral.","dependency_count":0,"dependent_count":1,"comment_count":0}
|
||||||
{"_type":"issue","id":"homemaker-py-d0s","title":"Experiment: inner-loop optimiser bake-off at equal oracle budgets","description":"DESIGN.md §7 Phase 1, §8.3. DOF is only ~rooms-1 (6–7 on corpus). Compare Nelder-Mead vs CMA-ES vs batched multi-start pattern search at equal oracle-call budgets, measuring fitness gained per oracle call and wall-clock (batch-friendliness matters — §4.6). Measure, don't commit blind.","acceptance_criteria":"Table of fitness-per-budget across \u003e=3 candidates; one optimiser chosen and recorded in DESIGN.md","status":"closed","priority":2,"issue_type":"task","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-06-11T23:36:59Z","created_by":"Bruno Postle","updated_at":"2026-06-13T08:48:13Z","started_at":"2026-06-12T21:22:15Z","closed_at":"2026-06-13T08:48:13Z","close_reason":"Bake-off complete: CMA-ES confirmed as Phase 1/2 optimiser. NM wins quality per eval but sequential architecture incompatible with batching (§4.6). Compass stalls on narrow valleys. Results in DESIGN.md §8.3 and experiments/bakeoff_innerloop.*","dependencies":[{"issue_id":"homemaker-py-d0s","depends_on_id":"homemaker-py-1p0","type":"blocks","created_at":"2026-06-12T00:39:35Z","created_by":"Bruno Postle","metadata":"{}"}],"dependency_count":1,"dependent_count":0,"comment_count":0}
|
{"_type":"issue","id":"homemaker-py-d0s","title":"Experiment: inner-loop optimiser bake-off at equal oracle budgets","description":"DESIGN.md §7 Phase 1, §8.3. DOF is only ~rooms-1 (6–7 on corpus). Compare Nelder-Mead vs CMA-ES vs batched multi-start pattern search at equal oracle-call budgets, measuring fitness gained per oracle call and wall-clock (batch-friendliness matters — §4.6). Measure, don't commit blind.","acceptance_criteria":"Table of fitness-per-budget across \u003e=3 candidates; one optimiser chosen and recorded in DESIGN.md","status":"closed","priority":2,"issue_type":"task","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-06-11T23:36:59Z","created_by":"Bruno Postle","updated_at":"2026-06-13T08:48:13Z","started_at":"2026-06-12T21:22:15Z","closed_at":"2026-06-13T08:48:13Z","close_reason":"Bake-off complete: CMA-ES confirmed as Phase 1/2 optimiser. NM wins quality per eval but sequential architecture incompatible with batching (§4.6). Compass stalls on narrow valleys. Results in DESIGN.md §8.3 and experiments/bakeoff_innerloop.*","dependencies":[{"issue_id":"homemaker-py-d0s","depends_on_id":"homemaker-py-1p0","type":"blocks","created_at":"2026-06-12T00:39:35Z","created_by":"Bruno Postle","metadata":"{}"}],"dependency_count":1,"dependent_count":0,"comment_count":0}
|
||||||
{"_type":"issue","id":"homemaker-py-7t1","title":"cpsat solver limits are not threadable from constructive_topology, so tests pay full solve cost","description":"homemaker-py-vjd raised solve_room_labels' wall-clock backstop from 2.0s to 30.0s so the deterministic budget governs (it had silently become the binding, load-dependent cap -- 2 of 24 harbor solves returned FEASIBLE not OPTIMAL). Correct, but the full test suite went from ~4.5 min to ~10 min: the three assign_cpsat tests now dominate it, because the solves that used to be truncated at 2s run to optimality.\\n\\nThe tests cannot opt out. operators.constructive_topology takes assign_solver but does not thread time_limit_s / deterministic_limit through to cpsat.solve_room_labels, so there is no way to ask for a cheaper solve in a test.\\n\\nThread the two limits through constructive_topology (and _assign_adjacency_aware) as optional parameters defaulting to the current values, then have the assign_cpsat tests pass a smaller deterministic_limit. The DEFAULT must not change -- the point is that production solves run to optimality deterministically; only the tests should economise.","acceptance_criteria":"constructive_topology accepts and forwards the solver limits; the assign_cpsat tests use a reduced deterministic_limit and the suite returns to roughly its previous runtime; default limits unchanged and all solves still OPTIMAL in the vjd measurement.","status":"open","priority":3,"issue_type":"task","owner":"noreply@anthropic.com","created_at":"2026-08-29T14:47:36Z","created_by":"Claude","updated_at":"2026-08-29T14:47:36Z","dependency_count":0,"dependent_count":0,"comment_count":0}
|
{"_type":"issue","id":"homemaker-py-7t1","title":"cpsat solver limits are not threadable from constructive_topology, so tests pay full solve cost","description":"homemaker-py-vjd raised solve_room_labels' wall-clock backstop from 2.0s to 30.0s so the deterministic budget governs (it had silently become the binding, load-dependent cap -- 2 of 24 harbor solves returned FEASIBLE not OPTIMAL). Correct, but the full test suite went from ~4.5 min to ~10 min: the three assign_cpsat tests now dominate it, because the solves that used to be truncated at 2s run to optimality.\\n\\nThe tests cannot opt out. operators.constructive_topology takes assign_solver but does not thread time_limit_s / deterministic_limit through to cpsat.solve_room_labels, so there is no way to ask for a cheaper solve in a test.\\n\\nThread the two limits through constructive_topology (and _assign_adjacency_aware) as optional parameters defaulting to the current values, then have the assign_cpsat tests pass a smaller deterministic_limit. The DEFAULT must not change -- the point is that production solves run to optimality deterministically; only the tests should economise.","acceptance_criteria":"constructive_topology accepts and forwards the solver limits; the assign_cpsat tests use a reduced deterministic_limit and the suite returns to roughly its previous runtime; default limits unchanged and all solves still OPTIMAL in the vjd measurement.","status":"closed","priority":3,"issue_type":"task","assignee":"Claude","owner":"noreply@anthropic.com","created_at":"2026-08-29T14:47:36Z","created_by":"Claude","updated_at":"2026-08-29T18:55:52Z","started_at":"2026-08-29T18:34:05Z","closed_at":"2026-08-29T18:55:52Z","close_reason":"Done. Suite ~10 min -\u003e ~6.8 min, with the remaining cost identified as genuine.\n\nTwo recoveries, and one deliberate non-recovery:\n\n1. test_assign_cpsat_matches_or_beats_greedy_secondary_adjacency ran the cpsat\n arm THREE times and averaged. Its own comment gave the reason: the cpsat path\n \"is not yet bit-reproducible (homemaker-py-fdp)\", so a single 10-seed\n aggregate could straddle greedy's deterministic value and the test was flaky\n by construction. fdp is fixed (§38.15), so one pass says exactly what three\n did. That was work spent papering over a bug that no longer exists -- worth\n more than the threading itself.\n\n2. constructive_topology and _assign_adjacency_aware now forward an optional\n cpsat_limits=(time_limit_s, deterministic_limit); default None keeps\n solve_room_labels' own defaults, so PRODUCTION IS UNCHANGED (verified: 24/24\n harbor solves still OPTIMAL at the defaults). It is not a tuning knob -- it\n exists so a test whose claim does not depend on optimality can economise.\n test_construction_assign_cpsat_yields_valid_seed asserts invariants only and\n uses it: 91s -\u003e 53s.\n\n Guarded a real trap there: too small a budget makes solve_room_labels return\n None, _assign_adjacency_aware falls back to GREEDY, and the test would pass\n while exercising nothing. It now counts fallbacks and fails if any occur.\n\n3. The two QUALITY comparisons keep the full budget deliberately. Their claims\n are about the optimum; cheapening them would weaken what they assert. That is\n why the suite does not return all the way to 4.5 min -- the residue is the\n honest price of solves reaching optimality deterministically, and should not\n be optimised away.\n\nAlso corrected a stale claim in the secondary-adjacency test's comment: it now\nnotes it measures ONLY \"not adjacent to\" fails, and is not a claim that cpsat\nseeds better overall -- §38.20 measured it markedly worse on total fails.\n","dependency_count":0,"dependent_count":0,"comment_count":0}
|
||||||
{"_type":"issue","id":"homemaker-py-5nw","title":"health-centre has no dedicated WC for the treatment room, so the Brand credit cannot be declared","description":"Split out of homemaker-py-3qj (DESIGN.md §38.14). The owner ruled that a treatment room 'may give access to a toilet, but this would be a dedicated toilet'. health-centre offers only t9 Public WC (count 2) and t10 Staff WC; declaring either adjacent to tr1 would make a public or staff WC dedicated to the treatment room, contradicting the ruling.\\n\\nEarning the adaptability credit here means ADDING a dedicated WC to the brief -- a new space with its own size/usage/adjacency -- which is programme authoring, not a config tweak, and wants the owner's decision on whether the building should have one.\\n\\nRelated: maple-court's level 1 carries four Neighborhoods and no toilet on that level at all, which may be a genuine gap in that brief too.","acceptance_criteria":"Either a dedicated WC is added to health-centre's programme and declared adjacent to tr1, or it is recorded as a deliberate omission; same question answered for maple-court's level 1.","status":"open","priority":3,"issue_type":"task","owner":"noreply@anthropic.com","created_at":"2026-08-29T10:57:27Z","created_by":"Claude","updated_at":"2026-08-29T10:57:27Z","dependency_count":0,"dependent_count":0,"comment_count":0}
|
{"_type":"issue","id":"homemaker-py-5nw","title":"health-centre has no dedicated WC for the treatment room, so the Brand credit cannot be declared","description":"Split out of homemaker-py-3qj (DESIGN.md §38.14). The owner ruled that a treatment room 'may give access to a toilet, but this would be a dedicated toilet'. health-centre offers only t9 Public WC (count 2) and t10 Staff WC; declaring either adjacent to tr1 would make a public or staff WC dedicated to the treatment room, contradicting the ruling.\\n\\nEarning the adaptability credit here means ADDING a dedicated WC to the brief -- a new space with its own size/usage/adjacency -- which is programme authoring, not a config tweak, and wants the owner's decision on whether the building should have one.\\n\\nRelated: maple-court's level 1 carries four Neighborhoods and no toilet on that level at all, which may be a genuine gap in that brief too.","acceptance_criteria":"Either a dedicated WC is added to health-centre's programme and declared adjacent to tr1, or it is recorded as a deliberate omission; same question answered for maple-court's level 1.","status":"open","priority":3,"issue_type":"task","owner":"noreply@anthropic.com","created_at":"2026-08-29T10:57:27Z","created_by":"Claude","updated_at":"2026-08-29T10:57:27Z","dependency_count":0,"dependent_count":0,"comment_count":0}
|
||||||
{"_type":"issue","id":"homemaker-py-3i3","title":"Is a missing required room correctly weighted at 1/32 of fitness?","description":"Split out of homemaker-py-1i8 (DESIGN.md §38.12), which deliberately fixed only the verbosity dependence and left the magnitude untouched.\n\nA missing required room now costs a fixed 5 fails, so under value *= 0.5 ** len(failures) it is a 1/32 penalty -- 32x the weight of a single crinkliness or proportion fail. Nobody has ever checked that ratio against intent. It may well be right (a missing room is a brief not met, arguably the most serious defect available), but it is currently an accident of how the cascade was built rather than a decision.\n\nThe question is what the objective should SAY, so it wants an owner ruling first and a measurement second: does the cascade's weight crowd out geometry-quality signal in the region where the search actually operates? Note most corpus layouts carry several missing instances at once, so the compounding is steep -- three missing rooms is 1/32768.","acceptance_criteria":"A recorded decision on the intended weight of a missing required room relative to a quality fail, with the cascade's fixed count set to match it; if the answer is 'as-is', that is recorded as a decision rather than left as an accident.","status":"open","priority":3,"issue_type":"task","owner":"noreply@anthropic.com","created_at":"2026-08-29T09:33:24Z","created_by":"Claude","updated_at":"2026-08-29T09:33:24Z","dependency_count":0,"dependent_count":0,"comment_count":0}
|
{"_type":"issue","id":"homemaker-py-3i3","title":"Is a missing required room correctly weighted at 1/32 of fitness?","description":"Split out of homemaker-py-1i8 (DESIGN.md §38.12), which deliberately fixed only the verbosity dependence and left the magnitude untouched.\n\nA missing required room now costs a fixed 5 fails, so under value *= 0.5 ** len(failures) it is a 1/32 penalty -- 32x the weight of a single crinkliness or proportion fail. Nobody has ever checked that ratio against intent. It may well be right (a missing room is a brief not met, arguably the most serious defect available), but it is currently an accident of how the cascade was built rather than a decision.\n\nThe question is what the objective should SAY, so it wants an owner ruling first and a measurement second: does the cascade's weight crowd out geometry-quality signal in the region where the search actually operates? Note most corpus layouts carry several missing instances at once, so the compounding is steep -- three missing rooms is 1/32768.","acceptance_criteria":"A recorded decision on the intended weight of a missing required room relative to a quality fail, with the cascade's fixed count set to match it; if the answer is 'as-is', that is recorded as a decision rather than left as an accident.","status":"open","priority":3,"issue_type":"task","owner":"noreply@anthropic.com","created_at":"2026-08-29T09:33:24Z","created_by":"Claude","updated_at":"2026-08-29T09:33:24Z","dependency_count":0,"dependent_count":0,"comment_count":0}
|
||||||
{"_type":"issue","id":"homemaker-py-1ue","title":"test_classify_fail_tier_covers_full_corpus passes only on a machine that has already run the scorer","description":"The test globs examples/**/*.fails and asserts checked \u003e 0, but .fails files are generated artefacts written by homemaker-fitness next to the .dom being scored. A clean checkout has none, so the test fails with 'assert 0 \u003e 0' -- it is asserting on the state of the developer's working tree, not on the code.\n\nSplit the two things it is doing: (1) that classify_fail_tier accepts every fail string the evaluator can emit, which should run against fail strings generated in-test from corpus .dom files (or a checked-in fixture list), and (2) the corpus-artefact sweep, which if it is worth keeping should skip rather than fail when no .fails exist.","acceptance_criteria":"The test passes in a fresh clone with no .fails present, and still fails if classify_fail_tier is given an unclassifiable fail string.","status":"closed","priority":3,"issue_type":"bug","owner":"noreply@anthropic.com","created_at":"2026-08-28T22:41:25Z","created_by":"Claude","updated_at":"2026-08-29T07:11:24Z","closed_at":"2026-08-29T07:11:24Z","close_reason":"Fixed. The test globbed examples/**/*.fails and asserted checked \u003e 0, but git\ntracks ZERO .fails files -- they are artefacts homemaker-fitness writes beside a\n.dom. Its own docstring claimed to cover \"every fail string ever emitted into a\nchecked-in native .fails file\", describing files that by design never exist in\nthe repo, so it could only ever pass on a machine that had already run the\nscorer, and failed `assert 0 \u003e 0` in a fresh clone.\n\nSplit into the three things it was conflating:\n\n test_classify_fail_tier_covers_every_fail_the_evaluator_emits\n GENERATES fails by scoring three corpus layouts chosen for breadth of\n failure kinds (harbor evolved-3M-nols-3 + generated, maple generated),\n and requires at least 8 distinct kinds so it cannot silently narrow.\n test_classify_fail_tier_rejects_an_unknown_fail_string\n the guard is only worth something if an unclassifiable string raises.\n test_classify_fail_tier_checks_any_native_fails_artefacts_present\n still sweeps .fails when a working tree happens to carry them, but never\n requires them.\n\nVerified by moving every .fails out of the tree and re-running: passes with none\npresent. Full suite now 376 passed, 0 failed.\n","dependency_count":0,"dependent_count":0,"comment_count":0}
|
{"_type":"issue","id":"homemaker-py-1ue","title":"test_classify_fail_tier_covers_full_corpus passes only on a machine that has already run the scorer","description":"The test globs examples/**/*.fails and asserts checked \u003e 0, but .fails files are generated artefacts written by homemaker-fitness next to the .dom being scored. A clean checkout has none, so the test fails with 'assert 0 \u003e 0' -- it is asserting on the state of the developer's working tree, not on the code.\n\nSplit the two things it is doing: (1) that classify_fail_tier accepts every fail string the evaluator can emit, which should run against fail strings generated in-test from corpus .dom files (or a checked-in fixture list), and (2) the corpus-artefact sweep, which if it is worth keeping should skip rather than fail when no .fails exist.","acceptance_criteria":"The test passes in a fresh clone with no .fails present, and still fails if classify_fail_tier is given an unclassifiable fail string.","status":"closed","priority":3,"issue_type":"bug","owner":"noreply@anthropic.com","created_at":"2026-08-28T22:41:25Z","created_by":"Claude","updated_at":"2026-08-29T07:11:24Z","closed_at":"2026-08-29T07:11:24Z","close_reason":"Fixed. The test globbed examples/**/*.fails and asserted checked \u003e 0, but git\ntracks ZERO .fails files -- they are artefacts homemaker-fitness writes beside a\n.dom. Its own docstring claimed to cover \"every fail string ever emitted into a\nchecked-in native .fails file\", describing files that by design never exist in\nthe repo, so it could only ever pass on a machine that had already run the\nscorer, and failed `assert 0 \u003e 0` in a fresh clone.\n\nSplit into the three things it was conflating:\n\n test_classify_fail_tier_covers_every_fail_the_evaluator_emits\n GENERATES fails by scoring three corpus layouts chosen for breadth of\n failure kinds (harbor evolved-3M-nols-3 + generated, maple generated),\n and requires at least 8 distinct kinds so it cannot silently narrow.\n test_classify_fail_tier_rejects_an_unknown_fail_string\n the guard is only worth something if an unclassifiable string raises.\n test_classify_fail_tier_checks_any_native_fails_artefacts_present\n still sweeps .fails when a working tree happens to carry them, but never\n requires them.\n\nVerified by moving every .fails out of the tree and re-running: passes with none\npresent. Full suite now 376 passed, 0 failed.\n","dependency_count":0,"dependent_count":0,"comment_count":0}
|
||||||
|
|
|
||||||
33
DESIGN.md
33
DESIGN.md
|
|
@ -5946,13 +5946,32 @@ changed — §39.4, §38.10–§38.12 — so these numbers are not directly comp
|
||||||
that table. The greedy-vs-cpsat comparison within this measurement is
|
that table. The greedy-vs-cpsat comparison within this measurement is
|
||||||
like-for-like and is what the verdict rests on.)*
|
like-for-like and is what the verdict rests on.)*
|
||||||
|
|
||||||
**Cost of the cap fix.** The two harbor solves that used to be cut off at 2 s now
|
**Cost of the cap fix, and what was recovered** (`homemaker-py-7t1`). The two
|
||||||
run to optimality, and the full test suite goes from ~4.5 min to ~10 min because
|
harbor solves that used to be cut off at 2 s now run to optimality, which took
|
||||||
the `assign_cpsat` tests dominate it. That is correctness bought with wall time,
|
the suite from ~4.5 min to ~10 min — the `assign_cpsat` tests dominate it. Two
|
||||||
and it is the right trade for a default, but the tests should not be paying it —
|
things recovered most of that, and one deliberately was not:
|
||||||
they cannot currently pass a smaller `deterministic_limit` because
|
|
||||||
`constructive_topology` does not thread the solver's limits through
|
- `test_assign_cpsat_matches_or_beats_greedy_secondary_adjacency` ran the cpsat
|
||||||
(`homemaker-py-7t1`).
|
arm **three times and averaged**, for a reason its own comment gives: the
|
||||||
|
cpsat path "is not yet bit-reproducible (`homemaker-py-fdp`)", so one 10-seed
|
||||||
|
aggregate could straddle greedy's deterministic value and the test was flaky
|
||||||
|
by construction. **`fdp` is fixed** (§38.15), so one pass now says exactly what
|
||||||
|
three did. The repeat was work spent papering over a bug that no longer exists.
|
||||||
|
- `constructive_topology` and `_assign_adjacency_aware` now forward an optional
|
||||||
|
`cpsat_limits=(time_limit_s, deterministic_limit)`. It is **not a tuning
|
||||||
|
knob** — production keeps the defaults so solves stay optimal and
|
||||||
|
deterministic. It exists so a test whose claim does not depend on optimality
|
||||||
|
can buy its runtime back. `test_construction_assign_cpsat_yields_valid_seed`
|
||||||
|
asserts invariants (every required space present, canonical genome) and uses
|
||||||
|
it: 91 s → 53 s. The trap there is that too small a budget makes
|
||||||
|
`solve_room_labels` return `None`, `_assign_adjacency_aware` falls back to
|
||||||
|
**greedy**, and the test passes while exercising nothing — so it counts
|
||||||
|
fallbacks and fails if any occur.
|
||||||
|
- The two *quality* comparisons keep the full budget. Their claims are about
|
||||||
|
the optimum, and cheapening them would weaken what they assert.
|
||||||
|
|
||||||
|
Net: ~10 min → ~6.8 min. Still above the pre-§38.20 4.5 min, and that residue is
|
||||||
|
the honest price of solves that now reach optimality deterministically.
|
||||||
|
|
||||||
**No default changes.** `assign_solver` was already default `greedy` for the
|
**No default changes.** `assign_solver` was already default `greedy` for the
|
||||||
independent reason §37.7 gives, and this reinforces it. What changes is that the
|
independent reason §37.7 gives, and this reinforces it. What changes is that the
|
||||||
|
|
|
||||||
|
|
@ -1111,7 +1111,9 @@ def _assign_adjacency_aware(lvl: dom.Node, room_codes: list[str], reqs,
|
||||||
n_outside: int = 1,
|
n_outside: int = 1,
|
||||||
scope: "set[dom.Node] | None" = None,
|
scope: "set[dom.Node] | None" = None,
|
||||||
beam_width: int = 1,
|
beam_width: int = 1,
|
||||||
assign_solver: str = "greedy") -> None:
|
assign_solver: str = "greedy",
|
||||||
|
cpsat_limits: "tuple[float, float] | None" = None,
|
||||||
|
) -> None:
|
||||||
"""Assign leaf types so rooms cluster around a connected circulation spine.
|
"""Assign leaf types so rooms cluster around a connected circulation spine.
|
||||||
|
|
||||||
s44 (DESIGN.md §11.2 follow-up): random type assignment leaves rooms stranded
|
s44 (DESIGN.md §11.2 follow-up): random type assignment leaves rooms stranded
|
||||||
|
|
@ -1162,6 +1164,12 @@ def _assign_adjacency_aware(lvl: dom.Node, room_codes: list[str], reqs,
|
||||||
unaffected either way. Falls through to the greedy/beam path on any
|
unaffected either way. Falls through to the greedy/beam path on any
|
||||||
solver failure (OR-Tools unavailable, infeasible, or timeout), so
|
solver failure (OR-Tools unavailable, infeasible, or timeout), so
|
||||||
behaviour is always defined.
|
behaviour is always defined.
|
||||||
|
|
||||||
|
``cpsat_limits`` (homemaker-py-7t1): optional ``(time_limit_s,
|
||||||
|
deterministic_limit)`` forwarded to :func:`cpsat.solve_room_labels`. ``None``
|
||||||
|
keeps that function's defaults, which is what production uses -- solves run
|
||||||
|
to optimality under a load-independent work-unit budget (§38.20). It exists
|
||||||
|
so a TEST can ask for a cheaper solve; it is not a tuning knob.
|
||||||
"""
|
"""
|
||||||
from . import geometry
|
from . import geometry
|
||||||
|
|
||||||
|
|
@ -1278,7 +1286,10 @@ def _assign_adjacency_aware(lvl: dom.Node, room_codes: list[str], reqs,
|
||||||
neighbors = {L: {nb for nb in _nbrs(L) if nb in room_set} for L in room_slots}
|
neighbors = {L: {nb for nb in _nbrs(L) if nb in room_set} for L in room_slots}
|
||||||
context_types = {L: {nb.type for nb in _nbrs(L) if nb.type and nb not in room_set}
|
context_types = {L: {nb.type for nb in _nbrs(L) if nb.type and nb not in room_set}
|
||||||
for L in room_slots}
|
for L in room_slots}
|
||||||
placed = cpsat.solve_room_labels(room_slots, codes, reqs, neighbors, context_types)
|
_lim = {} if cpsat_limits is None else dict(
|
||||||
|
zip(("time_limit_s", "deterministic_limit"), cpsat_limits))
|
||||||
|
placed = cpsat.solve_room_labels(room_slots, codes, reqs, neighbors,
|
||||||
|
context_types, **_lim)
|
||||||
|
|
||||||
if placed is not None:
|
if placed is not None:
|
||||||
for leaf, code in placed.items():
|
for leaf, code in placed.items():
|
||||||
|
|
@ -1415,6 +1426,7 @@ def constructive_topology(seed_root: dom.Node, reqs, rng: np.random.Generator,
|
||||||
construction_beam_width: int = 1,
|
construction_beam_width: int = 1,
|
||||||
multi_use: bool = False,
|
multi_use: bool = False,
|
||||||
assign_solver: str = "greedy",
|
assign_solver: str = "greedy",
|
||||||
|
cpsat_limits: "tuple[float, float] | None" = None,
|
||||||
repair_circulation: bool = False,
|
repair_circulation: bool = False,
|
||||||
preserve_circulation: bool = False) -> dom.Node:
|
preserve_circulation: bool = False) -> dom.Node:
|
||||||
"""Build a seed that instantiates every required space by construction.
|
"""Build a seed that instantiates every required space by construction.
|
||||||
|
|
@ -1502,7 +1514,8 @@ def constructive_topology(seed_root: dom.Node, reqs, rng: np.random.Generator,
|
||||||
_assign_adjacency_aware(lvl, rooms, reqs, rng,
|
_assign_adjacency_aware(lvl, rooms, reqs, rng,
|
||||||
interior_outside=interior_outside, n_outside=n_o,
|
interior_outside=interior_outside, n_outside=n_o,
|
||||||
beam_width=construction_beam_width,
|
beam_width=construction_beam_width,
|
||||||
assign_solver=assign_solver)
|
assign_solver=assign_solver,
|
||||||
|
cpsat_limits=cpsat_limits)
|
||||||
else:
|
else:
|
||||||
assign = rooms + ["C", "O"] # +core circulation, +outside
|
assign = rooms + ["C", "O"] # +core circulation, +outside
|
||||||
_grow_leaves(lvl, len(assign), rng, balance=depth_balanced)
|
_grow_leaves(lvl, len(assign), rng, balance=depth_balanced)
|
||||||
|
|
|
||||||
|
|
@ -501,16 +501,47 @@ def test_construction_assign_cpsat_yields_valid_seed():
|
||||||
# present, canonical genome.
|
# present, canonical genome.
|
||||||
from homemaker_layout import graph, programme
|
from homemaker_layout import graph, programme
|
||||||
|
|
||||||
|
from homemaker_layout import cpsat as cpsat_mod
|
||||||
|
|
||||||
reqs = programme.load_programme_dir(str(HARBOR))
|
reqs = programme.load_programme_dir(str(HARBOR))
|
||||||
types = sorted(reqs) + ["C", "O"]
|
types = sorted(reqs) + ["C", "O"]
|
||||||
seed = dom.load(str(HARBOR / "init.dom"))
|
seed = dom.load(str(HARBOR / "init.dom"))
|
||||||
for trial in range(5):
|
|
||||||
root = operators.constructive_topology(
|
# This test asserts INVARIANTS -- every required space present, canonical
|
||||||
seed, reqs, np.random.default_rng(trial), types,
|
# genome -- which do not depend on the labelling being optimal. So it buys
|
||||||
assign_solver="cpsat")
|
# its runtime back with a reduced deterministic budget (homemaker-py-7t1);
|
||||||
_, missing = graph.check_space_counts(root, reqs)
|
# harbor's model needs ~3.6 work units for optimality since §38.14's added
|
||||||
assert missing == [], f"trial {trial} left {missing}"
|
# adjacency, and paying that here dominated the suite for no extra coverage.
|
||||||
canonical(root)
|
#
|
||||||
|
# The trap: too small a budget makes solve_room_labels return None, and
|
||||||
|
# _assign_adjacency_aware then falls back to GREEDY -- the test would pass
|
||||||
|
# while exercising nothing. Guarded by counting fallbacks and requiring the
|
||||||
|
# solver to have answered every time.
|
||||||
|
fell_back = []
|
||||||
|
orig = cpsat_mod.solve_room_labels
|
||||||
|
|
||||||
|
def counting(*a, **k):
|
||||||
|
r = orig(*a, **k)
|
||||||
|
fell_back.append(r is None)
|
||||||
|
return r
|
||||||
|
|
||||||
|
cpsat_mod.solve_room_labels = counting
|
||||||
|
operators.cpsat = cpsat_mod
|
||||||
|
try:
|
||||||
|
for trial in range(5):
|
||||||
|
root = operators.constructive_topology(
|
||||||
|
seed, reqs, np.random.default_rng(trial), types,
|
||||||
|
assign_solver="cpsat", cpsat_limits=(30.0, 0.25))
|
||||||
|
_, missing = graph.check_space_counts(root, reqs)
|
||||||
|
assert missing == [], f"trial {trial} left {missing}"
|
||||||
|
canonical(root)
|
||||||
|
finally:
|
||||||
|
cpsat_mod.solve_room_labels = orig
|
||||||
|
operators.cpsat = cpsat_mod
|
||||||
|
|
||||||
|
assert fell_back and not any(fell_back), (
|
||||||
|
f"cpsat fell back to greedy on {sum(fell_back)}/{len(fell_back)} solves "
|
||||||
|
f"-- the reduced budget is too small and this test stopped testing cpsat")
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.skipif(not HARBOR.is_dir(), reason="harbor-house not available")
|
@pytest.mark.skipif(not HARBOR.is_dir(), reason="harbor-house not available")
|
||||||
|
|
@ -542,18 +573,22 @@ def test_assign_cpsat_matches_or_beats_greedy_secondary_adjacency():
|
||||||
# Per-seed outcomes are noisy (both solvers depend on the same random
|
# Per-seed outcomes are noisy (both solvers depend on the same random
|
||||||
# room-order shuffle before falling into their own placement logic), so
|
# room-order shuffle before falling into their own placement logic), so
|
||||||
# the comparison is on the aggregate over several seeds, not every seed
|
# the comparison is on the aggregate over several seeds, not every seed
|
||||||
# individually — measured on harbor-house (10 seeds): cpsat wins on
|
# individually.
|
||||||
# most, ties on a few, loses on rare ones, net ~13% fewer total fails.
|
#
|
||||||
# The cpsat path is not yet bit-reproducible (homemaker-py-fdp): the solver
|
# This used to run the cpsat arm THREE times and compare the mean, because
|
||||||
# itself is deterministic, but something upstream of it in
|
# `homemaker-py-fdp` left the cpsat path non-bit-reproducible -- a single
|
||||||
# _assign_adjacency_aware still varies, so a single 10-seed aggregate can
|
# 10-seed aggregate could straddle greedy's deterministic value, so the test
|
||||||
# straddle greedy's (deterministic) value. Averaging three repeats asserts
|
# was flaky by construction. fdp is fixed (§38.15: `noncirc` was ordered by
|
||||||
# what is actually claimed -- better IN AGGREGATE -- instead of being flaky
|
# `id()`), so cpsat is now deterministic and one pass says exactly as much
|
||||||
# by construction. Measured after §39.4: greedy 189, cpsat 185/177/180/182.
|
# as three did, at a third of the cost -- this test dominated the suite.
|
||||||
|
#
|
||||||
|
# NOTE this measures ONLY secondary-adjacency fails ("not adjacent to"),
|
||||||
|
# which is the decision cpsat actually solves. It is not a claim that cpsat
|
||||||
|
# seeds better overall: measured over 12 seeds it is markedly WORSE on total
|
||||||
|
# fails (§38.20), which is why `assign_solver` stays default greedy.
|
||||||
greedy = sum(secondary_fails("greedy"))
|
greedy = sum(secondary_fails("greedy"))
|
||||||
cpsat_runs = [sum(secondary_fails("cpsat")) for _ in range(3)]
|
cpsat = sum(secondary_fails("cpsat"))
|
||||||
mean_cpsat = sum(cpsat_runs) / len(cpsat_runs)
|
assert cpsat < greedy, f"cpsat {cpsat} vs greedy {greedy}"
|
||||||
assert mean_cpsat < greedy, f"cpsat {cpsat_runs} (mean {mean_cpsat}) vs greedy {greedy}"
|
|
||||||
|
|
||||||
|
|
||||||
def test_reassign_noop_without_reqs():
|
def test_reassign_noop_without_reqs():
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue