From 74bde266d057a35e30f0ab0200b9a759449b60b8 Mon Sep 17 00:00:00 2001 From: Codex instance Date: Thu, 30 Jul 2026 18:07:29 +0100 Subject: [PATCH] solver: select skyline candidate policy Make candidate ordering an explicit deterministic policy and benchmark ascending, descending, and exact-width-first choices. Keep ascending as the default because it produces the best measured time to first solution despite best-fit's slightly smaller tree. Retain the benchmark v1 interface and document why randomized and duplicate-work portfolio policies are deferred. Tests: Release and Debug CTest (10 passed each) Refs: #12 --- BENCHMARKING.md | 55 ++++++++++++++++++++++++++++++++++++++++- TESTING.md | 20 +++++++++------ benchmarks/benchmark.cc | 29 ++++++++++++---------- benchmarks/run.py | 13 +++++----- main.cc | 49 +++++++++++++++++++++++------------- tests/tests.cc | 24 +++++++++++++++--- 6 files changed, 141 insertions(+), 49 deletions(-) diff --git a/BENCHMARKING.md b/BENCHMARKING.md index 3c96f5e..d88be7a 100644 --- a/BENCHMARKING.md +++ b/BENCHMARKING.md @@ -16,7 +16,8 @@ The default policy is one unrecorded warm-up followed by five repetitions per case, with a 600-second timeout for each process. Solver stdout is captured; the probe renders into an in-memory stream so grids do not perturb terminal I/O. Override the policy with `--orders`, `--warmup`, `--repetitions`, `--timeout`, -and `--candidate-order`. Ascending candidate sizes are the production default. +and `--candidate-order`. The choices are `ascending`, `descending`, and +`best-fit`; ascending candidate sizes are the production default. Order 9 uses the constructive odd-order path, searching order 8 and then tiling the enlarged border, so it is suitable for normal local benchmarking: @@ -114,6 +115,58 @@ search bottleneck, while the existing route-boundary test still verifies that 11 selects construction. Revisit both sizes when order 10 completes within a practical test budget. +## Candidate policy selection + +Candidate ordering is a deterministic search policy and does not alter the +smallest-valley selection or set of placements tried. `ascending` tries +smaller available squares first and `descending` tries larger ones first. +`best-fit` first tries a square exactly as wide as the selected valley, because +that placement closes the valley without leaving a shelf remainder, then tries +the other sizes in ascending order. If no exact-width square fits, best-fit +and ascending are identical at that node. + +The policy comparison used the issue #12 working tree based on commit +`d751d1b`, Apple Clang 21.0.0, `-O3 -DNDEBUG`, macOS arm64, and one worker. +Order 8 used two warm-ups and seven sequential measured repetitions; direct +order 9 used one warm-up and three measured repetitions. All completed +results passed independent validation and node counts were stable: + +| Order | Policy | Counted median (range) | Counter-free median | Nodes | +| --- | --- | --- | --- | ---: | +| 8 | ascending | 0.808 s (0.790–0.852 s) | 0.794 s | 7,735,369 | +| 8 | descending | 1.310 s (1.279–1.449 s) | 1.268 s | 12,186,125 | +| 8 | best-fit | 0.817 s (0.813–0.857 s) | 0.823 s | 7,679,349 | +| 9 direct | ascending | 5.522 s (5.499–5.830 s) | not measured | 45,840,266 | +| 9 direct | best-fit | 5.651 s (5.533–5.820 s) | not measured | 45,746,016 | + +The earlier direct-order-9 descending probe exceeded its 15-second limit. +Ascending is retained as the stable single-threaded default because it had the +lowest measured median time to the first solution at both measured solvable +sizes. Best-fit's slightly smaller trees did not compensate for its policy +checks, while descending was substantially worse. Exhaustive infeasible +order-5 tests visit the same number of nodes under all three policies, which +checks that ordering does not affect completeness. + +One policy therefore applies to the currently measured sizes 8 and 9. This +does not establish that ascending is optimal for order 10: a bounded best-fit +order-9 comparison changed the search tree by only 0.2%, so there was no +evidence that repeating the known long order-10/11 search would be useful. +Keep 10 and 11 as opt-in benchmark cases. Public order 11 is particularly +important to interpret correctly: it constructs from an order-10 search, so it +does not independently measure an odd-order candidate policy. + +A worker portfolio was considered but not added. Running identical policies +duplicates the same deterministic traversal. Pairing ascending with best-fit +adds little diversity on the measured trees, and pairing ascending with +descending dedicates a worker to the consistently slower policy. Splitting a +shared frontier could avoid duplicated prefixes, but that is the parallel +frontier work tracked separately in issue #11. Seeded randomized ordering was +also rejected for now: the deterministic alternatives already select a clear +default, and there is no measurement showing that seed distributions would +improve time to first solution. The benchmark schema retains its nullable +seed field so a future evidence-backed randomized policy can report +reproducible runs without changing the format. + ## Post-correctness baseline This framework starts from commit `ce39d0a` after the rendering assertion fix diff --git a/TESTING.md b/TESTING.md index 8ca6a12..60f0a90 100644 --- a/TESTING.md +++ b/TESTING.md @@ -12,14 +12,18 @@ ctest --test-dir build --output-on-failure The tests independently check board dimensions, square multiplicities, bounds, overlap, and complete coverage. They cover small unsatisfiable solver inputs, a known order-8 solution, invalid placement diagnostics, and construction of an -order-9 solution from the order-8 fixture. The routed order-9 solver test -checks that its search counters exactly match order 8. The skyline tests check -smallest-width valley selection and deterministic tie-breaking, validate an -order-8 result with descending candidates, and independently validate a direct -order-9 search with ascending candidates. That direct test is deliberately -separate from the public order-9 route, which uses even-predecessor -construction. The rendering test also formats the known order-8 solution and -checks the resulting grid dimensions and coverage. +order-9 solution from the order-8 fixture. The routed best-fit order-9 solver +test checks that its search counters exactly match order 8. The skyline tests +check smallest-width valley selection and deterministic tie-breaking, validate +an order-8 result with descending candidates, and independently validate a +direct order-9 search with ascending candidates. An +exhaustive infeasible order also checks that all three policies visit the same +search space. The direct order-9 test is deliberately separate from the +public order-9 route, which uses even-predecessor construction. Orders 10 and +11 are not routine tests because both public routes require the same long +direct order-10 search; the route-boundary test still checks that order 11 +selects odd construction. The rendering test also formats the known order-8 +solution and checks the resulting grid dimensions and coverage. When Python is available, `reference-support` also tests the dependency-free placement JSON validator. If the optional OR-Tools package is present, it diff --git a/benchmarks/benchmark.cc b/benchmarks/benchmark.cc index f301981..3cb36b0 100644 --- a/benchmarks/benchmark.cc +++ b/benchmarks/benchmark.cc @@ -27,7 +27,7 @@ namespace { }; auto solve(std::uint64_t order, bool instrument, - CandidateOrder candidate_order, bool direct_search, + SearchPolicy policy, bool direct_search, SearchCounters &counters) -> TimedSolution { auto const predecessor_order = @@ -36,8 +36,8 @@ namespace { auto predecessor = instrument ? search_solution_instrumented(predecessor_order, counters, - candidate_order) - : search_solution(predecessor_order, candidate_order); + policy) + : search_solution(predecessor_order, policy); auto const search_end = Clock::now(); auto const construction_begin = Clock::now(); @@ -95,7 +95,7 @@ namespace { int main(int argc, char **argv) { if (argc < 3 || argc > 5) { std::cerr << "usage: partridge_benchmark ORDER counters|plain " - "[ascending|descending] [public|direct]\n"; + "[ascending|descending|best-fit] [public|direct]\n"; return 2; } auto const order = static_cast(std::strtoull(argv[1], nullptr, 10)); @@ -104,13 +104,16 @@ int main(int argc, char **argv) { std::cerr << "counter mode must be counters or plain\n"; return 2; } - auto const candidate_order = + auto const policy = argc < 4 || std::string_view(argv[3]) == "ascending" - ? CandidateOrder::ascending - : CandidateOrder::descending; + ? SearchPolicy::ascending + : std::string_view(argv[3]) == "descending" + ? SearchPolicy::descending + : SearchPolicy::best_fit; if (argc >= 4 && std::string_view(argv[3]) != "ascending" && - std::string_view(argv[3]) != "descending") { - std::cerr << "candidate order must be ascending or descending\n"; + std::string_view(argv[3]) != "descending" && + std::string_view(argv[3]) != "best-fit") { + std::cerr << "search policy must be ascending, descending, or best-fit\n"; return 2; } auto const direct_search = @@ -122,8 +125,7 @@ int main(int argc, char **argv) { } SearchCounters counters; - auto timed = - solve(order, instrument, candidate_order, direct_search, counters); + auto timed = solve(order, instrument, policy, direct_search, counters); auto const validation_begin = Clock::now(); auto const validation_ok = valid(order, timed.result); @@ -142,8 +144,9 @@ int main(int argc, char **argv) { << ",\"order\":" << order << ",\"instrumented\":" << boolean(instrument) << ",\"candidate_order\":\"" - << (candidate_order == CandidateOrder::ascending ? "ascending" - : "descending") + << (policy == SearchPolicy::ascending + ? "ascending" + : policy == SearchPolicy::descending ? "descending" : "best-fit") << "\"" << ",\"search_route\":\"" << (direct_search ? "direct" : "public") << "\"" diff --git a/benchmarks/run.py b/benchmarks/run.py index 8b97cf7..678492b 100755 --- a/benchmarks/run.py +++ b/benchmarks/run.py @@ -79,11 +79,11 @@ def environment(binary): } -def run_once(binary, order, mode, candidate_order, search_route, timeout): +def run_once(binary, order, mode, search_policy, search_route, timeout): started = time.monotonic() try: process = subprocess.run( - [str(binary), str(order), mode, candidate_order, search_route], + [str(binary), str(order), mode, search_policy, search_route], check=False, capture_output=True, text=True, @@ -218,7 +218,8 @@ def main(): parser.add_argument("--timeout", type=float, default=600.0) parser.add_argument( "--candidate-order", - choices=("ascending", "descending"), + dest="search_policy", + choices=("ascending", "descending", "best-fit"), default="ascending", ) parser.add_argument("--direct-search", action="store_true") @@ -251,7 +252,7 @@ def main(): args.binary, order, mode, - args.candidate_order, + args.search_policy, "direct" if args.direct_search else "public", args.timeout, ) @@ -262,7 +263,7 @@ def main(): args.binary, order, mode, - args.candidate_order, + args.search_policy, "direct" if args.direct_search else "public", args.timeout, ) @@ -290,7 +291,7 @@ def main(): "warmup_runs": args.warmup, "measured_repetitions": args.repetitions, "per_run_timeout_seconds": args.timeout, - "candidate_order": args.candidate_order, + "candidate_order": args.search_policy, "search_route": "direct" if args.direct_search else "public", "stdout": "captured; rendered grid suppressed by probe", }, diff --git a/main.cc b/main.cc index d84bc46..4c2cb5d 100644 --- a/main.cc +++ b/main.cc @@ -154,9 +154,12 @@ namespace { size_t completed_tasks = 0; }; - enum class CandidateOrder { + /** Deterministic order in which a skyline node tries fitting squares. */ + enum class SearchPolicy { ascending, descending, + /** Close the selected valley when possible, then try smaller sizes first. */ + best_fit, }; /** A maximal level skyline segment which is lower than its neighbours. */ @@ -199,7 +202,7 @@ namespace { template auto search_skyline(size_t const n, size_t const length, - CandidateOrder const candidate_order, + SearchPolicy const policy, std::vector &skyline, Avail &available, std::vector &squares, SearchCounters *const counters) noexcept -> bool { @@ -231,7 +234,7 @@ namespace { side, valley.height + side); squares.emplace_back(valley.x + valley.height * length, side); - if (search_skyline(n, length, candidate_order, skyline, + if (search_skyline(n, length, policy, skyline, available, squares, counters)) { return true; } @@ -246,8 +249,15 @@ namespace { return false; }; - if (candidate_order == CandidateOrder::ascending) { + if (policy == SearchPolicy::best_fit && largest == valley.width && + try_side(largest)) { + return true; + } + if (policy != SearchPolicy::descending) { for (size_t side = 1; side <= largest; ++side) { + if (policy == SearchPolicy::best_fit && side == valley.width) { + continue; + } if (try_side(side)) { return true; } @@ -272,7 +282,7 @@ namespace { * O(board width + n^2) local work per node and the same total state. */ template - auto search_solution_impl(size_t const n, CandidateOrder const candidate_order, + auto search_solution_impl(size_t const n, SearchPolicy const policy, SearchCounters *const counters) noexcept -> Results { auto const length = triangle_num(n); @@ -284,25 +294,25 @@ namespace { std::vector squares; squares.reserve(length); static_cast(search_skyline( - n, length, candidate_order, skyline, available, squares, counters)); + n, length, policy, skyline, available, squares, counters)); return {length, std::move(squares)}; } auto search_solution( size_t const n, - CandidateOrder const candidate_order = CandidateOrder::ascending) noexcept + SearchPolicy const policy = SearchPolicy::ascending) noexcept -> Results { - return search_solution_impl(n, candidate_order, nullptr); + return search_solution_impl(n, policy, nullptr); } auto search_solution_instrumented(size_t const n, SearchCounters &counters, - CandidateOrder const candidate_order = - CandidateOrder::ascending) noexcept + SearchPolicy const policy = + SearchPolicy::ascending) noexcept -> Results { counters = {}; - return search_solution_impl(n, candidate_order, &counters); + return search_solution_impl(n, policy, &counters); } /** Construct an odd-order solution from its even-order predecessor. */ @@ -335,20 +345,25 @@ namespace { return n >= 9 && n % 2 == 1; } - auto find_solution(size_t const n) noexcept -> Results { + auto find_solution( + size_t const n, + SearchPolicy const policy = SearchPolicy::ascending) noexcept -> Results { if (uses_odd_construction(n)) { - return construct_odd_solution(n, search_solution(n - 1)); + return construct_odd_solution(n, search_solution(n - 1, policy)); } - return search_solution(n); + return search_solution(n, policy); } auto find_solution_instrumented(size_t const n, - SearchCounters &counters) noexcept -> Results { + SearchCounters &counters, + SearchPolicy const policy = + SearchPolicy::ascending) noexcept + -> Results { if (uses_odd_construction(n)) { return construct_odd_solution( - n, search_solution_instrumented(n - 1, counters)); + n, search_solution_instrumented(n - 1, counters, policy)); } - return search_solution_instrumented(n, counters); + return search_solution_instrumented(n, counters, policy); } } // anon namespace diff --git a/tests/tests.cc b/tests/tests.cc index 7370ef4..661a571 100644 --- a/tests/tests.cc +++ b/tests/tests.cc @@ -249,8 +249,10 @@ namespace { auto test_odd_solver_route() -> int { SearchCounters even_counters; SearchCounters odd_counters; - auto const even = find_solution_instrumented(8, even_counters); - auto const odd = find_solution_instrumented(9, odd_counters); + auto const even = + find_solution_instrumented(8, even_counters, SearchPolicy::best_fit); + auto const odd = + find_solution_instrumented(9, odd_counters, SearchPolicy::best_fit); int failures = 0; failures += expect( @@ -358,7 +360,7 @@ namespace { SearchCounters descending_counters; auto const descending = search_solution_instrumented( - 8, descending_counters, CandidateOrder::descending); + 8, descending_counters, SearchPolicy::descending); auto validation = validate(8, descending); failures += expect( validation.valid(), @@ -367,7 +369,7 @@ namespace { SearchCounters direct_nine_counters; auto const direct_nine = search_solution_instrumented( - 9, direct_nine_counters, CandidateOrder::ascending); + 9, direct_nine_counters, SearchPolicy::ascending); validation = validate(9, direct_nine); failures += expect( validation.valid(), @@ -376,6 +378,20 @@ namespace { failures += expect( direct_nine_counters.search_nodes != descending_counters.search_nodes, "direct order-9 coverage unexpectedly reused predecessor construction"); + + SearchCounters ascending_exhaustive; + SearchCounters descending_exhaustive; + SearchCounters best_fit_exhaustive; + static_cast(search_solution_instrumented( + 5, ascending_exhaustive, SearchPolicy::ascending)); + static_cast(search_solution_instrumented( + 5, descending_exhaustive, SearchPolicy::descending)); + static_cast(search_solution_instrumented( + 5, best_fit_exhaustive, SearchPolicy::best_fit)); + failures += expect( + ascending_exhaustive.search_nodes == descending_exhaustive.search_nodes && + ascending_exhaustive.search_nodes == best_fit_exhaustive.search_nodes, + "candidate policy changed the exhaustive skyline search space"); return failures; }