From d751d1b13ed780abf305daa69136d3c3db2ddbe7 Mon Sep 17 00:00:00 2001 From: Codex instance Date: Thu, 30 Jul 2026 17:55:21 +0100 Subject: [PATCH] solver: replace cell DFS with skyline search Represent partial placements as column heights and branch on the narrowest local valley. This removes the board-area cell state and makes first-solution search substantially smaller for feasible orders. Expose direct-search and candidate-order benchmark controls so the skyline core can be measured independently of odd-order construction. Document the completeness argument and the 10/11 test-tier decision. Tests: Release, Debug, ASan, and UBSan CTest (10 passed each) Refs: #4 --- BENCHMARKING.md | 63 ++++++++- CMakeLists.txt | 1 + TESTING.md | 14 +- benchmarks/benchmark.cc | 45 ++++-- benchmarks/run.py | 32 ++++- main.cc | 300 +++++++++++++++++----------------------- tests/tests.cc | 39 ++++++ 7 files changed, 296 insertions(+), 198 deletions(-) diff --git a/BENCHMARKING.md b/BENCHMARKING.md index e24bd8f..3c96f5e 100644 --- a/BENCHMARKING.md +++ b/BENCHMARKING.md @@ -15,16 +15,25 @@ python3 benchmarks/run.py --binary build-benchmark/partridge_benchmark \ 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`, and -`--timeout`. 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: +Override the policy with `--orders`, `--warmup`, `--repetitions`, `--timeout`, +and `--candidate-order`. 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: ```sh python3 benchmarks/run.py --binary build-benchmark/partridge_benchmark \ --orders 8 9 --warmup 1 --repetitions 5 --timeout 60 > benchmark.json ``` +Use `--direct-search` when benchmarking the skyline core rather than the public +even-predecessor construction used for odd orders from 9 onwards: + +```sh +python3 benchmarks/run.py --binary build-benchmark/partridge_benchmark \ + --orders 6 7 8 9 --warmup 1 --repetitions 5 --timeout 60 \ + --candidate-order ascending --direct-search > direct.json +``` + The JSON contains every run and median, range, and median absolute deviation for solve, construction, independent validation, and rendering. Direct-search cases report zero construction time: their setup and allocation remain part of @@ -59,6 +68,52 @@ Do not use wall-clock thresholds as correctness checks. Keep the generated JSON outside version control unless it is being deliberately added as a named comparison baseline. +## Smallest-valley skyline + +The solver stores one filled height per board column instead of one value per +cell. Equal adjacent heights form conceptual vertical bars. A valley is a +maximal bar lower than both neighbours, with board edges treated as bars of +full board height. Each node scans for the smallest-width valley, breaking +ties by lower height and then leftmost position, and tries every available +square which fits at that valley's far-left edge. + +This branching remains complete: the bottom-left cell of the selected valley +must be covered, a square covering it cannot begin to the left across the +taller neighbour, and it cannot extend beyond the equal-height run without +overlap or leaving an unreachable hole. Trying every fitting available size +therefore includes the placement used by every possible completion. + +For board width `W` and order `n`, the skyline scan is `O(W)`. A node tries at +most `n` candidates and each placement or exact undo changes at most `n` +heights, giving `O(W + n^2)` local work. The skyline, multiplicities, +placements, and recursion stack use `O(W + n^2)` state, compared with the +former `O(W^2)` cell grid. + +One-run exploratory measurements used the issue #4 dirty worktree at base +commit `0a7ce1e`, Apple Clang 21.0.0, `-O3 -DNDEBUG`, macOS arm64, one worker, +no warm-up, and a 15-second timeout. Every completed result passed the +independent benchmark validator: + +| Order | Result | Ascending time | Ascending nodes | Descending time | Descending nodes | +| --- | --- | ---: | ---: | ---: | ---: | +| 6 | infeasible | 0.040 s | 659,598 | 0.039 s | 659,598 | +| 7 | infeasible | 3.103 s | 43,604,507 | 3.071 s | 43,604,507 | +| 8 | solution | 0.585 s | 7,735,369 | 0.941 s | 12,186,125 | +| 9 direct | solution | 3.831 s | 45,840,266 | timeout | unavailable | + +The infeasible orders exhaust the same tree in either direction. Ascending +was selected as the default because it reaches the first order-8 solution with +36% fewer nodes and also completed direct order 9 within the timeout; +descending direct order 9 did not. + +A direct ascending order-10 probe exceeded 20 seconds. Public order 10 is +also a direct search, and public order 11 first searches order 10 before using +odd-predecessor construction. Consequently neither 10 nor 11 is in the +routine correctness suite: doing so would test the same unresolved order-10 +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. + ## Post-correctness baseline This framework starts from commit `ce39d0a` after the rendering assertion fix diff --git a/CMakeLists.txt b/CMakeLists.txt index 758e28c..613f5bf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -24,6 +24,7 @@ if(BUILD_TESTING) add_test(NAME solver-small COMMAND partridge_tests solver-small) add_test(NAME solver-completion COMMAND partridge_tests solver-completion) add_test(NAME search-counters COMMAND partridge_tests search-counters) + add_test(NAME skyline-search COMMAND partridge_tests skyline-search) find_package(Python3 COMPONENTS Interpreter) if(Python3_Interpreter_FOUND) add_test(NAME benchmark-format diff --git a/TESTING.md b/TESTING.md index dfd1eb9..8ca6a12 100644 --- a/TESTING.md +++ b/TESTING.md @@ -13,9 +13,13 @@ 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 rendering test also -formats the known order-8 solution and checks the resulting grid dimensions and -coverage. +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. When Python is available, `reference-support` also tests the dependency-free placement JSON validator. If the optional OR-Tools package is present, it @@ -45,6 +49,4 @@ ctest --test-dir build-ubsan --output-on-failure ``` The sanitizer flags shown are supported by Clang and GCC. Other compilers may -require different flags. A feasible solver run currently exposes the -pre-existing defect tracked by issue #14; the test additions deliberately do -not include its separate fix. +require different flags. diff --git a/benchmarks/benchmark.cc b/benchmarks/benchmark.cc index e44894f..f301981 100644 --- a/benchmarks/benchmark.cc +++ b/benchmarks/benchmark.cc @@ -26,26 +26,29 @@ namespace { double construction_seconds; }; - auto solve(std::uint64_t order, bool instrument, SearchCounters &counters) + auto solve(std::uint64_t order, bool instrument, + CandidateOrder candidate_order, bool direct_search, + SearchCounters &counters) -> TimedSolution { auto const predecessor_order = - uses_odd_construction(order) ? order - 1 : order; + !direct_search && uses_odd_construction(order) ? order - 1 : order; auto const search_begin = Clock::now(); auto predecessor = instrument - ? search_solution_instrumented(predecessor_order, counters) - : search_solution(predecessor_order); + ? search_solution_instrumented(predecessor_order, counters, + candidate_order) + : search_solution(predecessor_order, candidate_order); auto const search_end = Clock::now(); auto const construction_begin = Clock::now(); - auto result = uses_odd_construction(order) + auto result = !direct_search && uses_odd_construction(order) ? construct_odd_solution(order, std::move(predecessor)) : std::move(predecessor); auto const construction_end = Clock::now(); return { std::move(result), seconds(search_begin, search_end), - uses_odd_construction(order) + !direct_search && uses_odd_construction(order) ? seconds(construction_begin, construction_end) : 0.0, }; @@ -90,8 +93,9 @@ namespace { } int main(int argc, char **argv) { - if (argc != 3) { - std::cerr << "usage: partridge_benchmark ORDER counters|plain\n"; + if (argc < 3 || argc > 5) { + std::cerr << "usage: partridge_benchmark ORDER counters|plain " + "[ascending|descending] [public|direct]\n"; return 2; } auto const order = static_cast(std::strtoull(argv[1], nullptr, 10)); @@ -100,9 +104,26 @@ int main(int argc, char **argv) { std::cerr << "counter mode must be counters or plain\n"; return 2; } + auto const candidate_order = + argc < 4 || std::string_view(argv[3]) == "ascending" + ? CandidateOrder::ascending + : CandidateOrder::descending; + 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"; + return 2; + } + auto const direct_search = + argc == 5 && std::string_view(argv[4]) == "direct"; + if (argc == 5 && std::string_view(argv[4]) != "public" && + std::string_view(argv[4]) != "direct") { + std::cerr << "search route must be public or direct\n"; + return 2; + } SearchCounters counters; - auto timed = solve(order, instrument, counters); + auto timed = + solve(order, instrument, candidate_order, direct_search, counters); auto const validation_begin = Clock::now(); auto const validation_ok = valid(order, timed.result); @@ -120,6 +141,12 @@ int main(int argc, char **argv) { << "{\"schema_version\":1" << ",\"order\":" << order << ",\"instrumented\":" << boolean(instrument) + << ",\"candidate_order\":\"" + << (candidate_order == CandidateOrder::ascending ? "ascending" + : "descending") + << "\"" + << ",\"search_route\":\"" + << (direct_search ? "direct" : "public") << "\"" << ",\"solved\":" << boolean(!timed.result.squares().empty()) << ",\"valid\":" << boolean(validation_ok) << ",\"timing_seconds\":{\"solve\":" << timed.search_seconds diff --git a/benchmarks/run.py b/benchmarks/run.py index 5484e23..8b97cf7 100755 --- a/benchmarks/run.py +++ b/benchmarks/run.py @@ -74,16 +74,16 @@ def environment(binary): "logical_cpus": os.cpu_count(), }, "workers": 1, - "search_policy": "single-threaded, deterministic, largest-fitting-first", + "search_policy": "single-threaded, deterministic, smallest-width valley", "seed": None, } -def run_once(binary, order, mode, timeout): +def run_once(binary, order, mode, candidate_order, search_route, timeout): started = time.monotonic() try: process = subprocess.run( - [str(binary), str(order), mode], + [str(binary), str(order), mode, candidate_order, search_route], check=False, capture_output=True, text=True, @@ -216,6 +216,12 @@ def main(): parser.add_argument("--warmup", type=int, default=1) parser.add_argument("--repetitions", type=int, default=5) parser.add_argument("--timeout", type=float, default=600.0) + parser.add_argument( + "--candidate-order", + choices=("ascending", "descending"), + default="ascending", + ) + parser.add_argument("--direct-search", action="store_true") parser.add_argument( "--measure-overhead", action="store_true", @@ -241,11 +247,25 @@ def main(): mode_results = {mode: {"runs": []} for mode in modes} for trial in range(args.warmup): for mode in trial_modes(modes, trial): - run_once(args.binary, order, mode, args.timeout) + run_once( + args.binary, + order, + mode, + args.candidate_order, + "direct" if args.direct_search else "public", + args.timeout, + ) for trial in range(args.repetitions): for mode in trial_modes(modes, trial): mode_results[mode]["runs"].append( - run_once(args.binary, order, mode, args.timeout) + run_once( + args.binary, + order, + mode, + args.candidate_order, + "direct" if args.direct_search else "public", + args.timeout, + ) ) for result in mode_results.values(): result["summary"] = summarize(result["runs"]) @@ -270,6 +290,8 @@ def main(): "warmup_runs": args.warmup, "measured_repetitions": args.repetitions, "per_run_timeout_seconds": args.timeout, + "candidate_order": args.candidate_order, + "search_route": "direct" if args.direct_search else "public", "stdout": "captured; rendered grid suppressed by probe", }, "cases": cases, diff --git a/main.cc b/main.cc index 64b4f9b..d84bc46 100644 --- a/main.cc +++ b/main.cc @@ -131,97 +131,6 @@ namespace { std::vector squares_; }; - /** An N * N grid of characters. */ - struct Grid { - // Type to use for the grid contents - using T = std::int_fast64_t; - - /** Construct a grid of given side-length. */ - explicit Grid(size_t length) : grid_(length * length, empty), length_(length) { - } - - Grid(Grid const &other) = delete; - - Grid(Grid &&other) noexcept = default; - - Grid &operator=(Grid const &other) = delete; - - Grid &operator=(Grid &&other) noexcept = default; - - ~Grid() noexcept = default; - - /** Get grid length */ - [[nodiscard]] auto end() const noexcept -> size_t { return static_cast(grid_.size()); } - - /** Add a square to the grid. */ - auto add(Square const &sq) noexcept -> void { - /* One would expect the fastest way to do this would be to have x be the - * fastest increasing index so we store [pos, pos + 1,..., pos+length, ...] - * But experimentation tells us this isn't so, and storing - * [pos, pos + length, ..., pos + 1, ...] is faster! - */ - for (auto x = 0; x < sq.length(); ++x) { - for (auto y = sq.pos(); y < sq.pos() + sq.length() * length_; y += length_) { - grid_[x + y] = filled; - } - } - } - - /** Clear a square from the grid. */ - auto clear(Square const &sq) noexcept -> void { - for (auto x = 0; x < sq.length(); ++x) { - for (auto y = sq.pos(); y < sq.pos() + sq.length() * length_; y += length_) { - grid_[x + y] = empty; - } - } - } - - /** \brief Get length of the largest square that fits at \a pos in the grid. - */ - [[nodiscard]] auto largest_square(Pos pos, size_t n) const noexcept -> size_t { - assert(pos < end()); - - /* Because of how we walk through the grid (starting at 0,0 then increasing - * x followed by y) we can assume that if the position (b, y) is clear - * (i.e. a '.') then (b, y + i) is clear for all i > 0. - * - * This means we only need to look for the first non-clear position along the - * current row. - */ - auto const pos_x = pos % length_; - auto const pos_y0 = pos - pos_x; - auto b = pos; - // Make sure we don't go looking in the next row. - auto const e = std::min(pos + n, pos_y0 + length_); - while (b < e) { - if (grid_[b] != empty) { break; } - ++b; - } - // Check that this length fits vertically as well. - auto const len = b - pos; - auto const pos_y = pos / length_; - auto const ye = std::min(pos_y + len, length_); - return ye - pos_y; - } - - /** Get the next position to check starting at pos. - * - * Returns grid_.length() if no more positions available. - */ - [[nodiscard]] auto next_pos(Pos pos) const noexcept -> Pos { - auto const b = grid_.begin() + static_cast(pos); - auto const p = std::find(b, grid_.end(), empty); - return p - grid_.begin(); - } - - private: - std::vector grid_; ///< The grid - size_t length_; ///< Side length - - static constexpr char empty = 0; ///< Character used for an empty cell. - static constexpr char filled = 1; ///< Character used for a filled cell, - }; - /** Get the n-th triangular number. */ auto triangle_num(size_t n) noexcept -> size_t { return (n * (n + 1)) / 2; } @@ -245,112 +154,155 @@ namespace { size_t completed_tasks = 0; }; - /** Search directly for a solution to the \a n th Partridge problem. - * - * Returns the grid of the solution. - */ + enum class CandidateOrder { + ascending, + descending, + }; + + /** A maximal level skyline segment which is lower than its neighbours. */ + struct Valley { + size_t x; + size_t height; + size_t width; + }; + + /** Return the narrowest local valley, breaking ties by height then x. */ + [[nodiscard]] auto smallest_valley(std::vector const &skyline) noexcept + -> Valley { + Valley best{0, 0, skyline.size()}; + bool found = false; + for (size_t begin = 0; begin < skyline.size();) { + auto end = begin + 1; + while (end < skyline.size() && skyline[end] == skyline[begin]) { + ++end; + } + auto const left_height = + begin == 0 ? skyline.size() : skyline[begin - 1]; + auto const right_height = + end == skyline.size() ? skyline.size() : skyline[end]; + auto const lower_than_left = skyline[begin] < left_height; + auto const lower_than_right = skyline[begin] < right_height; + auto const width = end - begin; + if (lower_than_left && lower_than_right && + (!found || width < best.width || + (width == best.width && skyline[begin] < best.height) || + (width == best.width && skyline[begin] == best.height && + begin < best.x))) { + best = {begin, skyline[begin], width}; + found = true; + } + begin = end; + } + assert(found); + return best; + } + template - auto search_solution_impl(size_t const n, SearchCounters *const counters) noexcept - -> Results { - /* Implementation is iterative, as opposed to recursive. - * - * The recursive implementation is easier to understand - but is - * slightly slower because of the repeated function calls (and - * entry/exit). - * - * The basic algorithm is to start at the origin of the grid we - * want to place squares on and iterate over the permutations of - * available squares until we find one that fits. - */ - - // grid is our in-progress grid of square positions. - auto const length = triangle_num(n); - Grid grid(length); - - /* avail_sqs is a vector indexed by square length indicating how many - * squares are available. Initially set up so that avail_sqs[i] = i. - */ - Avail avail_sqs; - for (auto i = 0; i <= n; ++i) { avail_sqs.push_back(i); } - - /* sqs is a vector used as a stack of the squares currently placed. - * We reserve the length we need so as not to have too many allocations. - */ - std::vector sqs; - sqs.reserve(length); - - // Start at the origin with a square of longest side length. - Pos pos = 0; - size_t idx = n; - + auto search_skyline(size_t const n, size_t const length, + CandidateOrder const candidate_order, + std::vector &skyline, Avail &available, + std::vector &squares, + SearchCounters *const counters) noexcept -> bool { if constexpr (Instrument) { assert(counters != nullptr); ++counters->search_nodes; } - while (true) { + if (squares.size() == length) { + return true; + } + + auto const valley = smallest_valley(skyline); + auto const largest = + std::min({n, valley.width, length - valley.height}); + auto try_side = [&](size_t const side) { if constexpr (Instrument) { ++counters->loop_iterations; } - /* If the idx is 0 we've looked at all possible square lengths for this - * position, and they've failed. Pop the last square of the stack, remove - * it from the grid and try the next smaller size in the same position. - */ - if (idx == 0) { - // No squares on the stack -> failed to find a solution. - if (sqs.empty()) { break; } - - auto sq = sqs.back(); - sqs.pop_back(); - grid.clear(sq); - ++avail_sqs[sq.length()]; - if constexpr (Instrument) { - ++counters->backtracks; - } - pos = sq.pos(); - idx = sq.length() - 1; - continue; + if (available[side] == 0) { + return false; } - // If there are no squares available of the current size try the next one. - if (avail_sqs[idx] == 0) { - --idx; - continue; - } - - /* Place a square of side length idx at pos, push this onto the stack and - * set up to look at the next position. - */ - auto const sq = Square(pos, idx); if constexpr (Instrument) { ++counters->attempted_placements; } - --avail_sqs[idx]; - grid.add(sq); - sqs.push_back(sq); + --available[side]; + std::fill_n(skyline.begin() + static_cast(valley.x), + side, valley.height + side); + squares.emplace_back(valley.x + valley.height * length, side); - pos = grid.next_pos(pos + idx); - - // Have we reached the end? If so success! - if (pos == grid.end()) { break; } - - if constexpr (Instrument) { - ++counters->search_nodes; + if (search_skyline(n, length, candidate_order, skyline, + available, squares, counters)) { + return true; } - idx = grid.largest_square(pos, n); - } - return {length, sqs}; + squares.pop_back(); + std::fill_n(skyline.begin() + static_cast(valley.x), + side, valley.height); + ++available[side]; + if constexpr (Instrument) { + ++counters->backtracks; + } + return false; + }; + + if (candidate_order == CandidateOrder::ascending) { + for (size_t side = 1; side <= largest; ++side) { + if (try_side(side)) { + return true; + } + } + } else { + for (auto side = largest; side != 0; --side) { + if (try_side(side)) { + return true; + } + } + } + return false; } - auto search_solution(size_t const n) noexcept -> Results { - return search_solution_impl(n, nullptr); + /** Search directly for a solution to the \a n th Partridge problem. + * + * The state is one filled height per board column. At each node the + * narrowest local valley is found by a linear scan and candidates are tried + * at its far-left edge. A placement or undo touches one entry per square + * column. Scanning the profile costs O(board width); trying up to n + * candidates and updating up to n columns for each costs O(n^2), for + * 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, + SearchCounters *const counters) noexcept + -> Results { + auto const length = triangle_num(n); + std::vector skyline(length); + Avail available(n + 1); + for (size_t side = 0; side <= n; ++side) { + available[side] = side; + } + std::vector squares; + squares.reserve(length); + static_cast(search_skyline( + n, length, candidate_order, skyline, available, squares, counters)); + + return {length, std::move(squares)}; + } + + auto search_solution( + size_t const n, + CandidateOrder const candidate_order = CandidateOrder::ascending) noexcept + -> Results { + return search_solution_impl(n, candidate_order, nullptr); } auto search_solution_instrumented(size_t const n, - SearchCounters &counters) noexcept -> Results { + SearchCounters &counters, + CandidateOrder const candidate_order = + CandidateOrder::ascending) noexcept + -> Results { counters = {}; - return search_solution_impl(n, &counters); + return search_solution_impl(n, candidate_order, &counters); } /** Construct an odd-order solution from its even-order predecessor. */ diff --git a/tests/tests.cc b/tests/tests.cc index c9757a0..7370ef4 100644 --- a/tests/tests.cc +++ b/tests/tests.cc @@ -343,6 +343,42 @@ namespace { return failures; } + auto test_skyline_search() -> int { + int failures = 0; + auto const narrowest = + smallest_valley(std::vector{4, 2, 2, 4, 0, 0, 0, 4}); + failures += expect( + narrowest.x == 1 && narrowest.height == 2 && narrowest.width == 2, + "skyline did not select the smallest-width valley"); + + auto const tie = + smallest_valley(std::vector{4, 1, 4, 4, 2, 4}); + failures += expect(tie.x == 1 && tie.height == 1 && tie.width == 1, + "skyline valley tie-break is not deterministic"); + + SearchCounters descending_counters; + auto const descending = search_solution_instrumented( + 8, descending_counters, CandidateOrder::descending); + auto validation = validate(8, descending); + failures += expect( + validation.valid(), + "descending skyline search returned an invalid order-8 solution:\n" + + validation.text()); + + SearchCounters direct_nine_counters; + auto const direct_nine = search_solution_instrumented( + 9, direct_nine_counters, CandidateOrder::ascending); + validation = validate(9, direct_nine); + failures += expect( + validation.valid(), + "direct skyline search returned an invalid order-9 solution:\n" + + validation.text()); + failures += expect( + direct_nine_counters.search_nodes != descending_counters.search_nodes, + "direct order-9 coverage unexpectedly reused predecessor construction"); + return failures; + } + auto test_solver_completion() -> int { int failures = 0; for (auto const order: std::array{1, 8}) { @@ -385,6 +421,9 @@ int main(int argc, char **argv) { if (test == "search-counters") { return test_search_counters(); } + if (test == "skyline-search") { + return test_skyline_search(); + } std::cerr << "unknown test: " << test << '\n'; return 2; }