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

@@ -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

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

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::uint64_t>(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") << "\""

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",
         },

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<bool Instrument>
   auto search_skyline(size_t const n, size_t const length,
-                      CandidateOrder const candidate_order,
+                      SearchPolicy const policy,
                       std::vector<size_t> &skyline, Avail &available,
                       std::vector<Square> &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<Instrument>(n, length, candidate_order, skyline,
+      if (search_skyline<Instrument>(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<bool Instrument>
-  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<Square> squares;
     squares.reserve(length);
     static_cast<void>(search_skyline<Instrument>(
-        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<false>(n, candidate_order, nullptr);
+    return search_solution_impl<false>(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<true>(n, candidate_order, &counters);
+    return search_solution_impl<true>(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
 

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<void>(search_solution_instrumented(
+        5, ascending_exhaustive, SearchPolicy::ascending));
+    static_cast<void>(search_solution_instrumented(
+        5, descending_exhaustive, SearchPolicy::descending));
+    static_cast<void>(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;
   }