From 69b9816553220edc264b5ba3b1160208d0f81b5e Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Thu, 10 Sep 2026 08:22:34 +0200 Subject: [PATCH 01/15] comparison: shape solution comparison contract and search integration Zdev-Change-Id: Zd450758a16d392690607cf153a5160083ad5c3a5cd6812f461a16610075fc80a --- .zdev/comparison/TASKS.md | 11 + .zdev/comparison/area.toml | 7 + .../background/comparison-and-search.md | 273 ++++++++++++++++++ .zdev/comparison/brief.md | 260 +++++++++++++++++ .zdev/config.toml | 6 + 5 files changed, 557 insertions(+) create mode 100644 .zdev/comparison/TASKS.md create mode 100644 .zdev/comparison/area.toml create mode 100644 .zdev/comparison/background/comparison-and-search.md create mode 100644 .zdev/comparison/brief.md create mode 100644 .zdev/config.toml diff --git a/.zdev/comparison/TASKS.md b/.zdev/comparison/TASKS.md new file mode 100644 index 0000000000..9a8553a6cd --- /dev/null +++ b/.zdev/comparison/TASKS.md @@ -0,0 +1,11 @@ + + +# Tasks: comparison + +- Total: 0 +- Ready: 0 +- Blocked: 0 +- Done: 0 + +| ID | Task | State | Blocked by | +| --- | --- | --- | --- | diff --git a/.zdev/comparison/area.toml b/.zdev/comparison/area.toml new file mode 100644 index 0000000000..0255555eb7 --- /dev/null +++ b/.zdev/comparison/area.toml @@ -0,0 +1,7 @@ +schema_version = 1 +tag = "comparison" +title = "Non-mutating solution comparison" +objective = "Define and integrate a non-mutating solution comparison contract for Gecode best-solution search." +branch = "feature/comparison" +lifecycle = "open" +base_commit = "6b7de57b0414fe2f8af3513743c9835840a3c019" diff --git a/.zdev/comparison/background/comparison-and-search.md b/.zdev/comparison/background/comparison-and-search.md new file mode 100644 index 0000000000..4f43dcd28d --- /dev/null +++ b/.zdev/comparison/background/comparison-and-search.md @@ -0,0 +1,273 @@ +# Comparison and best-solution search + +This note records the analysis behind the [comparison brief](../brief.md). +Repository observations refer to `main` at +`6b7de57b0414fe2f8af3513743c9835840a3c019`. Symbol names are the primary source +anchors; line numbers may move during implementation. + +## What is missing today? + +[`Space::constrain()`](../../../gecode/kernel/core.hpp) specifies a restriction +to solutions better than an incumbent. The default implementation does nothing +in [`core.cpp`](../../../gecode/kernel/core.cpp). It is a mutation of a search +space, not a query about two solutions. + +The implementation already uses it for both purposes: + +| Source and symbol | Current arbitration | +| --- | --- | +| [`search/par/bab.hpp`](../../../gecode/search/par/bab.hpp), `BAB::solution` | Constrain the candidate by the incumbent; discard it if propagation fails. | +| Same file, `BAB::constrain` | Constrain the retained incumbent by the incoming one; retain the old one if propagation does not fail. | +| [`search/seq/bab.hpp`](../../../gecode/search/seq/bab.hpp), `BAB::constrain` | The same reverse-direction test, charging propagation through the worker statistics. | +| [`search/par/pbs.hpp`](../../../gecode/search/par/pbs.hpp), `CollectBest::add` / `constrain` | Constrain the retained incumbent by the candidate; replace it if propagation fails. | +| [`search/seq/rbs.cpp`](../../../gecode/search/seq/rbs.cpp), `RBS::constrain` | Apply the reverse-direction test to `last`, then update master and child engine. | + +For a strict total objective on fully determined costs, the forward test accepts +strict improvements. The reverse test also accepts ties: the old solution cannot +be strictly better than an equal candidate. Thus current arbitration is not +uniform even for simple objectives. + +With a custom constraint, propagation need not decide satisfiability. Posting +may add auxiliaries or branchers, change the supposedly completed solution, or +leave an undecided relation. Two clone/constrain/status probes do not repair +that generally: failure is useful evidence, but non-failure is not a proof of +the converse ordering. Full search could decide some such questions, at a cost +and with a contract quite different from comparing objective values. + +The expected gain is removal of this posting/propagation work during arbitration, +including work done under a central mutex. It does not remove solution ownership +clones, worker incumbent copies, pruning, or recomputation. No measured speedup +is claimed. + +## Not every constrain method defines an order + +[`car-sequencing.cpp`](../../../examples/car-sequencing.cpp), +`CarSequencing::constrain`, is an ordinary scalar objective: reduce `nstall`. +The balanced objective in [`test/search.cpp`](../../../test/search.cpp), +`HasSolutions::constrain`, compares an absolute difference of sums. Both have +simple read-only comparisons without requiring one particular cost variable +representation. + +[`cartesian-heart.cpp`](../../../examples/cartesian-heart.cpp), +`CartesianHeart::constrain`, seeks a next point sufficiently separated in selected +directions. Its disjunction allows either direction of movement in the second +coordinate; sufficiently separated points can satisfy both directed tests. +This is not an asymmetric improvement order, yet the example uses sequential BAB. +Changing `Space` to require a meaningful order for all BAB uses would lose an +existing useful model pattern. + +Other limits follow from the same distinction: diversity enumeration, an +incumbent-dependent neighborhood, changing objective weights, or a policy that +depends on search history may not have a fixed transitive ranking. Some can be +reformulated with fixed policy state; the engine must not infer that reformulation. + +## Non-fixed spaces + +[`Space::status()`](../../../gecode/kernel/core.cpp) returns `SS_SOLVED` when +no brancher has alternatives left. It does not inspect every variable for +assignment. This is why a generic solved-space check cannot validate cost data. +Calling it inside a const comparison would also introduce propagation. + +For integer helpers, requiring assigned objective values is sufficient; requiring +all variables to be assigned is unnecessary and can reject valid models. A stable +space with an assigned cost and unfinished auxiliary search has a determinate +objective key, although it is not necessarily a feasible incumbent to give search. +The distinction belongs in the public documentation. + +Comparing arbitrary domains asks a different question. For example, minimization +domains `[1,4]` and `[3,6]` overlap: comparing minima ranks bounds, not the eventual +solutions. Some disjoint domains admit a guaranteed relation, but adding a generic +domain comparison would need to distinguish unknown from true incomparability +and would not replace propagation-based pruning. Defer that interface. + +## Ordering and nested improvement restrictions + +A total ordering of objective equivalence classes is the useful common case: +scalar, lexicographic, fixed weighted sums, and fixed hierarchical criteria. +Two different assignments can be equivalent. Replacing an incumbent only on a +strict improvement avoids equal-result churn, but equivalence must also mean +that the two incumbent cuts are interchangeable. + +Let `F(b)` denote the set of feasible objective-complete solutions allowed by +the restriction posted by `constrain(b)`. For exact ordinary optimization, +`F(b) = {x | x is better than b}`. Transitivity then gives +`F(a) subset F(b)` whenever `a` is better than `b`. + +The search code depends on this nesting: workers can have older cuts, accept +new cuts, and retain local incumbents. An asset that has already explored or +pruned a region cannot recover it merely because a new comparison method exists. +For stepped or otherwise approximate optimization, the equality above can be +relaxed, but cut nesting and a clearly stated weaker optimality promise remain +necessary. + +The four ordering outcomes follow the familiar partial-order distinction between +less, equivalent, greater, and unordered in the +[C++ comparison specification](https://eel.is/c++draft/cmp.partialord). +Gecode's direction should be objective-relative (`BETTER` / `WORSE`) rather than +numeric. The project's CMake library targets currently require `cxx_std_17`, so +using `std::partial_ordering` would unnecessarily couple this change to a language +upgrade. A small enum is adequate. The fifth proposed value is unavailable +comparison, not a fifth relation. + +## Why incomparability is more than a collector policy + +For two minimization objectives, `(1,5)` and `(5,1)` are incomparable under +Pareto dominance. Neither is an equal-quality substitute for the other. A full +Pareto engine needs an incumbent set, cuts excluding dominated regions without +excluding other frontier points, and a policy for withdrawing previously emitted +points if a later point dominates them. A comparator is useful infrastructure +but does not provide any of this. + +Even a promise of finding just one maximal solution needs care with the existing +parallel portfolio protocol. Consider four feasible points: + +| Point | Objectives | Role | +| --- | --- | --- | +| A | `(6,6)` | An old shared incumbent. | +| B | `(5,1)` | One asset's new local incumbent. | +| C | `(1,5)` | The concurrently retained global incumbent. | +| D | `(0,4)` | A feasible improvement of C that does not dominate B. | + +B and C both dominate A but not each other. An asset already constrained to +dominate B can exclude D. Adding the cut for C does not restore D. If the +collector discards B as an incomparable candidate and that asset subsequently +exhausts, its exhaustion does not prove C maximal. In +[`Par::PBS::report`](../../../gecode/search/par/pbs.hpp), an exhausted asset +stops the current run; `next()` can then return no solution without reporting a +limit stop. Treating incomparability as a tie would therefore overstate what +the portfolio has established. + +A purpose-built dominance-chain engine can return one maximal point without +enumerating a frontier. That possibility does not justify silently applying +the existing total-objective portfolio protocol to partial orders. Recommend +representing incomparability and rejecting it in these engines for this PR. + +## Float steps expose a second policy + +The six objective bases in +[`minimodel/optimize.cpp`](../../../gecode/minimodel/optimize.cpp) include +integer scalar and lexicographic minimization/maximization, and two float bases. +The float cuts use `best.cost().max() - step` for minimization and +`best.cost().min() + step` for maximization, with strict relations. + +[`FloatVarImp::assigned()`](../../../gecode/float/var-imp/float.hpp) delegates +to [`FloatVal::tight()`](../../../gecode/float/val.hpp), which accepts a +singleton or adjacent representable endpoints. An assigned float is not always +an exact real singleton. Using an undocumented midpoint would introduce a new +policy without respecting the current endpoint-based bounds. + +With exact singleton costs, let an incumbent cost be 10, a candidate cost be +9.5, and step be 1. The candidate is numerically better, but it does not satisfy +the incumbent's `< 9` improvement restriction. The current forward BAB probe +rejects it. The current reverse PBS probe constrains 10 to `< 8.5`, fails, and +accepts 9.5. There is already a policy difference between these paths; it cannot +be preserved by one uniform better/equal/worse test. + +Treating values within a step as equivalent does not solve this. With step 1, +costs 0 and 0.75 would be equivalent, as would 0.75 and 1.5, but 0 and 1.5 would +not. Equivalence in a weak ordering must be transitive; see the +[C++ strict weak order requirements](https://eel.is/c++draft/concept.strictweakorder). +Such a tolerance relation must not be advertised as ordinary equality. + +Recommend comparing conservative endpoint quality independently of the step, +while keeping step-based pruning. A concurrent sub-step improvement can then +be retained, and applying its threshold still tightens future search under a +common step policy. This makes ranking uniform, but changes which results may +be reported. Preserve a reporting step only through an explicitly separate +admission policy, if that behavior is required. + +FlatZinc is a required follow-through, not an integer-only afterthought: +[`FlatZincSpace::constrain()`](../../../gecode/flatzinc/flatzinc.cpp) uses an +interval-valued `val() +/- step` threshold for floats, rather than exactly the +MiniModel scalar endpoint expression. Its rounding and boundary behavior need +focused checks before the recommendation becomes an implementation decision. +The relevant relation posting is in +[`float/rel.cpp`](../../../gecode/float/rel.cpp); strict relations combine +a bound restriction with disequality. Do not rewrite them as an assumed +non-strict minimum-step comparison. + +There is a concrete reason not to copy the MiniModel key mechanically: +[`NqFloat::post()`](../../../gecode/float/rel/nq.hpp) rejects an assigned +interval overlapping the threshold interval. For minimization at step zero, +an incumbent `[l,u]` with adjacent endpoints excludes the candidate singleton +`[l,l]` in FlatZinc, whereas a singleton incumbent `[u,u]` admits it. Both +incumbents have the same upper bound. Thus upper-bound equality alone would +claim equivalence between different FlatZinc cuts. The implementation must choose +a key compatible with that cut or obtain agreement to align the float policies. +This is an unresolved detail within the float decision, not evidence against a +model-defined comparison method. + +## Interface alternatives and compatibility + +| Alternative | Assessment | +| --- | --- | +| `bool better(const Space&) const` | Enough for one total-objective acceptance decision, but cannot distinguish equivalent from incomparable without more calls or methods. | +| Three ordering values only | The smallest total-objective interface, but cannot report the partial-order distinction raised in the proposal. | +| Four ordering values plus `UNAVAILABLE` | Recommended: one virtual, explicit direction and incomparability, no pure virtual requirement on satisfaction spaces. | +| Four ordering values plus a throwing default | Plausible, but missing support first encountered on a worker must be forwarded safely; the current thread runner cannot do that. | +| A separate capability virtual or optimization base | Adds a second piece of model configuration, or moves a Space-level search operation behind another abstraction. Not needed solely for this change. | +| Extract a serialized or type-erased objective key | Useful for distributed search, but unnecessary machinery for two in-process spaces. | +| Infer comparison by clone/constrain/status | Potentially expensive and undecided; keep only as a clearly limited legacy behavior if compatibility requires it. | + +A virtual added to `Space` changes the ABI. A non-pure default avoids source +breakage in every satisfaction model, but cannot magically provide a meaningful +comparison for old custom optimization models. The brief recommends a Gecode 7 +runtime requirement at incumbent-arbitration boundaries and preserves standalone +sequential progression. Whether to keep a legacy concurrent path is a user +choice, not something the implementation should decide by catching arbitrary +exceptions or interpreting incomparability as missing support. + +An inherited comparison also deserves attention: an application may derive from +`IntMinimizeSpace` but override `constrain()` to optimize a different criterion. +It must override comparison as well; the engine cannot validate arbitrary +logical consistency. State this alongside the existing obligation to provide +a sound improvement restriction. + +## Integration details worth retaining + +- [`seq/bab.hpp`](../../../gecode/search/seq/bab.hpp), `BAB::next`, already + obtains solutions under the incumbent restriction. Adding comparison to every + sequential solution is unnecessary for ordinary execution and would disrupt + constrain-only progression. +- [`seq/path.hpp`](../../../gecode/search/seq/path.hpp) and + [`par/path.hpp`](../../../gecode/search/par/path.hpp) apply the incumbent at + the recomputation mark. Those calls must remain constraints. +- `Par::BAB::Worker::better` clones the incumbent and constrains current work. + Const comparison does not make clone/update internals safe for concurrent + mutation. Preserve the existing synchronization around ownership and cloning. +- `Par::PBS::CollectBest` retains a solution and a `reporter` pointer. Its `get` + clones that solution; a rejected equal or worse candidate must not overwrite + `reporter` or trigger a new propagation broadcast. +- [`par/engine.hpp`](../../../gecode/search/par/engine.hpp), `Engine::next`, + returns already queued solutions before resuming work. External incumbent + updates need a deliberate check of that pending-result path. +- [`support/thread/thread.cpp`](../../../gecode/support/thread/thread.cpp) + calls `Runnable::run()` on detached threads without a catch boundary. + `Par::BAB` and `Par::PBS` also hold manually managed locks around arbitration. + A default throwing comparison inserted at these points is not a complete + implementation. Comparison errors need safe reporting and nesting behavior. +- [`driver.hh`](../../../gecode/driver.hh), `Driver::ScriptBase`, already + declares `compare(const Space&, std::ostream&) const` for Gist display. + Bring the inherited one-argument overload into scope with a suitable using + declaration. [`flatzinc.hh`](../../../gecode/flatzinc.hh) has display + overloads too; retain their behavior and overload visibility. Virtual search + calls through `Space` must dispatch to the objective comparison. Custom scripts + overriding the new one-argument method may themselves need `using + Script::compare` (or their actual base name): + [`Gist::VarComparator`](../../../gecode/gist/gist.hpp) calls the display + overload through `S`, where a new derived overload would otherwise hide it. + +## Existing validation seams + +[`test/search.cpp`](../../../test/search.cpp) has BAB, RBS, PBS, and mixed +search-builder portfolio tests, with recomputation distances, stop/resume, and +thread choices already represented. Extend selected patterns instead of +duplicating the whole matrix. The balanced objective is a useful non-scalar +example; tests that only check a final optimum need focused companions for +tie handling, external updates, and unsupported outcomes. + +[`CMakeLists.txt`](../../../CMakeLists.txt) defines `gecode-test`, a CTest +build fixture, and the normal check arguments. Tests are named `Search::...`; +[`test/test.cpp`](../../../test/test.cpp) treats `-test '^Search::'` as a +prefix filter. The `-threads` runner option is separate from search-thread +options inside each test. No new validation framework is needed. diff --git a/.zdev/comparison/brief.md b/.zdev/comparison/brief.md new file mode 100644 index 0000000000..95af3f30c2 --- /dev/null +++ b/.zdev/comparison/brief.md @@ -0,0 +1,260 @@ +# Non-mutating solution comparison + +## Objective + +Give Gecode 7 a model-defined, non-mutating way to compare solution quality, +and use it wherever best-solution search arbitrates between independently +produced incumbents. A custom objective should work in parallel BAB and PBS +without posting constraints or propagating either solution to decide which +one to retain. + +This area is isolated on `feature/comparison`, based on `main` at +`6b7de57b0414fe2f8af3513743c9835840a3c019`. Its records have `pull-request` +ownership: review them on the branch and remove them with `zdev cleanup squash` +before squash merge. + +This brief is a design proposal. The requested exploration creates the area +and records the recommendations; it does not authorize implementation tasks. +The compatibility and float choices below remain recommendations pending +discussion. + +## Boundaries + +The proposed implementation PR includes the Space contract, standard objective +classes, FlatZinc optimization, incumbent arbitration in BAB/PBS/RBS, documentation, +and focused regression coverage. Include the small driver change needed to keep +the existing Gist display-comparison overload visible. + +Retain `constrain()` for pruning, recomputation, restarts, and model-defined +sequential solution progression. Do not replace it with a comparison of domain +bounds. Preserve the existing search ownership, synchronization, stopping, and +restart protocols except where comparison outcomes must reach the caller. + +Exclude Pareto-front enumeration, a new search engine, objective extraction or +serialization, a comparator registry, heterogeneous-model conversion, changing +the C++ language requirement, and a performance benchmark campaign. No slices +or tasks are needed for this exploration. + +## Terms and proposed interface + +An **incumbent** is a feasible solution retained as the reference for future +search. **Better** is relative to the model's objective, so a larger integer is +better for maximization. **Equivalent** means equal objective quality and an +interchangeable future improvement restriction, not equal assignments. +**Incomparable** is a known result under a partial ordering; it does not mean +that an objective is unassigned or comparison has not been implemented. + +Recommend one virtual member of `Space`, with receiver-relative results: + +```cpp +enum SpaceComparison { + SC_BETTER, + SC_EQUIVALENT, + SC_WORSE, + SC_INCOMPARABLE, + SC_UNAVAILABLE +}; + +virtual SpaceComparison compare(const Space& other) const; +``` + +The first four values describe the ordering. `SC_UNAVAILABLE` is explicitly +outside that ordering: no comparison was supplied, or the objective data is not +ready for the model's documented comparison. The default returns this value. +The exact enum spelling is provisional. This uses the current C++17 baseline +and needs neither an additional capability virtual nor a new objective base +class. Do not assign a useful ordering to `SC_UNAVAILABLE`. + +The method reads objective data only. It must not call `status()`, post a +constraint, clone either argument, alter domains or branchers, or change shared +model state. Small temporary allocations, such as existing `IntVarArgs` cost +accessors, need not be prohibited. The public method need not be `noexcept`; +normal valid comparisons must not throw. + +Operands must be stable, non-failed spaces with compatible objective meaning. +Comparison can work on an otherwise unfinished space when its objective is +already determined. Scalar and lexicographic integer helpers require assigned +objective variables; unrelated variables may remain unassigned. Missing +objective values return `SC_UNAVAILABLE`, without propagation. Do not require +every model variable to be fixed, and do not use `SS_SOLVED` as proof that the +objective is assigned. A caller passing an external incumbent to a search engine +must additionally supply a feasible solution, not merely a promising bound. + +All participating assets must use the same objective direction, component +interpretation, and pruning policy. Built-in comparisons should diagnose +incompatible objective families or lexicographic dimensions using the project's +existing error conventions; a successful cast alone cannot prove model +compatibility. Distinct model classes may cooperate through an intentionally +shared objective interface, but automatic conversion is outside this PR. + +For exact optimization, define `better(a,b)` by `a.compare(b) == SC_BETTER`. +It must be irreflexive and transitive, reverse to `SC_WORSE`, and agree with +`constrain(b)` on objective-complete feasible solutions. Equivalence must be +transitive and preserve comparisons with every third solution. For the existing +single-incumbent engines, every pair must be better, worse, or equivalent. + +There is also a search requirement: if `a` replaces `b`, the solutions admitted +by the new improvement restriction must be a subset of those admitted by the +old one. Equivalent incumbents must admit the same future improvements. This +is what makes accumulated cuts, local incumbents, and portfolio exhaustion +compatible. A pairwise ranking alone cannot establish this property. + +## Search behavior + +Use the same orientation everywhere: compare the incoming solution against the +retained incumbent. Install the first valid incumbent; thereafter replace only +for `SC_BETTER`. Keep the existing incumbent for `SC_EQUIVALENT` and `SC_WORSE`. +This deliberately removes the current replacement of equal solutions at some +call sites; it does not promise a deterministic parallel solution sequence. + +| Boundary | Proposed change | +| --- | --- | +| Parallel BAB `solution()` | Compare candidate with global incumbent before cloning, broadcasting, or enqueuing it; never constrain the completed candidate as a comparison probe. | +| Sequential and parallel BAB `constrain()` | Compare an incoming external incumbent before replacing the retained solution; keep the existing pruning of current spaces and recomputation marks. | +| Parallel PBS `CollectBest::add()` and `constrain()` | Select the winner without modifying the retained solution; update `reporter` only when installing a new incumbent. | +| RBS `constrain()` | Compare with `last` before updating it and forwarding the new bound to the master and child engine. | +| Sequential BAB `next()` and sequential PBS scheduling | Keep their existing control flow: sequential BAB already searches under the incumbent cut; sequential PBS forwards incumbents to its assets. | +| Path recomputation, stolen work, worker `better()`, restart `master()` | Continue calling `constrain()` on search spaces. These are pruning operations, not solution arbitration. | + +Preserve accepted-solution queue order and ownership. Audit queued parallel BAB +solutions when accepting an external incumbent: no queued result returned after +the update may violate the new incumbent contract. Filtering those pending +solutions is in scope if required; changing queue policy otherwise is not. + +`SC_INCOMPARABLE` is expressible for direct model use and a future Pareto engine, +but existing best-solution engines must report unsupported use. Do not silently +treat it as a tie, choose an arbitrary winner, or claim Pareto completeness. +Partial orders can support finding a single maximal solution in a deliberately +designed dominance-chain search, but that is not the current PBS asset protocol. + +Missing or unsupported comparison must produce a caller-visible diagnostic, +not normal exhaustion, a hang, or termination of a detached worker thread. +Check the first incumbent's self-comparison where needed to expose unavailable +support even before a second solution arrives. No propagation is allowed for +this check. Parallel reporting must stop/wake safely and deliver the error at +the controlling call after workers are quiescent. Nested PBS/RBS/BAB composition +must carry it through worker boundaries. Limit this work to comparison failures; +do not turn it into general exception-safety refactoring. + +## Standard objectives and migration + +Implement direct scalar and lexicographic integer comparison in all four +MiniModel integer optimization bases, plus integer optimization in +`FlatZincSpace`. Compare values with relational operators, not subtraction. +Respect lexicographic order and existing vector-length semantics; validate +compatible dimensions rather than accidentally comparing different objectives. +Models overriding an inherited `constrain()` with a different objective must +also override its inherited comparison. + +Recommended Gecode 7 compatibility policy: require comparison for incumbent +arbitration. Preserve constrain-only standalone sequential BAB/RBS operation, +including non-ordering uses such as `cartesian-heart`. Existing models still +compile because the new virtual is not pure, but a model used in a portfolio, +parallel BAB, or external-incumbent arbitration must supply comparison. DFS and +other satisfaction-only paths do not require it. Document the runtime migration +and ordinary ABI impact of adding a virtual function. + +The alternative is an explicitly documented legacy path for `SC_UNAVAILABLE`. +It preserves more existing executions but retains propagation-dependent +arbitration and needs a clearly stated limit on the new guarantee. Do not put +clone/constrain probing inside `Space::compare()` or call such a fallback a +general comparison algorithm. The choice is still open. + +### Floating-point recommendation + +Separate objective ranking from the improvement step. For the existing float +bases, propose ranking the upper cost bound for minimization and the lower cost +bound for maximization, matching the incumbent endpoint used in their cuts. +Keep the step in `constrain()`, with a common step policy across assets. Compare +assigned float variables using Gecode's meaning of assigned, which can include +an interval between adjacent representable values. + +This is a bound-quality ordering, not a claim to know the exact real value +inside an interval. It intentionally allows retaining a better concurrent +candidate whose improvement is smaller than the step. Equivalence uses equal +ranking keys; being within a step is not equivalence. The latter would be +non-transitive. Approximate pruning needs the subset property above, but need +not admit every solution ranked better. + +Before implementing this choice, reconcile it explicitly with FlatZinc's +interval-valued float threshold, strict relations, rounding, and exact threshold +boundaries. In particular, equal upper bounds do not necessarily give equivalent +FlatZinc minimization cuts when the lower bounds differ. Its comparator needs a +cut-compatible key; do not copy the MiniModel endpoint rule without resolving +this difference. Preserve the posted improvement constraints unless the user +approves a change. If the step must instead hold between reported solutions, this +ordering alone is insufficient: a separate admission policy or a bounded legacy +float path must be chosen. Do not quietly redefine comparison as a tolerance +test. Float behavior is a material open decision, not an implementation detail. + +## Open questions + +1. Adopt the proposed Gecode 7 comparison requirement, or retain an explicit + legacy arbitration fallback for constrain-only models? +2. Adopt float bound ranking with step-based pruning, allowing improvements + smaller than the step among concurrent results, or preserve the reporting + step through an additional policy? Recommend the former, subject to focused + interval-boundary checks. +3. Confirm the proposed partial-order result with rejection in existing engines. + A Pareto-front engine is separate work; if it is required here, reshape the + area before splitting implementation tasks. + +The optional questions in the shaping conversation have not yet been answered. +These recommendations are concrete enough to review, but are not settled user +decisions. + +## Testing + +For this exploration: existing structural checks only. No production code or +tests change, and no build is needed to validate a planning artifact. + +For implementation: focused coverage in the existing search test suite, then +the existing search regressions. The observable risks justify these cases: + +- Direct scalar, lexicographic, and custom-objective comparisons: direction, + equal quality, unassigned objective data, and unchanged input domains/state. + Include an objective fixed in an otherwise unfinished space. +- Better/equal/worse external incumbent updates, including an older incumbent + arriving late and pending parallel results. Returned results must not regress. +- A small custom non-scalar objective through BAB, PBS, and a portfolio with an + RBS/BAB asset. Check improvement and the known optimum without requiring a + fixed thread schedule or solution count. Reuse existing engine test patterns. +- Missing comparison and an incomparable result must fail cleanly in a direct + and a nested parallel path. A focused termination check is necessary because + the current thread runner has no exception forwarding. +- If float ranking is accepted: zero/nonzero step, improvement smaller than the + step, the strict threshold, and tight non-singleton intervals in MiniModel + and FlatZinc. Check the agreed ranking/pruning distinction explicitly. +- Compile the ordering and Gist display overloads together in a representative + script; a model overriding one overload must not hide the other from Gist. + +Keep constrain-only sequential behavior covered by existing tests and one +relevant example smoke check. Do not multiply all new cases across the existing +large recomputation/thread matrix. Do not add a harness, general property-testing +layer, timing assertions, or benchmarks merely to justify cheap comparison. + +## Validation and completion + +The shaping deliverable is this brief, its indexed research note, and a passing +`zdev check comparison --format json`. No tasks are imported during exploration. + +Implementation is complete when every incumbent arbitration site above uses +the agreed contract, standard objectives and migration behavior are covered, +unsupported outcomes reach callers safely, and the focused plus existing search +checks pass. Include documentation of objective readiness, ties, incomparability, +and `compare()`/`constrain()` consistency. + +The repository provides CMake's `gecode-test` target and a `-test` prefix filter. +For example, configure a build under `build/comparison` with `BUILD_TESTING=ON`, +build `gecode-test`, then run `build/comparison/gecode-test -test '^Search::' +-iter 2 -threads 1`. The final flag serializes test runners; individual tests +still exercise parallel search. Retain normal variable features and run the +relevant FlatZinc checks if its comparison changes. Use the existing build/CI +setup for optional dependencies; do not invent a second test configuration. + +## Background + +- [Comparison and search analysis](background/comparison-and-search.md): source + map, the limits of constrain-based comparison, ordering/cut requirements, + Pareto and float counterexamples, interface alternatives, and implementation + seams. This brief remains the authoritative proposal. diff --git a/.zdev/config.toml b/.zdev/config.toml new file mode 100644 index 0000000000..4b1cd6b487 --- /dev/null +++ b/.zdev/config.toml @@ -0,0 +1,6 @@ +schema_version = 1 + +[project] +name = "gecode" +record = "pull-request" +trunk = "main" From 6e8cacc79e55d59961f5b65ea7ee77562ca69aa7 Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Thu, 10 Sep 2026 09:00:04 +0200 Subject: [PATCH 02/15] comparison: require optimization comparison and refine objective readiness Zdev-Change-Id: Z5c772894bb48c0852974820982b13020d663b002850618c6bcba63a53e9e4571 --- .../background/comparison-and-search.md | 47 ++++++--- .zdev/comparison/brief.md | 98 ++++++++++++------- 2 files changed, 93 insertions(+), 52 deletions(-) diff --git a/.zdev/comparison/background/comparison-and-search.md b/.zdev/comparison/background/comparison-and-search.md index 4f43dcd28d..db9a8ce305 100644 --- a/.zdev/comparison/background/comparison-and-search.md +++ b/.zdev/comparison/background/comparison-and-search.md @@ -53,8 +53,10 @@ representation. directions. Its disjunction allows either direction of movement in the second coordinate; sufficiently separated points can satisfy both directed tests. This is not an asymmetric improvement order, yet the example uses sequential BAB. -Changing `Space` to require a meaningful order for all BAB uses would lose an -existing useful model pattern. +It represents a different search contract: diversity must account for all +previous solutions, not merely the latest incumbent. Gecode 7 may impose new +requirements on optimization spaces, so preserving this use through an +unavailable comparison result or legacy arbitration path is not required. Other limits follow from the same distinction: diversity enumeration, an incumbent-dependent neighborhood, changing objective weights, or a policy that @@ -74,6 +76,22 @@ space with an assigned cost and unfinished auxiliary search has a determinate objective key, although it is not necessarily a feasible incumbent to give search. The distinction belongs in the public documentation. +The recommended readiness contract follows the existing value accessor: +[`IntVar::val()`](../../../gecode/int/var/int.hpp) explicitly throws +`Int::ValOfUnassignedVar` when its variable is unassigned. Integer convenience +`constrain()` methods use `val()` on the incumbent objective. Comparison would +require determined objective data on both operands and use the same error +convention. It would not require unrelated variables to be assigned. + +An explicit undetermined result is a viable alternative for a caller intending +to inspect partial objectives and recover without an exception. In the existing +search consumers, however, an undetermined incumbent is still an error: +discarding it may lose a valid improvement, accepting it cannot establish +betterness, and calling `status()` to resolve it violates the comparison +contract. Without such a caller, this extra result adds no useful search +behavior. Readiness remains an open decision in the brief; a precondition is +the recommendation. + Comparing arbitrary domains asks a different question. For example, minimization domains `[1,4]` and `[3,6]` overlap: comparing minima ranks bounds, not the eventual solutions. Some disjoint domains admit a guaranteed relation, but adding a generic @@ -106,8 +124,9 @@ less, equivalent, greater, and unordered in the Gecode's direction should be objective-relative (`BETTER` / `WORSE`) rather than numeric. The project's CMake library targets currently require `cxx_std_17`, so using `std::partial_ordering` would unnecessarily couple this change to a language -upgrade. A small enum is adequate. The fifth proposed value is unavailable -comparison, not a fifth relation. +upgrade. A small enum is adequate. Undetermined objective data and an absent +comparison implementation are outside this ordering; the current recommendation +reports them as errors rather than adding a fifth relation. ## Why incomparability is more than a collector policy @@ -203,19 +222,19 @@ model-defined comparison method. | --- | --- | | `bool better(const Space&) const` | Enough for one total-objective acceptance decision, but cannot distinguish equivalent from incomparable without more calls or methods. | | Three ordering values only | The smallest total-objective interface, but cannot report the partial-order distinction raised in the proposal. | -| Four ordering values plus `UNAVAILABLE` | Recommended: one virtual, explicit direction and incomparability, no pure virtual requirement on satisfaction spaces. | -| Four ordering values plus a throwing default | Plausible, but missing support first encountered on a worker must be forwarded safely; the current thread runner cannot do that. | +| Four ordering values plus an undetermined result | Useful only if callers deliberately compare partial objectives and need a recoverable outcome; existing search would still have to reject such incumbents. | +| Four ordering values with objective readiness as a precondition | Recommended: comparison results describe valid comparisons; missing support and unassigned objectives follow error conventions. A non-pure throwing default can leave satisfaction models unaffected. | | A separate capability virtual or optimization base | Adds a second piece of model configuration, or moves a Space-level search operation behind another abstraction. Not needed solely for this change. | | Extract a serialized or type-erased objective key | Useful for distributed search, but unnecessary machinery for two in-process spaces. | -| Infer comparison by clone/constrain/status | Potentially expensive and undecided; keep only as a clearly limited legacy behavior if compatibility requires it. | +| Infer comparison by clone/constrain/status | Potentially expensive and undecided; no compatibility requirement justifies it for this Gecode 7 change. | A virtual added to `Space` changes the ABI. A non-pure default avoids source breakage in every satisfaction model, but cannot magically provide a meaningful -comparison for old custom optimization models. The brief recommends a Gecode 7 -runtime requirement at incumbent-arbitration boundaries and preserves standalone -sequential progression. Whether to keep a legacy concurrent path is a user -choice, not something the implementation should decide by catching arbitrary -exceptions or interpreting incomparability as missing support. +comparison for old custom optimization models. The settled Gecode 7 direction +permits requiring comparison on optimization spaces. Most models inherit scalar +integer comparison from the existing convenience classes; custom objectives +must implement it. No legacy concurrent fallback is needed. This leaves the +readiness contract as a separate choice rather than conflating it with migration. An inherited comparison also deserves attention: an application may derive from `IntMinimizeSpace` but override `constrain()` to optimize a different criterion. @@ -227,8 +246,8 @@ a sound improvement restriction. - [`seq/bab.hpp`](../../../gecode/search/seq/bab.hpp), `BAB::next`, already obtains solutions under the incumbent restriction. Adding comparison to every - sequential solution is unnecessary for ordinary execution and would disrupt - constrain-only progression. + sequential solution is unnecessary for ordinary execution. Not calling it + there does not require a separate compatibility policy for optimization models. - [`seq/path.hpp`](../../../gecode/search/seq/path.hpp) and [`par/path.hpp`](../../../gecode/search/par/path.hpp) apply the incumbent at the recomputation mark. Those calls must remain constraints. diff --git a/.zdev/comparison/brief.md b/.zdev/comparison/brief.md index 95af3f30c2..772fbd9766 100644 --- a/.zdev/comparison/brief.md +++ b/.zdev/comparison/brief.md @@ -15,8 +15,9 @@ before squash merge. This brief is a design proposal. The requested exploration creates the area and records the recommendations; it does not authorize implementation tasks. -The compatibility and float choices below remain recommendations pending -discussion. +Gecode 7 may impose new comparison requirements on optimization spaces; a +legacy arbitration fallback is not required. Objective readiness, float behavior, +and partial-order handling below remain proposals pending discussion. ## Boundaries @@ -25,16 +26,21 @@ classes, FlatZinc optimization, incumbent arbitration in BAB/PBS/RBS, documentat and focused regression coverage. Include the small driver change needed to keep the existing Gist display-comparison overload visible. -Retain `constrain()` for pruning, recomputation, restarts, and model-defined -sequential solution progression. Do not replace it with a comparison of domain -bounds. Preserve the existing search ownership, synchronization, stopping, and +Retain `constrain()` for pruning, recomputation, and restarts. Do not replace it +with a comparison of domain bounds. Preserve the existing search ownership, +synchronization, stopping, and restart protocols except where comparison outcomes must reach the caller. -Exclude Pareto-front enumeration, a new search engine, objective extraction or -serialization, a comparator registry, heterogeneous-model conversion, changing +Exclude diverse-solution search, Pareto-front enumeration, a new search engine, +objective extraction or serialization, a comparator registry, +heterogeneous-model conversion, changing the C++ language requirement, and a performance benchmark campaign. No slices or tasks are needed for this exploration. +Diverse-solution search needs restrictions with respect to all previous +solutions, rather than comparison with one incumbent. It is a separate search +contract and does not justify weakening the optimization comparison requirement. + ## Terms and proposed interface An **incumbent** is a feasible solution retained as the reference for future @@ -51,19 +57,20 @@ enum SpaceComparison { SC_BETTER, SC_EQUIVALENT, SC_WORSE, - SC_INCOMPARABLE, - SC_UNAVAILABLE + SC_INCOMPARABLE }; virtual SpaceComparison compare(const Space& other) const; ``` -The first four values describe the ordering. `SC_UNAVAILABLE` is explicitly -outside that ordering: no comparison was supplied, or the objective data is not -ready for the model's documented comparison. The default returns this value. -The exact enum spelling is provisional. This uses the current C++17 baseline -and needs neither an additional capability virtual nor a new objective base -class. Do not assign a useful ordering to `SC_UNAVAILABLE`. +Recommend keeping the result restricted to ordering outcomes. Missing comparison +support and undetermined objective data are errors, not ordering results. The +readiness decision is still open; this is the recommended interface if the +precondition is adopted. The exact enum spelling is provisional. This uses the +current C++17 baseline and needs neither an additional capability virtual nor a +new objective base class. A non-pure default that reports unsupported use can +leave satisfaction-only spaces unaffected; optimization spaces must provide +the comparison, usually through a convenience base. The method reads objective data only. It must not call `status()`, post a constraint, clone either argument, alter domains or branchers, or change shared @@ -74,12 +81,26 @@ normal valid comparisons must not throw. Operands must be stable, non-failed spaces with compatible objective meaning. Comparison can work on an otherwise unfinished space when its objective is already determined. Scalar and lexicographic integer helpers require assigned -objective variables; unrelated variables may remain unassigned. Missing -objective values return `SC_UNAVAILABLE`, without propagation. Do not require +objective variables on both operands; unrelated variables may remain unassigned. +Recommend treating missing objective values as a precondition violation, using +the existing unassigned-value error convention without propagation. Do not require every model variable to be fixed, and do not use `SS_SOLVED` as proof that the objective is assigned. A caller passing an external incumbent to a search engine must additionally supply a feasible solution, not merely a promising bound. +`a.constrain(b)` requires determined objective data in `b` while pruning the +unfinished receiver `a`; `a.compare(b)` requires determined data in both. +For integer lexicographic convenience classes, require every cost component +to be assigned, rather than making readiness depend on where the first +difference happens to occur. Custom objectives define which data determines +their quality; that need not be a stored objective variable. + +The alternative is a distinct `SC_UNDETERMINED` outcome for callers deliberately +comparing partial objectives. It would not mean Pareto incomparability, and +search would still have to report an error when given an undetermined incumbent. +No current search consumer needs a recoverable partial-objective result, so +recommend the precondition instead of adding this state speculatively. + All participating assets must use the same objective direction, component interpretation, and pruning policy. Built-in comparisons should diagnose incompatible objective families or lexicographic dimensions using the project's @@ -129,9 +150,9 @@ designed dominance-chain search, but that is not the current PBS asset protocol. Missing or unsupported comparison must produce a caller-visible diagnostic, not normal exhaustion, a hang, or termination of a detached worker thread. -Check the first incumbent's self-comparison where needed to expose unavailable -support even before a second solution arrives. No propagation is allowed for -this check. Parallel reporting must stop/wake safely and deliver the error at +Do not add a capability protocol or routine self-comparison merely to discover +whether a required override exists. Parallel reporting must stop/wake safely +and deliver the error at the controlling call after workers are quiescent. Nested PBS/RBS/BAB composition must carry it through worker boundaries. Limit this work to comparison failures; do not turn it into general exception-safety refactoring. @@ -146,19 +167,19 @@ compatible dimensions rather than accidentally comparing different objectives. Models overriding an inherited `constrain()` with a different objective must also override its inherited comparison. -Recommended Gecode 7 compatibility policy: require comparison for incumbent -arbitration. Preserve constrain-only standalone sequential BAB/RBS operation, -including non-ordering uses such as `cartesian-heart`. Existing models still -compile because the new virtual is not pure, but a model used in a portfolio, -parallel BAB, or external-incumbent arbitration must supply comparison. DFS and -other satisfaction-only paths do not require it. Document the runtime migration +Settled Gecode 7 direction: optimization spaces may be required to supply +comparison; no legacy clone/constrain arbitration fallback is needed. The +ordinary integer minimization and maximization cases inherit it from the existing +convenience classes. Custom optimization models implement the same contract. +DFS and other satisfaction-only paths do not require it. Document the migration and ordinary ABI impact of adding a virtual function. -The alternative is an explicitly documented legacy path for `SC_UNAVAILABLE`. -It preserves more existing executions but retains propagation-dependent -arbitration and needs a clearly stated limit on the new guarantee. Do not put -clone/constrain probing inside `Space::compare()` or call such a fallback a -general comparison algorithm. The choice is still open. +The ordinary sequential BAB loop need not call comparison merely to enforce +this requirement: it already searches under the incumbent cut. Preserving old +non-ordering BAB uses is not a design constraint. Treat `cartesian-heart` as +evidence of a separate diverse-solution use, not a reason for an unavailable +comparison result or a compatibility path. Its all-previous-solutions search +semantics are outside this PR. ### Floating-point recommendation @@ -189,8 +210,10 @@ test. Float behavior is a material open decision, not an implementation detail. ## Open questions -1. Adopt the proposed Gecode 7 comparison requirement, or retain an explicit - legacy arbitration fallback for constrain-only models? +1. Require determined objectives on both operands and report violations using + the existing error convention, or return a separate undetermined result? + Recommend the precondition: the search consumers cannot recover from an + undetermined incumbent, and unrelated variables may remain unassigned. 2. Adopt float bound ranking with step-based pruning, allowing improvements smaller than the step among concurrent results, or preserve the reporting step through an additional policy? Recommend the former, subject to focused @@ -199,9 +222,8 @@ test. Float behavior is a material open decision, not an implementation detail. A Pareto-front engine is separate work; if it is required here, reshape the area before splitting implementation tasks. -The optional questions in the shaping conversation have not yet been answered. -These recommendations are concrete enough to review, but are not settled user -decisions. +The Gecode 7 requirement and exclusion of diverse-solution search are settled. +The choices listed here remain recommendations, not settled user decisions. ## Testing @@ -228,8 +250,8 @@ the existing search regressions. The observable risks justify these cases: - Compile the ordering and Gist display overloads together in a representative script; a model overriding one overload must not hide the other from Gist. -Keep constrain-only sequential behavior covered by existing tests and one -relevant example smoke check. Do not multiply all new cases across the existing +Keep ordinary sequential optimization covered by existing tests. Do not +multiply all new cases across the existing large recomputation/thread matrix. Do not add a harness, general property-testing layer, timing assertions, or benchmarks merely to justify cheap comparison. From 482099cc8337d806e51fd8aaca0203c71b7a7467 Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Thu, 10 Sep 2026 09:41:52 +0200 Subject: [PATCH 03/15] comparison: make comparability model-defined and plan future search uses Zdev-Change-Id: Zd3bd5210f7a14908a3b4cf47a040ce93d4488f82ec178872c8b90824895e3f87 --- .../background/comparison-and-search.md | 75 ++++++++--- .zdev/comparison/brief.md | 123 ++++++++++++------ 2 files changed, 141 insertions(+), 57 deletions(-) diff --git a/.zdev/comparison/background/comparison-and-search.md b/.zdev/comparison/background/comparison-and-search.md index db9a8ce305..8541d9b952 100644 --- a/.zdev/comparison/background/comparison-and-search.md +++ b/.zdev/comparison/background/comparison-and-search.md @@ -70,33 +70,74 @@ no brancher has alternatives left. It does not inspect every variable for assignment. This is why a generic solved-space check cannot validate cost data. Calling it inside a const comparison would also introduce propagation. -For integer helpers, requiring assigned objective values is sufficient; requiring -all variables to be assigned is unnecessary and can reject valid models. A stable -space with an assigned cost and unfinished auxiliary search has a determinate -objective key, although it is not necessarily a feasible incumbent to give search. -The distinction belongs in the public documentation. - -The recommended readiness contract follows the existing value accessor: +The individual space defines what information suffices to establish a relation. +Even requiring all objective components to be assigned would be too strong as +a generic rule: a lexicographic model can establish a strict ordering from a +decisive prefix, regardless of the remaining components. A derived model property +can likewise be comparable without a stored, assigned objective variable. +Comparability may depend on the pair, not on a unary readiness flag. + +For scalar integer convenience classes, the existing value accessor is a natural +implementation: [`IntVar::val()`](../../../gecode/int/var/int.hpp) explicitly throws `Int::ValOfUnassignedVar` when its variable is unassigned. Integer convenience -`constrain()` methods use `val()` on the incumbent objective. Comparison would -require determined objective data on both operands and use the same error -convention. It would not require unrelated variables to be assigned. +`constrain()` methods use `val()` on the incumbent objective. Scalar comparison +can use it on both costs. This is that class's contract, not an assignment +requirement on all implementations of `Space::compare()`. An explicit undetermined result is a viable alternative for a caller intending to inspect partial objectives and recover without an exception. In the existing -search consumers, however, an undetermined incumbent is still an error: +search consumers, however, inability to establish a required comparison is +still an error: discarding it may lose a valid improvement, accepting it cannot establish betterness, and calling `status()` to resolve it violates the comparison contract. Without such a caller, this extra result adds no useful search -behavior. Readiness remains an open decision in the brief; a precondition is -the recommendation. +behavior. How to report insufficient information remains an open decision; +a model-defined precondition is the recommendation. A generic assignment or +`SS_SOLVED` test must not stand in for the model's judgment. Comparing arbitrary domains asks a different question. For example, minimization domains `[1,4]` and `[3,6]` overlap: comparing minima ranks bounds, not the eventual -solutions. Some disjoint domains admit a guaranteed relation, but adding a generic -domain comparison would need to distinguish unknown from true incomparability -and would not replace propagation-based pruning. Defer that interface. +solutions. Some disjoint domains admit a guaranteed relation, and an individual +model may report that relation when sound under its contract. No generic domain +comparison machinery is needed. Distinguish insufficient information from true +incomparability and leave propagation-based pruning in `constrain()`. + +## Advance planning for other search policies + +The compatibility check for this PR is whether the pairwise interface remains +usable when the engine retains more than one solution. It should: `compare(a,b)` +describes a model-defined relation, not whether either argument is the one +global incumbent. The current single-incumbent engines impose additional cut +nesting requirements; those must not be presented as requirements of all future +search policies. + +In diverse-solution search, each accepted solution can contribute a restriction +against accepting similar future solutions. If these restrictions accumulate +conjunctively, repeated model-specific calls can express them without a batch +interface. What changes is engine state: an archive of relevant solutions or +restrictions, replay during recomputation, and delivery of all restrictions to +assets. A worker that has seen only the latest accepted point is not generally +up to date. Two concurrently produced points may each differ from the old archive +but fail the diversity requirement with respect to each other, so acceptance +also needs coordination against the current archive. + +Ordering cannot substitute for a diversity relation: two spaces with equivalent +cost can be very different solutions. Conversely, adding distance or history to +the ordering return type would force unrelated policies together. Leave a future +diversity admission/restriction contract separate; no new methods are justified +in this PR merely to reserve names. + +A Pareto engine can reuse pairwise comparison against its frontier, with its own +rules for dominated, incomparable, and equivalent candidates. It also needs +frontier-preserving pruning and explicit output semantics when a later solution +dominates an earlier one. Objective equivalence does not itself choose whether +to retain one or many assignments. This is why the comparison result and the +engine's retention policy should remain distinct. + +These are design checks, not implementation commitments. They justify keeping +comparison pairwise, model-defined, and independent of solution-history storage; +they do not require a new archive class or engine now. ## Ordering and nested improvement restrictions @@ -223,7 +264,7 @@ model-defined comparison method. | `bool better(const Space&) const` | Enough for one total-objective acceptance decision, but cannot distinguish equivalent from incomparable without more calls or methods. | | Three ordering values only | The smallest total-objective interface, but cannot report the partial-order distinction raised in the proposal. | | Four ordering values plus an undetermined result | Useful only if callers deliberately compare partial objectives and need a recoverable outcome; existing search would still have to reject such incumbents. | -| Four ordering values with objective readiness as a precondition | Recommended: comparison results describe valid comparisons; missing support and unassigned objectives follow error conventions. A non-pure throwing default can leave satisfaction models unaffected. | +| Four ordering values with model-defined comparability as a precondition | Recommended: results describe valid comparisons; missing support or insufficient information follows error conventions. A non-pure throwing default can leave satisfaction models unaffected. | | A separate capability virtual or optimization base | Adds a second piece of model configuration, or moves a Space-level search operation behind another abstraction. Not needed solely for this change. | | Extract a serialized or type-erased objective key | Useful for distributed search, but unnecessary machinery for two in-process spaces. | | Infer comparison by clone/constrain/status | Potentially expensive and undecided; no compatibility requirement justifies it for this Gecode 7 change. | diff --git a/.zdev/comparison/brief.md b/.zdev/comparison/brief.md index 772fbd9766..37202b373a 100644 --- a/.zdev/comparison/brief.md +++ b/.zdev/comparison/brief.md @@ -16,8 +16,9 @@ before squash merge. This brief is a design proposal. The requested exploration creates the area and records the recommendations; it does not authorize implementation tasks. Gecode 7 may impose new comparison requirements on optimization spaces; a -legacy arbitration fallback is not required. Objective readiness, float behavior, -and partial-order handling below remain proposals pending discussion. +legacy arbitration fallback is not required. Each space defines when it has +enough information to compare. How to report insufficient information, float +behavior, and partial-order handling remain proposals pending discussion. ## Boundaries @@ -28,25 +29,27 @@ the existing Gist display-comparison overload visible. Retain `constrain()` for pruning, recomputation, and restarts. Do not replace it with a comparison of domain bounds. Preserve the existing search ownership, -synchronization, stopping, and -restart protocols except where comparison outcomes must reach the caller. +synchronization, stopping, and restart protocols except where comparison +outcomes must reach the caller. -Exclude diverse-solution search, Pareto-front enumeration, a new search engine, +Exclude implementation of diverse-solution search, Pareto-front enumeration, +a new search engine, objective extraction or serialization, a comparator registry, heterogeneous-model conversion, changing the C++ language requirement, and a performance benchmark campaign. No slices or tasks are needed for this exploration. -Diverse-solution search needs restrictions with respect to all previous -solutions, rather than comparison with one incumbent. It is a separate search -contract and does not justify weakening the optimization comparison requirement. +Include limited advance planning for diverse-solution and Pareto search so this +interface leaves those uses possible. Their implementation and concrete APIs +remain separate work; see Future search below. ## Terms and proposed interface An **incumbent** is a feasible solution retained as the reference for future search. **Better** is relative to the model's objective, so a larger integer is -better for maximization. **Equivalent** means equal objective quality and an -interchangeable future improvement restriction, not equal assignments. +better for maximization. **Equivalent** means equivalent quality according to +the model, not equal assignments. Interchangeable improvement restrictions are +an additional requirement of the current single-incumbent optimization engines. **Incomparable** is a known result under a partial ordering; it does not mean that an objective is unassigned or comparison has not been implemented. @@ -64,7 +67,7 @@ virtual SpaceComparison compare(const Space& other) const; ``` Recommend keeping the result restricted to ordering outcomes. Missing comparison -support and undetermined objective data are errors, not ordering results. The +support and insufficient information to compare are errors, not ordering results. The readiness decision is still open; this is the recommended interface if the precondition is adopted. The exact enum spelling is provisional. This uses the current C++17 baseline and needs neither an additional capability virtual nor a @@ -78,28 +81,34 @@ model state. Small temporary allocations, such as existing `IntVarArgs` cost accessors, need not be prohibited. The public method need not be `noexcept`; normal valid comparisons must not throw. -Operands must be stable, non-failed spaces with compatible objective meaning. -Comparison can work on an otherwise unfinished space when its objective is -already determined. Scalar and lexicographic integer helpers require assigned -objective variables on both operands; unrelated variables may remain unassigned. -Recommend treating missing objective values as a precondition violation, using -the existing unassigned-value error convention without propagation. Do not require -every model variable to be fixed, and do not use `SS_SOLVED` as proof that the -objective is assigned. A caller passing an external incumbent to a search engine -must additionally supply a feasible solution, not merely a promising bound. - -`a.constrain(b)` requires determined objective data in `b` while pruning the -unfinished receiver `a`; `a.compare(b)` requires determined data in both. -For integer lexicographic convenience classes, require every cost component -to be assigned, rather than making readiness depend on where the first -difference happens to occur. Custom objectives define which data determines -their quality; that need not be a stored objective variable. - -The alternative is a distinct `SC_UNDETERMINED` outcome for callers deliberately -comparing partial objectives. It would not mean Pareto incomparability, and -search would still have to report an error when given an undetermined incumbent. -No current search consumer needs a recoverable partial-objective result, so -recommend the precondition instead of adding this state speculatively. +The generic contract is that the operands contain enough information for the +individual space to establish the comparison it reports. There is no generic +assignment test, requirement that every objective component have a value, or +requirement to call `status()`. Comparability can depend on the pair: a space +may know how it relates to one operand but lack information for another. +Reported relations must remain sound under the model's interpretation. + +For example, scalar integer convenience classes naturally compare assigned +cost values using `val()`. A lexicographic model can establish a strict relation +from a decisive prefix without knowing later components. A custom model may +compare a derived property or prove a relation from domains. These are model +contracts, not restrictions imposed by `Space` or a generic search preflight. +Do not conflate lack of enough information with genuine Pareto incomparability. + +Search invokes comparison on its stable solution spaces. Direct callers must +respect the individual space's documented comparison domain; the generic +interface does not require a completed search or fully assigned space. A space +being comparable does not by itself make it a valid incumbent: external +incumbents must also satisfy the engine's solution contract. Likewise, +`constrain()` retains its own model-defined requirements on its argument. + +Recommend treating insufficient information for the requested comparison as +a precondition violation, reported using the appropriate model error convention. +For scalar integer helpers, the existing unassigned-value exception fits. +The alternative is a distinct `SC_UNDETERMINED` result, allowing a caller to +recover when the model cannot establish the relation. The current arbitration +sites would still need an error policy for this result. The reporting choice +is open; model ownership of comparability is settled. All participating assets must use the same objective direction, component interpretation, and pruning policy. Built-in comparisons should diagnose @@ -120,6 +129,37 @@ old one. Equivalent incumbents must admit the same future improvements. This is what makes accumulated cuts, local incumbents, and portfolio exhaustion compatible. A pairwise ranking alone cannot establish this property. +## Future search + +Keep pairwise comparison separate from the policy for retaining solutions and +restricting future search. The comparison method must not assume that its +argument is the sole incumbent, consult an engine-owned solution history, or +encode whether a solution should be retained in a particular archive. + +For diverse-solution search, a future engine needs restrictions against all +previous accepted solutions. Repeated calls to a model-specific restriction may +suffice when they accumulate conjunctively; recomputed nodes and portfolio assets +must receive every relevant restriction, not just the latest one. Concurrent +candidates also need to be checked against the accepted history, including +each other, before both are returned. The archive and its acceptance policy +belong to that search design. Pairwise objective ordering does not answer +whether a point is sufficiently different from another, and equal objective +quality does not mean duplicate solutions. + +For Pareto search, a future engine can compare a candidate with each retained +frontier member, discard dominated candidates, and remove members dominated by +the candidate. Its pruning must preserve the rest of the frontier. Equivalent +quality need not imply keeping only one assignment; that is an engine policy. +These uses motivate retaining the distinction between equivalence and +incomparability, without adding history arguments, distance results, or an +archive abstraction to this PR. + +The nested-cut and equivalent-cut requirements above apply to the present +single-incumbent engines, not to every future consumer of comparison. Before +finalizing the interface, check that direct pairwise calls can support these +uses and that no global assignment or single-incumbent assumption has entered +the Space contract. Do not implement either future engine here. + ## Search behavior Use the same orientation everywhere: compare the incoming solution against the @@ -210,10 +250,11 @@ test. Float behavior is a material open decision, not an implementation detail. ## Open questions -1. Require determined objectives on both operands and report violations using - the existing error convention, or return a separate undetermined result? - Recommend the precondition: the search consumers cannot recover from an - undetermined incumbent, and unrelated variables may remain unassigned. +1. When a space lacks enough information for a requested comparison, should it + report a precondition violation or return a separate undetermined result? + Recommend the precondition for the present search consumers. In either case, + the individual space decides what information suffices; assignment is not + a generic requirement. 2. Adopt float bound ranking with step-based pruning, allowing improvements smaller than the step among concurrent results, or preserve the reporting step through an additional policy? Recommend the former, subject to focused @@ -222,7 +263,8 @@ test. Float behavior is a material open decision, not an implementation detail. A Pareto-front engine is separate work; if it is required here, reshape the area before splitting implementation tasks. -The Gecode 7 requirement and exclusion of diverse-solution search are settled. +The Gecode 7 requirement, model-defined comparability, and advance planning +without implementation of diverse-solution search are settled. The choices listed here remain recommendations, not settled user decisions. ## Testing @@ -234,8 +276,9 @@ For implementation: focused coverage in the existing search test suite, then the existing search regressions. The observable risks justify these cases: - Direct scalar, lexicographic, and custom-objective comparisons: direction, - equal quality, unassigned objective data, and unchanged input domains/state. - Include an objective fixed in an otherwise unfinished space. + equal quality, model-defined insufficient information, and unchanged inputs. + Include a valid model comparison with unassigned variables, such as a + decisive lexicographic prefix, to guard against generic assignment checks. - Better/equal/worse external incumbent updates, including an older incumbent arriving late and pending parallel results. Returned results must not regress. - A small custom non-scalar objective through BAB, PBS, and a portfolio with an From 7c3f4893811b08ad7753f6edc9b33d035fa378e7 Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Thu, 10 Sep 2026 10:05:12 +0200 Subject: [PATCH 04/15] Add tasks to comparison Zdev-Change-Id: Z1c007f6a8f418dab7835ce3c161f32811819842b08897f39166326aacd7afcb5 --- .zdev/comparison/TASKS.md | 11 +- .zdev/comparison/brief.md | 135 +++++++++--------- ...n-and-integer-objective-implementations.md | 34 +++++ ...y-independently-of-the-improvement-step.md | 34 +++++ ...quential-bab-and-restart-incumbent-upda.md | 34 +++++ ...rallel-bab-and-safely-report-comparison.md | 35 +++++ ...through-portfolios-and-complete-the-mig.md | 35 +++++ 7 files changed, 248 insertions(+), 70 deletions(-) create mode 100644 .zdev/comparison/tasks/001-add-space-comparison-and-integer-objective-implementations.md create mode 100644 .zdev/comparison/tasks/002-compare-float-quality-independently-of-the-improvement-step.md create mode 100644 .zdev/comparison/tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md create mode 100644 .zdev/comparison/tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md create mode 100644 .zdev/comparison/tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md diff --git a/.zdev/comparison/TASKS.md b/.zdev/comparison/TASKS.md index 9a8553a6cd..b2e92c565a 100644 --- a/.zdev/comparison/TASKS.md +++ b/.zdev/comparison/TASKS.md @@ -2,10 +2,15 @@ # Tasks: comparison -- Total: 0 -- Ready: 0 -- Blocked: 0 +- Total: 5 +- Ready: 1 +- Blocked: 4 - Done: 0 | ID | Task | State | Blocked by | | --- | --- | --- | --- | +| [comparison-001](tasks/001-add-space-comparison-and-integer-objective-implementations.md) | Add Space comparison and integer objective implementations | ready | — | +| [comparison-002](tasks/002-compare-float-quality-independently-of-the-improvement-step.md) | Compare float quality independently of the improvement step | blocked | comparison-001 | +| [comparison-003](tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md) | Use comparison for sequential BAB and restart incumbent updates | blocked | comparison-001 | +| [comparison-004](tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md) | Use comparison for parallel BAB and safely report comparison failures | blocked | comparison-002, comparison-003 | +| [comparison-005](tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md) | Integrate comparison through portfolios and complete the migration | blocked | comparison-004 | diff --git a/.zdev/comparison/brief.md b/.zdev/comparison/brief.md index 37202b373a..193bbb5b68 100644 --- a/.zdev/comparison/brief.md +++ b/.zdev/comparison/brief.md @@ -13,12 +13,10 @@ This area is isolated on `feature/comparison`, based on `main` at ownership: review them on the branch and remove them with `zdev cleanup squash` before squash merge. -This brief is a design proposal. The requested exploration creates the area -and records the recommendations; it does not authorize implementation tasks. -Gecode 7 may impose new comparison requirements on optimization spaces; a -legacy arbitration fallback is not required. Each space defines when it has -enough information to compare. How to report insufficient information, float -behavior, and partial-order handling remain proposals pending discussion. +The design decisions below are approved for task drafting. Import of the exact +task bundle and implementation remain separate steps. Gecode 7 may impose new +comparison requirements on optimization spaces; no legacy arbitration fallback +is required. Each space defines when it has enough information to compare. ## Boundaries @@ -35,9 +33,8 @@ outcomes must reach the caller. Exclude implementation of diverse-solution search, Pareto-front enumeration, a new search engine, objective extraction or serialization, a comparator registry, -heterogeneous-model conversion, changing -the C++ language requirement, and a performance benchmark campaign. No slices -or tasks are needed for this exploration. +heterogeneous-model conversion, changing the C++ language requirement, and a +performance benchmark campaign. Keep this area unsliced. Include limited advance planning for diverse-solution and Pareto search so this interface leaves those uses possible. Their implementation and concrete APIs @@ -53,7 +50,7 @@ an additional requirement of the current single-incumbent optimization engines. **Incomparable** is a known result under a partial ordering; it does not mean that an objective is unassigned or comparison has not been implemented. -Recommend one virtual member of `Space`, with receiver-relative results: +Add one virtual member of `Space`, with receiver-relative results: ```cpp enum SpaceComparison { @@ -66,14 +63,14 @@ enum SpaceComparison { virtual SpaceComparison compare(const Space& other) const; ``` -Recommend keeping the result restricted to ordering outcomes. Missing comparison -support and insufficient information to compare are errors, not ordering results. The -readiness decision is still open; this is the recommended interface if the -precondition is adopted. The exact enum spelling is provisional. This uses the -current C++17 baseline and needs neither an additional capability virtual nor a -new objective base class. A non-pure default that reports unsupported use can -leave satisfaction-only spaces unaffected; optimization spaces must provide -the comparison, usually through a convenience base. +Keep the result restricted to ordering outcomes. Missing comparison support and +insufficient information to compare are errors, not ordering results. Use the +names above unless an existing repository conflict requires a local naming +adjustment. This uses the current C++17 baseline and needs neither an additional +capability virtual nor a new objective base class. The non-pure default reports +unsupported use through a Gecode exception; satisfaction-only spaces need not +override it. Optimization spaces provide comparison, usually through a +convenience base. The method reads objective data only. It must not call `status()`, post a constraint, clone either argument, alter domains or branchers, or change shared @@ -102,13 +99,11 @@ being comparable does not by itself make it a valid incumbent: external incumbents must also satisfy the engine's solution contract. Likewise, `constrain()` retains its own model-defined requirements on its argument. -Recommend treating insufficient information for the requested comparison as +Treat insufficient information for the requested comparison as a precondition violation, reported using the appropriate model error convention. For scalar integer helpers, the existing unassigned-value exception fits. -The alternative is a distinct `SC_UNDETERMINED` result, allowing a caller to -recover when the model cannot establish the relation. The current arbitration -sites would still need an error policy for this result. The reporting choice -is open; model ownership of comparability is settled. +Do not add an undetermined or unavailable result. This reporting choice and +model ownership of comparability are settled. All participating assets must use the same objective direction, component interpretation, and pruning policy. Built-in comparisons should diagnose @@ -120,8 +115,10 @@ shared objective interface, but automatic conversion is outside this PR. For exact optimization, define `better(a,b)` by `a.compare(b) == SC_BETTER`. It must be irreflexive and transitive, reverse to `SC_WORSE`, and agree with `constrain(b)` on objective-complete feasible solutions. Equivalence must be -transitive and preserve comparisons with every third solution. For the existing -single-incumbent engines, every pair must be better, worse, or equivalent. +transitive and preserve comparisons with every third solution. For the initial +exact single-incumbent engine policy, every pair encountered must be better, +worse, or equivalent. This does not restrict the Space interface or a future +best-effort engine policy. There is also a search requirement: if `a` replaces `b`, the solutions admitted by the new improvement restriction must be a subset of those admitted by the @@ -160,6 +157,17 @@ finalizing the interface, check that direct pairwise calls can support these uses and that no global assignment or single-incumbent assumption has entered the Space contract. Do not implement either future engine here. +A likely future BAB/PBS best-effort policy is: an incoming solution incomparable +with the current incumbent becomes the new incumbent. Keep this possibility +explicit. Comparison still reports `SC_INCOMPARABLE`; acceptance and restrictions +are engine decisions. Such replacement need not yield a monotonically improving +sequence or nested cuts. Existing local cuts, recomputation, and asset exhaustion +can then prevent completeness or even maximality guarantees. A later experiment +should test precisely that replacement rule and describe the resulting search +guarantees. Initial rejection in this PR is a policy choice, not a claim that +best-effort support is impossible. No public policy option, experiment framework, +or best-effort implementation is required now. + ## Search behavior Use the same orientation everywhere: compare the incoming solution against the @@ -182,11 +190,13 @@ solutions when accepting an external incumbent: no queued result returned after the update may violate the new incumbent contract. Filtering those pending solutions is in scope if required; changing queue policy otherwise is not. -`SC_INCOMPARABLE` is expressible for direct model use and a future Pareto engine, -but existing best-solution engines must report unsupported use. Do not silently -treat it as a tie, choose an arbitrary winner, or claim Pareto completeness. -Partial orders can support finding a single maximal solution in a deliberately -designed dominance-chain search, but that is not the current PBS asset protocol. +Handle `SC_INCOMPARABLE` explicitly at the search acceptance boundary. The initial +policy reports unsupported use. Do not fold it into equality, worse, or a +comparison-precondition error. Keep the treatment consistent across BAB, PBS, +RBS external updates, and pending results, using a small internal helper if +that avoids repetition. Do not build a policy framework. Documentation and +tests must describe rejection as the current engine policy, leaving the +best-effort replacement rule above possible without changing SpaceComparison. Missing or unsupported comparison must produce a caller-visible diagnostic, not normal exhaustion, a hang, or termination of a detached worker thread. @@ -221,11 +231,11 @@ evidence of a separate diverse-solution use, not a reason for an unavailable comparison result or a compatibility path. Its all-previous-solutions search semantics are outside this PR. -### Floating-point recommendation +### Floating-point policy -Separate objective ranking from the improvement step. For the existing float -bases, propose ranking the upper cost bound for minimization and the lower cost -bound for maximization, matching the incumbent endpoint used in their cuts. +Separate objective ranking from the improvement step. For the MiniModel float +bases, rank the upper cost bound for minimization and the lower cost bound for +maximization, matching the incumbent endpoint used in their cuts. Keep the step in `constrain()`, with a common step policy across assets. Compare assigned float variables using Gecode's meaning of assigned, which can include an interval between adjacent representable values. @@ -237,42 +247,32 @@ ranking keys; being within a step is not equivalence. The latter would be non-transitive. Approximate pruning needs the subset property above, but need not admit every solution ranked better. -Before implementing this choice, reconcile it explicitly with FlatZinc's -interval-valued float threshold, strict relations, rounding, and exact threshold -boundaries. In particular, equal upper bounds do not necessarily give equivalent -FlatZinc minimization cuts when the lower bounds differ. Its comparator needs a -cut-compatible key; do not copy the MiniModel endpoint rule without resolving -this difference. Preserve the posted improvement constraints unless the user -approves a change. If the step must instead hold between reported solutions, this -ordering alone is insufficient: a separate admission policy or a bounded legacy -float path must be chosen. Do not quietly redefine comparison as a tolerance -test. Float behavior is a material open decision, not an implementation detail. - -## Open questions - -1. When a space lacks enough information for a requested comparison, should it - report a precondition violation or return a separate undetermined result? - Recommend the precondition for the present search consumers. In either case, - the individual space decides what information suffices; assignment is not - a generic requirement. -2. Adopt float bound ranking with step-based pruning, allowing improvements - smaller than the step among concurrent results, or preserve the reporting - step through an additional policy? Recommend the former, subject to focused - interval-boundary checks. -3. Confirm the proposed partial-order result with rejection in existing engines. - A Pareto-front engine is separate work; if it is required here, reshape the - area before splitting implementation tasks. - -The Gecode 7 requirement, model-defined comparability, and advance planning -without implementation of diverse-solution search are settled. -The choices listed here remain recommendations, not settled user decisions. +Preserve FlatZinc's existing interval-valued float constraints. Their strict +disequality rejects overlap with the threshold interval, so use the lower cost +bound for FlatZinc minimization and the upper cost bound for maximization. +These keys differ from MiniModel's scalar-threshold keys by design. Validate +that common-step rounding preserves nesting and that equal keys yield equivalent +cuts on comparable solutions, including adjacent-endpoint intervals. Do not +silently align the two families by changing their constraints. If focused checks +disprove these properties, report the specific counterexample before changing +the agreed ranking/pruning behavior. No separate reporting-step policy or legacy +float arbitration path is part of the approved work. + +## Remaining implementation checks + +No product decision remains before task drafting. Confirm float cut compatibility +with focused boundary checks and preserve safe error delivery through nested +engines. These are implementation proof obligations, not authority to change +the approved behavior. Best-effort incomparable replacement is a future +experiment, not an unresolved choice for the initial engine policy. ## Testing For this exploration: existing structural checks only. No production code or tests change, and no build is needed to validate a planning artifact. -For implementation: focused coverage in the existing search test suite, then +Agreed implementation testing level: focused coverage in the existing search +suite, then the existing search regressions. The observable risks justify these cases: - Direct scalar, lexicographic, and custom-objective comparisons: direction, @@ -287,7 +287,7 @@ the existing search regressions. The observable risks justify these cases: - Missing comparison and an incomparable result must fail cleanly in a direct and a nested parallel path. A focused termination check is necessary because the current thread runner has no exception forwarding. -- If float ranking is accepted: zero/nonzero step, improvement smaller than the +- For float ranking: zero/nonzero step, improvement smaller than the step, the strict threshold, and tight non-singleton intervals in MiniModel and FlatZinc. Check the agreed ranking/pruning distinction explicitly. - Compile the ordering and Gist display overloads together in a representative @@ -300,8 +300,9 @@ layer, timing assertions, or benchmarks merely to justify cheap comparison. ## Validation and completion -The shaping deliverable is this brief, its indexed research note, and a passing -`zdev check comparison --format json`. No tasks are imported during exploration. +The planning deliverable is this brief, its indexed research note, and an +independently challenged task bundle. Run `zdev check comparison --format json` +and present the exact bundle for approval before importing it. Implementation is complete when every incumbent arbitration site above uses the agreed contract, standard objectives and migration behavior are covered, diff --git a/.zdev/comparison/tasks/001-add-space-comparison-and-integer-objective-implementations.md b/.zdev/comparison/tasks/001-add-space-comparison-and-integer-objective-implementations.md new file mode 100644 index 0000000000..3acfa05d3c --- /dev/null +++ b/.zdev/comparison/tasks/001-add-space-comparison-and-integer-objective-implementations.md @@ -0,0 +1,34 @@ ++++ +schema_version = 1 +id = "comparison-001" +key = "integer-comparison" +area = "comparison" +status = "open" +complexity = "standard" +afk = true +priority = "high" +blocked_by = [] ++++ +# Add Space comparison and integer objective implementations + +## Outcome + +Models can compare spaces without mutation, with integer convenience classes and FlatZinc providing the standard implementations. + +## Context + +The approved [brief](../brief.md) defines four ordering outcomes and model-defined comparability. Space currently only has constrain(); MiniModel supplies scalar and lexicographic integer objectives, while Driver::ScriptBase and FlatZinc already have Gist display compare overloads. Start in kernel/core.hpp and core.cpp, minimodel.hh and optimize.cpp, driver.hh, and flatzinc.hh/flatzinc.cpp; use test/search.cpp for focused coverage. + +## Boundaries + +- Add the API, integer implementations, their documentation, and overload compatibility. Leave search arbitration and float implementations to dependent tasks; do not add generic assignment checks or a capability API. + +## Done when + +- [ ] Space::compare(const Space&) const returns the four SpaceComparison outcomes; its default reports unsupported use through a Gecode exception. No unavailable result is added. +- [ ] Scalar min/max and lexicographic min/max, plus FlatZinc integer optimization, compare in the agreed direction without mutation. Lexicographic comparison accepts a decisive assigned prefix without requiring later components; insufficient information for the requested comparison raises the appropriate error. +- [ ] Focused tests cover direction, equivalence, a decisive prefix with an unassigned suffix, insufficient scalar data, and incompatible objective families/dimensions. The ordering and Gist display overloads remain callable, including through a custom script. + +## Validation + +- Build gecode-test and run the new comparison tests plus relevant existing integer/FlatZinc tests. Compile a representative custom script using both compare overloads; no new test harness. diff --git a/.zdev/comparison/tasks/002-compare-float-quality-independently-of-the-improvement-step.md b/.zdev/comparison/tasks/002-compare-float-quality-independently-of-the-improvement-step.md new file mode 100644 index 0000000000..3ae0fc3837 --- /dev/null +++ b/.zdev/comparison/tasks/002-compare-float-quality-independently-of-the-improvement-step.md @@ -0,0 +1,34 @@ ++++ +schema_version = 1 +id = "comparison-002" +key = "float-comparison" +area = "comparison" +status = "open" +complexity = "advanced" +afk = true +priority = "normal" +blocked_by = ["comparison-001"] ++++ +# Compare float quality independently of the improvement step + +## Outcome + +MiniModel and FlatZinc float optimization expose a documented comparison consistent with their existing pruning boundaries, without enforcing the step between compared solutions. + +## Context + +The [brief](../brief.md) specifies MiniModel upper-bound minimization/lower-bound maximization and FlatZinc lower-bound minimization/upper-bound maximization. The difference follows from scalar versus interval-valued strict thresholds; see [float analysis](../background/comparison-and-search.md). Start in minimodel/optimize.cpp, flatzinc/flatzinc.cpp, float/rel.cpp, and float/rel/nq.hpp. + +## Boundaries + +- Implement comparison and focused tests/documentation. Preserve constrain(), strictness, interval arithmetic, and step semantics; do not add a reporting-step filter or legacy comparison fallback. + +## Done when + +- [ ] Both MiniModel float bases and FlatZinc float optimization implement the agreed family-specific ordering and document what makes their operands comparable, including tight non-singleton intervals. +- [ ] Tests demonstrate ranking of sub-step improvements, equal keys, both directions, zero/nonzero step, strict threshold boundaries, and adjacent-endpoint intervals. Equal keys produce equivalent cuts and improved keys produce nested cuts on representative comparable solutions. +- [ ] Comparison leaves both inputs unchanged. If a boundary check disproves cut compatibility, report its concrete counterexample rather than silently changing the approved semantics. + +## Validation + +- Run focused float comparison/cut tests and relevant existing float and FlatZinc tests. Derive the cut-nesting argument from the current posting code; no timing tests or broad floating-point matrix. diff --git a/.zdev/comparison/tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md b/.zdev/comparison/tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md new file mode 100644 index 0000000000..defd9d8a62 --- /dev/null +++ b/.zdev/comparison/tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md @@ -0,0 +1,34 @@ ++++ +schema_version = 1 +id = "comparison-003" +key = "sequential-incumbents" +area = "comparison" +status = "open" +complexity = "standard" +afk = true +priority = "normal" +blocked_by = ["comparison-001"] ++++ +# Use comparison for sequential BAB and restart incumbent updates + +## Outcome + +Sequential BAB and RBS select external incumbents by comparison while preserving ordinary pruning and restart behavior. + +## Context + +Seq::BAB::constrain in search/seq/bab.hpp and RBS::constrain in search/seq/rbs.cpp currently mutate the retained solution and call status(). test/search.cpp has a balanced custom objective and existing restart/portfolio tests. CarSequencing is an example with a custom scalar constrain(). Follow the [brief](../brief.md); the [source map](../background/comparison-and-search.md) distinguishes arbitration from pruning. + +## Boundaries + +- Change external-incumbent arbitration, the directly affected model overrides, focused tests, and migration documentation. Keep ordinary sequential next(), path pruning, and restart hooks; no best-effort mode or policy framework. + +## Done when + +- [ ] Incoming incumbents replace retained ones only when better; equal/worse leave them intact. A valid incomparable outcome is explicitly rejected by the initial engine policy, separately from model comparison errors. +- [ ] Actual working spaces still receive constrain(), and recomputation marks and restart state remain correct. A small internal acceptance helper may keep later BAB/PBS policy handling consistent. +- [ ] Relevant custom optimization test models and examples implement their ordering rather than receiving a generic default ranking. Focused tests cover late better/equal/worse updates, the balanced objective, and direct comparison-policy errors. + +## Validation + +- Run focused external-update tests and existing sequential BAB/RBS search tests, including stop/resume. Smoke-check the migrated optimization example. No new tests of unrelated pruning internals. diff --git a/.zdev/comparison/tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md b/.zdev/comparison/tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md new file mode 100644 index 0000000000..0a9fc47445 --- /dev/null +++ b/.zdev/comparison/tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md @@ -0,0 +1,35 @@ ++++ +schema_version = 1 +id = "comparison-004" +key = "parallel-bab" +area = "comparison" +status = "open" +complexity = "advanced" +afk = true +priority = "normal" +blocked_by = ["comparison-002", "comparison-003"] ++++ +# Use comparison for parallel BAB and safely report comparison failures + +## Outcome + +Parallel BAB arbitrates solutions without mutating them, preserves pending-result correctness, and returns comparison failures to the controlling caller. + +## Context + +Par::BAB::solution and constrain in search/par/bab.hpp currently probe with constrain/status under m_search. search/par/engine.hpp returns queued results before resuming workers; support/thread/thread.cpp has no exception forwarding. The preceding tasks supply objective implementations and the initial acceptance policy. Follow the ownership and nesting requirements in the [brief](../brief.md). + +## Boundaries + +- Change BAB arbitration, its required queue/error handling, and focused tests/documentation. Preserve worker locks, ownership, pruning, and recomputation. Limit error transport to comparison and acceptance failures; no general thread-runtime rewrite. + +## Done when + +- [ ] Candidates and external incumbents use incoming-versus-retained comparison. Only accepted incumbents are cloned/broadcast; completed solutions are never constrained or propagated to rank them. +- [ ] After an external incumbent update, queued results that violate the new incumbent contract are not returned. Accepted queue ordering and ownership remain valid. +- [ ] Missing comparison, model comparison errors, and initial-policy rejection of incomparability wake/block workers safely and reach the controlling call instead of terminating a detached thread, hanging, or reporting normal exhaustion. The engine can be safely destroyed after failure. +- [ ] Focused tests check monotone results and the known optimum for a small custom objective, late external bounds with pending results, sub-step float ranking, and safe failure delivery. + +## Validation + +- Run focused parallel BAB tests and existing BAB stop/resume/recomputation coverage. Use bounded failure tests that detect hangs; do not require deterministic parallel solution counts or schedules. diff --git a/.zdev/comparison/tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md b/.zdev/comparison/tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md new file mode 100644 index 0000000000..a750aee650 --- /dev/null +++ b/.zdev/comparison/tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md @@ -0,0 +1,35 @@ ++++ +schema_version = 1 +id = "comparison-005" +key = "portfolio-comparison" +area = "comparison" +status = "open" +complexity = "advanced" +afk = true +priority = "normal" +blocked_by = ["comparison-004"] ++++ +# Integrate comparison through portfolios and complete the migration + +## Outcome + +PBS, including nested RBS/BAB assets, consistently applies the initial comparison policy and safely delivers failures, with the complete change documented and regression-checked. + +## Context + +Par::PBS::CollectBest::add/constrain mutate the retained solution; reporter controls subsequent broadcasts. PBS::report and Slave::run coordinate n_busy, tostop, and the completion handshake. Nested asset next() calls can now raise comparison failures. Use search/par/pbs.hpp and hh, search/seq/pbs.hpp, and the existing SEBPBS tests in test/search.cpp; follow the [brief](../brief.md). + +## Boundaries + +- Integrate portfolio arbitration, necessary nested failure transport, focused tests, and user-facing migration notes. Preserve satisfaction collection, asset scheduling, and completion handshakes. Do not implement best-effort replacement, Pareto archives, or diversity search. + +## Done when + +- [ ] CollectBest accepts only better candidates initially, preserves reporter on rejection, and never mutates a solution merely to rank it. External updates and pending results use the same policy as BAB/RBS. +- [ ] Comparison and policy failures from the collector or nested asset calls stop/wake and complete safely, reaching the outer caller; deletion after failure does not hang or race worker completion. +- [ ] Focused direct and mixed PBS/RBS/BAB tests establish expected objective improvement/optimum, sub-step float ranking, equivalent-candidate handling, and safe incomparable/error reporting. Existing satisfaction paths remain functional. +- [ ] API/search documentation and the Gecode 7 changelog explain migration, model-defined comparability, float ranking, and current rejection policy. They explicitly preserve the possible future best-effort rule 'incomparable incoming solution becomes current best' without claiming completeness or requiring a changed comparison interface. + +## Validation + +- Run focused portfolio and nested-failure tests, then the existing Search:: suite and relevant FlatZinc checks. Audit remaining constrain/status calls to confirm they are pruning rather than arbitration. Run the existing no-thread build/check path for touched conditional code; no new configuration matrix. From 25eb5174f801c3bf720ef3ecdad799452738736e Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Thu, 10 Sep 2026 10:25:44 +0200 Subject: [PATCH 05/15] comparison: add space and integer objective comparison Zdev-Change-Id: Z87d7de4220478ed2e6d58d55f4648cde323acfbbb28ecb589c39215a802aeb03 --- .zdev/comparison/TASKS.md | 12 +- ...n-and-integer-objective-implementations.md | 18 ++- gecode/driver.hh | 1 + gecode/flatzinc.hh | 2 + gecode/flatzinc/flatzinc.cpp | 15 +++ gecode/kernel/core.cpp | 5 + gecode/kernel/core.hpp | 20 ++++ gecode/kernel/exception.cpp | 3 + gecode/kernel/exception.hpp | 7 ++ gecode/minimodel.hh | 12 ++ gecode/minimodel/optimize.cpp | 58 +++++++++- test/flatzinc.cpp | 40 +++++++ test/search.cpp | 107 ++++++++++++++++++ 13 files changed, 289 insertions(+), 11 deletions(-) diff --git a/.zdev/comparison/TASKS.md b/.zdev/comparison/TASKS.md index b2e92c565a..b3c4435c82 100644 --- a/.zdev/comparison/TASKS.md +++ b/.zdev/comparison/TASKS.md @@ -3,14 +3,14 @@ # Tasks: comparison - Total: 5 -- Ready: 1 -- Blocked: 4 -- Done: 0 +- Ready: 2 +- Blocked: 2 +- Done: 1 | ID | Task | State | Blocked by | | --- | --- | --- | --- | -| [comparison-001](tasks/001-add-space-comparison-and-integer-objective-implementations.md) | Add Space comparison and integer objective implementations | ready | — | -| [comparison-002](tasks/002-compare-float-quality-independently-of-the-improvement-step.md) | Compare float quality independently of the improvement step | blocked | comparison-001 | -| [comparison-003](tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md) | Use comparison for sequential BAB and restart incumbent updates | blocked | comparison-001 | +| [comparison-001](tasks/001-add-space-comparison-and-integer-objective-implementations.md) | Add Space comparison and integer objective implementations | done | — | +| [comparison-002](tasks/002-compare-float-quality-independently-of-the-improvement-step.md) | Compare float quality independently of the improvement step | ready | comparison-001 | +| [comparison-003](tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md) | Use comparison for sequential BAB and restart incumbent updates | ready | comparison-001 | | [comparison-004](tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md) | Use comparison for parallel BAB and safely report comparison failures | blocked | comparison-002, comparison-003 | | [comparison-005](tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md) | Integrate comparison through portfolios and complete the migration | blocked | comparison-004 | diff --git a/.zdev/comparison/tasks/001-add-space-comparison-and-integer-objective-implementations.md b/.zdev/comparison/tasks/001-add-space-comparison-and-integer-objective-implementations.md index 3acfa05d3c..82598ee382 100644 --- a/.zdev/comparison/tasks/001-add-space-comparison-and-integer-objective-implementations.md +++ b/.zdev/comparison/tasks/001-add-space-comparison-and-integer-objective-implementations.md @@ -3,7 +3,7 @@ schema_version = 1 id = "comparison-001" key = "integer-comparison" area = "comparison" -status = "open" +status = "done" complexity = "standard" afk = true priority = "high" @@ -25,10 +25,20 @@ The approved [brief](../brief.md) defines four ordering outcomes and model-defin ## Done when -- [ ] Space::compare(const Space&) const returns the four SpaceComparison outcomes; its default reports unsupported use through a Gecode exception. No unavailable result is added. -- [ ] Scalar min/max and lexicographic min/max, plus FlatZinc integer optimization, compare in the agreed direction without mutation. Lexicographic comparison accepts a decisive assigned prefix without requiring later components; insufficient information for the requested comparison raises the appropriate error. -- [ ] Focused tests cover direction, equivalence, a decisive prefix with an unassigned suffix, insufficient scalar data, and incompatible objective families/dimensions. The ordering and Gist display overloads remain callable, including through a custom script. +- [x] Space::compare(const Space&) const returns the four SpaceComparison outcomes; its default reports unsupported use through a Gecode exception. No unavailable result is added. +- [x] Scalar min/max and lexicographic min/max, plus FlatZinc integer optimization, compare in the agreed direction without mutation. Lexicographic comparison accepts a decisive assigned prefix without requiring later components; insufficient information for the requested comparison raises the appropriate error. +- [x] Focused tests cover direction, equivalence, a decisive prefix with an unassigned suffix, insufficient scalar data, and incompatible objective families/dimensions. The ordering and Gist display overloads remain callable, including through a custom script. ## Validation - Build gecode-test and run the new comparison tests plus relevant existing integer/FlatZinc tests. Compile a representative custom script using both compare overloads; no new test harness. + +## Result + +Added and independently verified non-mutating Space comparison with integer objective implementations and overload compatibility. + +Validation: + +- Clean external CMake build of gecode-test passed. +- Search::Comparison and FlatZinc::IntegerObjectiveComparison passed. +- Complete existing FlatZinc suite and representative custom-script overload compilation passed. diff --git a/gecode/driver.hh b/gecode/driver.hh index 0dd99b4caf..6c4662f859 100755 --- a/gecode/driver.hh +++ b/gecode/driver.hh @@ -881,6 +881,7 @@ namespace Gecode { namespace Driver { template class ScriptBase : public BaseSpace { public: + using BaseSpace::compare; /// Constructor ScriptBase(const Options& opt); /// Constructor used for cloning diff --git a/gecode/flatzinc.hh b/gecode/flatzinc.hh index d4b29dc5da..f0f2f224b0 100755 --- a/gecode/flatzinc.hh +++ b/gecode/flatzinc.hh @@ -626,6 +626,8 @@ namespace Gecode { namespace FlatZinc { /// Compare this space with space \a s and print the differences on /// \a out void compare(const Space& s, std::ostream& out) const; + /// Compare integer optimization objectives + virtual SpaceComparison compare(const Space& s) const; /// Compare this space with space \a s and print the differences on /// \a out using \a p void compare(const FlatZincSpace& s, std::ostream& out, diff --git a/gecode/flatzinc/flatzinc.cpp b/gecode/flatzinc/flatzinc.cpp index 0d525dd1f0..25fe49f3d6 100644 --- a/gecode/flatzinc/flatzinc.cpp +++ b/gecode/flatzinc/flatzinc.cpp @@ -2078,6 +2078,21 @@ namespace Gecode { namespace FlatZinc { } } + SpaceComparison + FlatZincSpace::compare(const Space& s) const { + const FlatZincSpace* other = dynamic_cast(&s); + if (other == nullptr) + throw DynamicCastFailed("FlatZincSpace::compare"); + if (!_optVarIsInt || !other->_optVarIsInt || + (_method != other->_method) || + ((_method != MIN) && (_method != MAX))) + throw DynamicCastFailed("FlatZincSpace::compare"); + int a=iv[_optVar].val(), b=other->iv[other->_optVar].val(); + if (a == b) + return SC_EQUIVALENT; + return ((a < b) == (_method == MIN)) ? SC_BETTER : SC_WORSE; + } + bool FlatZincSpace::slave(const MetaInfo& mi) { if (mi.type() == MetaInfo::RESTART) { diff --git a/gecode/kernel/core.cpp b/gecode/kernel/core.cpp index 54ab989695..21b8584bfb 100644 --- a/gecode/kernel/core.cpp +++ b/gecode/kernel/core.cpp @@ -896,6 +896,11 @@ namespace Gecode { Space::constrain(const Space&) { } + SpaceComparison + Space::compare(const Space&) const { + throw SpaceNoComparison("Space::compare"); + } + bool Space::master(const MetaInfo& mi) { switch (mi.type()) { diff --git a/gecode/kernel/core.hpp b/gecode/kernel/core.hpp index 08423430ee..e1eba7db59 100755 --- a/gecode/kernel/core.hpp +++ b/gecode/kernel/core.hpp @@ -53,6 +53,14 @@ namespace Gecode { + /// Result of comparing two spaces according to their objectives + enum SpaceComparison { + SC_BETTER, ///< The receiver has a better objective + SC_EQUIVALENT, ///< Both objectives are equivalent + SC_WORSE, ///< The receiver has a worse objective + SC_INCOMPARABLE ///< Neither objective dominates the other + }; + class Space; /** @@ -2148,6 +2156,18 @@ namespace Gecode { * \ingroup TaskModelScript */ GECODE_KERNEL_EXPORT virtual void constrain(const Space& best); + /** + * \brief Compare this space's objective with \a other + * + * The comparison is read-only and receiver-relative. Implementations may + * require objective information sufficient for the relation they report. + * The default implementation throws SpaceNoComparison. + * + * Optimization models overriding constrain with a different objective + * must also override compare. + */ + GECODE_KERNEL_EXPORT virtual SpaceComparison + compare(const Space& other) const; /** * \brief Master configuration function for meta search engines * diff --git a/gecode/kernel/exception.cpp b/gecode/kernel/exception.cpp index e16c6cd61a..994d76f9c4 100644 --- a/gecode/kernel/exception.cpp +++ b/gecode/kernel/exception.cpp @@ -47,6 +47,9 @@ namespace Gecode { SpaceNoBrancher::SpaceNoBrancher(const char* l) : Exception(l,"Attempt to commit with no brancher") {} + SpaceNoComparison::SpaceNoComparison(const char* l) + : Exception(l,"Space does not support comparison") {} + SpaceIllegalAlternative::SpaceIllegalAlternative(const char* l) : Exception(l,"Attempt to commit with illegal alternative") {} diff --git a/gecode/kernel/exception.hpp b/gecode/kernel/exception.hpp index 792f4975e6..d118b10a9f 100644 --- a/gecode/kernel/exception.hpp +++ b/gecode/kernel/exception.hpp @@ -68,6 +68,13 @@ namespace Gecode { SpaceNoBrancher(const char* l); }; + /// %Exception: Comparison not implemented by space + class GECODE_KERNEL_EXPORT SpaceNoComparison : public Exception { + public: + /// Initialize with location \a l + SpaceNoComparison(const char* l); + }; + /// %Exception: Commit with illegal alternative class GECODE_KERNEL_EXPORT SpaceIllegalAlternative : public Exception { public: diff --git a/gecode/minimodel.hh b/gecode/minimodel.hh index 59c7869e61..66e6eeb71d 100755 --- a/gecode/minimodel.hh +++ b/gecode/minimodel.hh @@ -2442,6 +2442,9 @@ namespace Gecode { /// Member function constraining according to decreasing cost GECODE_MINIMODEL_EXPORT virtual void constrain(const Space& best); + /// Compare integer costs (both costs must be assigned) + GECODE_MINIMODEL_EXPORT + virtual SpaceComparison compare(const Space& other) const; /// Return variable with current cost virtual IntVar cost(void) const = 0; }; @@ -2459,6 +2462,9 @@ namespace Gecode { /// Member function constraining according to increasing cost GECODE_MINIMODEL_EXPORT virtual void constrain(const Space& best); + /// Compare integer costs (both costs must be assigned) + GECODE_MINIMODEL_EXPORT + virtual SpaceComparison compare(const Space& other) const; /// Return variable with current cost virtual IntVar cost(void) const = 0; }; @@ -2476,6 +2482,9 @@ namespace Gecode { /// Member function constraining according to decreasing costs GECODE_MINIMODEL_EXPORT virtual void constrain(const Space& best); + /// Compare integer costs lexicographically + GECODE_MINIMODEL_EXPORT + virtual SpaceComparison compare(const Space& other) const; /// Return variables with current costs virtual IntVarArgs cost(void) const = 0; }; @@ -2493,6 +2502,9 @@ namespace Gecode { /// Member function constraining according to increasing costs GECODE_MINIMODEL_EXPORT virtual void constrain(const Space& best); + /// Compare integer costs lexicographically + GECODE_MINIMODEL_EXPORT + virtual SpaceComparison compare(const Space& other) const; /// Return variables with current costs virtual IntVarArgs cost(void) const = 0; }; diff --git a/gecode/minimodel/optimize.cpp b/gecode/minimodel/optimize.cpp index 6442ada179..320e37da4c 100755 --- a/gecode/minimodel/optimize.cpp +++ b/gecode/minimodel/optimize.cpp @@ -35,6 +35,13 @@ namespace Gecode { + static SpaceComparison + compare_int(int x, int y, bool minimize) { + if (x == y) + return SC_EQUIVALENT; + return ((x < y) == minimize) ? SC_BETTER : SC_WORSE; + } + void IntMinimizeSpace::constrain(const Space& _best) { const IntMinimizeSpace* best = @@ -44,6 +51,14 @@ namespace Gecode { rel(*this, cost(), IRT_LE, best->cost().val()); } + SpaceComparison + IntMinimizeSpace::compare(const Space& other) const { + const IntMinimizeSpace* s = dynamic_cast(&other); + if (s == nullptr) + throw DynamicCastFailed("IntMinimizeSpace::compare"); + return compare_int(cost().val(),s->cost().val(),true); + } + void IntMaximizeSpace::constrain(const Space& _best) { @@ -54,6 +69,14 @@ namespace Gecode { rel(*this, cost(), IRT_GR, best->cost().val()); } + SpaceComparison + IntMaximizeSpace::compare(const Space& other) const { + const IntMaximizeSpace* s = dynamic_cast(&other); + if (s == nullptr) + throw DynamicCastFailed("IntMaximizeSpace::compare"); + return compare_int(cost().val(),s->cost().val(),false); + } + void IntLexMinimizeSpace::constrain(const Space& _best) { @@ -68,6 +91,23 @@ namespace Gecode { rel(*this, cx, IRT_LE, bn); } + SpaceComparison + IntLexMinimizeSpace::compare(const Space& other) const { + const IntLexMinimizeSpace* s = + dynamic_cast(&other); + if (s == nullptr) + throw DynamicCastFailed("IntLexMinimizeSpace::compare"); + IntVarArgs a(cost()), b(s->cost()); + if (a.size() != b.size()) + throw MiniModel::ArgumentSizeMismatch("IntLexMinimizeSpace::compare"); + for (int i=0; i(&other); + if (s == nullptr) + throw DynamicCastFailed("IntLexMaximizeSpace::compare"); + IntVarArgs a(cost()), b(s->cost()); + if (a.size() != b.size()) + throw MiniModel::ArgumentSizeMismatch("IntLexMaximizeSpace::compare"); + for (int i=0; i + model(const char* source) { + Gecode::FlatZinc::Printer p; + std::stringstream ss(source); + return std::unique_ptr + (Gecode::FlatZinc::parse(ss,p,olog)); + } + public: + IntegerObjectiveComparison(void) + : Base("FlatZinc::IntegerObjectiveComparison") {} + + virtual bool run(void) { + using namespace Gecode; + std::unique_ptr one = + model("var 1..1: x; solve minimize x;\n"); + std::unique_ptr two = + model("var 2..2: x; solve minimize x;\n"); + std::unique_ptr high = + model("var 2..2: x; solve maximize x;\n"); + std::unique_ptr low = + model("var 1..1: x; solve maximize x;\n"); + if (!one || !two || !high || !low || + (one->compare(*two) != SC_BETTER) || + (two->compare(*one) != SC_WORSE) || + (one->compare(*one) != SC_EQUIVALENT) || + (high->compare(*low) != SC_BETTER)) + return false; + try { + (void) one->compare(*high); + return false; + } catch (const DynamicCastFailed&) {} + return true; + } + }; + + IntegerObjectiveComparison integer_objective_comparison; + /// Verify that statistics do not override an explicit Gist mode. class GistStatisticsMode : public Base { private: diff --git a/test/search.cpp b/test/search.cpp index b1f0354815..ce910c9b2f 100644 --- a/test/search.cpp +++ b/test/search.cpp @@ -1142,6 +1142,112 @@ namespace Test { WorkerControlPBSPause worker_control_pbs_pause; + /// Scalar integer objective used for comparison tests + class MinObjective : public IntMinimizeSpace { + public: + IntVar x; + MinObjective(int l, int u) : x(*this,l,u) {} + MinObjective(MinObjective& s) : IntMinimizeSpace(s) { + x.update(*this,s.x); + } + virtual Space* copy(void) { return new MinObjective(*this); } + virtual IntVar cost(void) const { return x; } + }; + + /// Scalar maximization objective used to check objective families + class MaxObjective : public IntMaximizeSpace { + public: + IntVar x; + MaxObjective(int v) : x(*this,v,v) {} + MaxObjective(MaxObjective& s) : IntMaximizeSpace(s) { + x.update(*this,s.x); + } + virtual Space* copy(void) { return new MaxObjective(*this); } + virtual IntVar cost(void) const { return x; } + }; + + /// Lexicographic integer objective used for comparison tests + class LexObjective : public IntLexMinimizeSpace { + public: + IntVarArray x; + LexObjective(int a, int l, int u, int n=2) : x(*this,n,l,u) { + rel(*this,x[0],IRT_EQ,a); + (void) status(); + } + LexObjective(LexObjective& s) : IntLexMinimizeSpace(s) { + x.update(*this,s.x); + } + virtual Space* copy(void) { return new LexObjective(*this); } + virtual IntVarArgs cost(void) const { return x; } + }; + + /// Lexicographic maximization objective used for comparison tests + class LexMaxObjective : public IntLexMaximizeSpace { + public: + IntVarArray x; + LexMaxObjective(int a) : x(*this,2,0,2) { + rel(*this,x[0],IRT_EQ,a); + (void) status(); + } + LexMaxObjective(LexMaxObjective& s) : IntLexMaximizeSpace(s) { + x.update(*this,s.x); + } + virtual Space* copy(void) { return new LexMaxObjective(*this); } + virtual IntVarArgs cost(void) const { return x; } + }; + + /// Space without objective comparison support + class PlainSpace : public Space { + public: + PlainSpace(void) {} + PlainSpace(PlainSpace& s) : Space(s) {} + virtual Space* copy(void) { return new PlainSpace(*this); } + }; + + /// Test objective comparison independently of search arbitration + class Comparison : public Base { + public: + Comparison(void) : Base("Search::Comparison") {} + virtual bool run(void) { + MinObjective one(1,1), two(2,2), one_again(1,1), open(0,2); + if ((one.compare(two) != SC_BETTER) || + (two.compare(one) != SC_WORSE) || + (one.compare(one_again) != SC_EQUIVALENT)) + return false; + MaxObjective high(2), low(1); + if ((high.compare(low) != SC_BETTER) || + (low.compare(high) != SC_WORSE)) + return false; + LexObjective lp(1,0,2), lq(2,0,2); + if ((lp.compare(lq) != SC_BETTER) || + (lq.compare(lp) != SC_WORSE)) + return false; + LexMaxObjective lmp(2), lmq(1); + if ((lmp.compare(lmq) != SC_BETTER) || + (lmq.compare(lmp) != SC_WORSE)) + return false; + try { + (void) open.compare(one); + return false; + } catch (const Int::ValOfUnassignedVar&) {} + try { + (void) one.compare(high); + return false; + } catch (const DynamicCastFailed&) {} + LexObjective short_cost(1,0,2,1); + try { + (void) lp.compare(short_cost); + return false; + } catch (const MiniModel::ArgumentSizeMismatch&) {} + PlainSpace plain; + try { + (void) plain.compare(plain); + return false; + } catch (const SpaceNoComparison&) {} + return true; + } + }; + /// %Test for depth-first search template class DFS : public Test { @@ -1496,6 +1602,7 @@ namespace Test { public: /// Perform creation and registration Create(void) { + (void) new Comparison; // Depth-first search for (unsigned int t = 1; t<=4; t++) for (unsigned int c_d = 1; c_d<10; c_d++) From d76c6fa6518708131f4c4260692031265e4de86c Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Thu, 10 Sep 2026 11:54:45 +0200 Subject: [PATCH 06/15] comparison: add float objective comparison Zdev-Change-Id: Z738bbca9dd306371480ab382cbd9492a52cd416d460e53b9512a5f83e57476d7 --- .zdev/comparison/TASKS.md | 6 +- ...y-independently-of-the-improvement-step.md | 18 ++- gecode/flatzinc.hh | 11 +- gecode/flatzinc/flatzinc.cpp | 28 ++++- gecode/minimodel.hh | 12 ++ gecode/minimodel/optimize.cpp | 24 ++++ test/flatzinc.cpp | 114 ++++++++++++++++++ test/search.cpp | 99 +++++++++++++++ 8 files changed, 298 insertions(+), 14 deletions(-) diff --git a/.zdev/comparison/TASKS.md b/.zdev/comparison/TASKS.md index b3c4435c82..c7240c10c5 100644 --- a/.zdev/comparison/TASKS.md +++ b/.zdev/comparison/TASKS.md @@ -3,14 +3,14 @@ # Tasks: comparison - Total: 5 -- Ready: 2 +- Ready: 1 - Blocked: 2 -- Done: 1 +- Done: 2 | ID | Task | State | Blocked by | | --- | --- | --- | --- | | [comparison-001](tasks/001-add-space-comparison-and-integer-objective-implementations.md) | Add Space comparison and integer objective implementations | done | — | -| [comparison-002](tasks/002-compare-float-quality-independently-of-the-improvement-step.md) | Compare float quality independently of the improvement step | ready | comparison-001 | +| [comparison-002](tasks/002-compare-float-quality-independently-of-the-improvement-step.md) | Compare float quality independently of the improvement step | done | comparison-001 | | [comparison-003](tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md) | Use comparison for sequential BAB and restart incumbent updates | ready | comparison-001 | | [comparison-004](tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md) | Use comparison for parallel BAB and safely report comparison failures | blocked | comparison-002, comparison-003 | | [comparison-005](tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md) | Integrate comparison through portfolios and complete the migration | blocked | comparison-004 | diff --git a/.zdev/comparison/tasks/002-compare-float-quality-independently-of-the-improvement-step.md b/.zdev/comparison/tasks/002-compare-float-quality-independently-of-the-improvement-step.md index 3ae0fc3837..6235d3a198 100644 --- a/.zdev/comparison/tasks/002-compare-float-quality-independently-of-the-improvement-step.md +++ b/.zdev/comparison/tasks/002-compare-float-quality-independently-of-the-improvement-step.md @@ -3,7 +3,7 @@ schema_version = 1 id = "comparison-002" key = "float-comparison" area = "comparison" -status = "open" +status = "done" complexity = "advanced" afk = true priority = "normal" @@ -25,10 +25,20 @@ The [brief](../brief.md) specifies MiniModel upper-bound minimization/lower-boun ## Done when -- [ ] Both MiniModel float bases and FlatZinc float optimization implement the agreed family-specific ordering and document what makes their operands comparable, including tight non-singleton intervals. -- [ ] Tests demonstrate ranking of sub-step improvements, equal keys, both directions, zero/nonzero step, strict threshold boundaries, and adjacent-endpoint intervals. Equal keys produce equivalent cuts and improved keys produce nested cuts on representative comparable solutions. -- [ ] Comparison leaves both inputs unchanged. If a boundary check disproves cut compatibility, report its concrete counterexample rather than silently changing the approved semantics. +- [x] Both MiniModel float bases and FlatZinc float optimization implement the agreed family-specific ordering and document what makes their operands comparable, including tight non-singleton intervals. +- [x] Tests demonstrate ranking of sub-step improvements, equal keys, both directions, zero/nonzero step, strict threshold boundaries, and adjacent-endpoint intervals. Equal keys produce equivalent cuts and improved keys produce nested cuts on representative comparable solutions. +- [x] Comparison leaves both inputs unchanged. If a boundary check disproves cut compatibility, report its concrete counterexample rather than silently changing the approved semantics. ## Validation - Run focused float comparison/cut tests and relevant existing float and FlatZinc tests. Derive the cut-nesting argument from the current posting code; no timing tests or broad floating-point matrix. + +## Result + +Implemented and independently verified endpoint-based float comparison with existing step-based pruning preserved. + +Validation: + +- Built gecode-test and ran the exact focused MiniModel and FlatZinc float comparison tests. +- Strict float relation checks and the complete FlatZinc test group passed. +- Independent review confirmed equal-cut equivalence, nested improved cuts, adjacent-endpoint behavior, and non-mutating comparison. diff --git a/gecode/flatzinc.hh b/gecode/flatzinc.hh index f0f2f224b0..047a2f6c1b 100755 --- a/gecode/flatzinc.hh +++ b/gecode/flatzinc.hh @@ -576,7 +576,7 @@ namespace Gecode { namespace FlatZinc { Gecode::FloatVarArray fv_aux; /// Indicates whether a float variable is introduced by mzn2fzn std::vector fv_introduced; - /// Step by which a next solution has to have lower cost + /// Step used by float optimization cuts and comparison compatibility Gecode::FloatNum step; #endif /// Whether the introduced variables still need to be copied @@ -626,7 +626,14 @@ namespace Gecode { namespace FlatZinc { /// Compare this space with space \a s and print the differences on /// \a out void compare(const Space& s, std::ostream& out) const; - /// Compare integer optimization objectives + /** + * \brief Compare compatible optimization objectives + * + * Integer objectives compare assigned values. Float objectives require + * the same direction and step, and assigned (possibly adjacent-endpoint) + * intervals. Float minimization ranks lower endpoints and maximization + * ranks upper endpoints, independently of the step used by constrain(). + */ virtual SpaceComparison compare(const Space& s) const; /// Compare this space with space \a s and print the differences on /// \a out using \a p diff --git a/gecode/flatzinc/flatzinc.cpp b/gecode/flatzinc/flatzinc.cpp index 25fe49f3d6..a7c08401b0 100644 --- a/gecode/flatzinc/flatzinc.cpp +++ b/gecode/flatzinc/flatzinc.cpp @@ -865,7 +865,11 @@ namespace Gecode { namespace FlatZinc { intVarCount(-1), boolVarCount(-1), floatVarCount(-1), setVarCount(-1), _optVar(-1), _optVarIsInt(true), _lns(0), _lnsInitialSolution(0), _random(random), - _solveAnnotations(nullptr), needAuxVars(true) { + _solveAnnotations(nullptr), +#ifdef GECODE_HAS_FLOAT_VARS + step(0.0), +#endif + needAuxVars(true) { branchInfo.init(); } @@ -2083,14 +2087,28 @@ namespace Gecode { namespace FlatZinc { const FlatZincSpace* other = dynamic_cast(&s); if (other == nullptr) throw DynamicCastFailed("FlatZincSpace::compare"); - if (!_optVarIsInt || !other->_optVarIsInt || + if ((_optVarIsInt != other->_optVarIsInt) || (_method != other->_method) || ((_method != MIN) && (_method != MAX))) throw DynamicCastFailed("FlatZincSpace::compare"); - int a=iv[_optVar].val(), b=other->iv[other->_optVar].val(); - if (a == b) + if (_optVarIsInt) { + int a=iv[_optVar].val(), b=other->iv[other->_optVar].val(); + if (a == b) + return SC_EQUIVALENT; + return ((a < b) == (_method == MIN)) ? SC_BETTER : SC_WORSE; + } +#ifdef GECODE_HAS_FLOAT_VARS + if (step != other->step) + throw DynamicCastFailed("FlatZincSpace::compare"); + FloatVal a=fv[_optVar].val(), b=other->fv[other->_optVar].val(); + FloatNum ak=(_method == MIN) ? a.min() : a.max(); + FloatNum bk=(_method == MIN) ? b.min() : b.max(); + if (ak == bk) return SC_EQUIVALENT; - return ((a < b) == (_method == MIN)) ? SC_BETTER : SC_WORSE; + return ((ak < bk) == (_method == MIN)) ? SC_BETTER : SC_WORSE; +#else + throw DynamicCastFailed("FlatZincSpace::compare"); +#endif } bool diff --git a/gecode/minimodel.hh b/gecode/minimodel.hh index 66e6eeb71d..117c020367 100755 --- a/gecode/minimodel.hh +++ b/gecode/minimodel.hh @@ -2517,6 +2517,9 @@ namespace Gecode { * The class supports using a step value \a step that will make sure * that better solutions must be better by at least the value of * \a step. + * Comparison ranks the upper endpoint of assigned costs independently of + * the step. Both operands must use this base class and the same step. + * Assigned costs may be tight intervals between adjacent float values. * * \ingroup TaskModelMiniModelOptimize */ @@ -2532,6 +2535,9 @@ namespace Gecode { /// Member function constraining according to cost GECODE_MINIMODEL_EXPORT virtual void constrain(const Space& best); + /// Compare upper endpoints of assigned costs with a common step policy + GECODE_MINIMODEL_EXPORT + virtual SpaceComparison compare(const Space& other) const; /// Return variable with current cost virtual FloatVar cost(void) const = 0; }; @@ -2542,6 +2548,9 @@ namespace Gecode { * The class supports using a step value \a step that will make sure * that better solutions must be better by at least the value of * \a step. + * Comparison ranks the lower endpoint of assigned costs independently of + * the step. Both operands must use this base class and the same step. + * Assigned costs may be tight intervals between adjacent float values. * * \ingroup TaskModelMiniModelOptimize */ @@ -2557,6 +2566,9 @@ namespace Gecode { /// Member function constraining according to cost GECODE_MINIMODEL_EXPORT virtual void constrain(const Space& best); + /// Compare lower endpoints of assigned costs with a common step policy + GECODE_MINIMODEL_EXPORT + virtual SpaceComparison compare(const Space& other) const; /// Return variable with current cost virtual FloatVar cost(void) const = 0; }; diff --git a/gecode/minimodel/optimize.cpp b/gecode/minimodel/optimize.cpp index 320e37da4c..0e5253e5c3 100755 --- a/gecode/minimodel/optimize.cpp +++ b/gecode/minimodel/optimize.cpp @@ -149,6 +149,18 @@ namespace Gecode { rel(*this, cost(), FRT_LE, best->cost().max()-step); } + SpaceComparison + FloatMinimizeSpace::compare(const Space& other) const { + const FloatMinimizeSpace* s = + dynamic_cast(&other); + if ((s == nullptr) || (step != s->step)) + throw DynamicCastFailed("FloatMinimizeSpace::compare"); + FloatNum a=cost().val().max(), b=s->cost().val().max(); + if (a == b) + return SC_EQUIVALENT; + return (a < b) ? SC_BETTER : SC_WORSE; + } + void FloatMaximizeSpace::constrain(const Space& _best) { @@ -159,6 +171,18 @@ namespace Gecode { rel(*this, cost(), FRT_GR, best->cost().min()+step); } + SpaceComparison + FloatMaximizeSpace::compare(const Space& other) const { + const FloatMaximizeSpace* s = + dynamic_cast(&other); + if ((s == nullptr) || (step != s->step)) + throw DynamicCastFailed("FloatMaximizeSpace::compare"); + FloatNum a=cost().val().min(), b=s->cost().val().min(); + if (a == b) + return SC_EQUIVALENT; + return (a > b) ? SC_BETTER : SC_WORSE; + } + #endif } diff --git a/test/flatzinc.cpp b/test/flatzinc.cpp index 2f8a221adf..3a9dc8798a 100755 --- a/test/flatzinc.cpp +++ b/test/flatzinc.cpp @@ -34,6 +34,9 @@ #include "test/flatzinc.hh" #include +#include +#include +#include namespace Test { namespace FlatZinc { @@ -105,6 +108,117 @@ namespace Test { namespace FlatZinc { IntegerObjectiveComparison integer_objective_comparison; +#ifdef GECODE_HAS_FLOAT_VARS + /// Verify float objective comparison and its interval-valued cuts. + class FloatObjectiveComparison : public Base { + private: + static std::unique_ptr + model(Gecode::FloatVal v, bool minimize, Gecode::FloatNum step=0.0) { + std::stringstream source; + source << std::showpoint + << std::setprecision(std::numeric_limits::max_digits10) + << "var float: x = " << v.min() << "; solve " + << (minimize ? "minimize" : "maximize") + << " x;\n"; + Gecode::FlatZinc::Printer p; + std::unique_ptr result + (Gecode::FlatZinc::parse(source,p,olog)); + if (result) { + result->fv[result->optVar()] = + Gecode::FloatVar(*result,v.min(),v.max()); + result->step = step; + } + return result; + } + + static bool admitted(Gecode::FloatVal candidate, + const Gecode::FlatZinc::FlatZincSpace& incumbent, + bool minimize, Gecode::FloatNum step) { + std::unique_ptr c = + model(candidate,minimize,step); + c->constrain(incumbent); + return c->status() != Gecode::SS_FAILED; + } + public: + FloatObjectiveComparison(void) + : Base("FlatZinc::FloatObjectiveComparison") {} + + virtual bool run(void) { + using namespace Gecode; + using Gecode::FlatZinc::FlatZincSpace; + const FloatNum next = std::nextafter(1.0,2.0); + std::unique_ptr m9=model(9.5,true,1.0); + std::unique_ptr m10=model(10.0,true,1.0); + std::unique_ptr m10b=model(10.0,true,1.0); + std::unique_ptr x11=model(10.5,false,1.0); + std::unique_ptr x10=model(10.0,false,1.0); + std::unique_ptr x10b=model(10.0,false,1.0); + std::unique_ptr adjacent=model(FloatVal(1.0,next),true); + std::unique_ptr adjacent_key=model(1.0,true); + std::unique_ptr adjacent_max= + model(FloatVal(1.0,next),false); + std::unique_ptr adjacent_max_key=model(next,false); + if (!m9 || !m10 || !m10b || !x11 || !x10 || !x10b || !adjacent || + !adjacent_key || + !adjacent_max || !adjacent_max_key || + (m9->compare(*m10) != SC_BETTER) || + (m10->compare(*m9) != SC_WORSE) || + (m10->compare(*m10b) != SC_EQUIVALENT) || + (x11->compare(*x10) != SC_BETTER) || + (x10->compare(*x11) != SC_WORSE) || + (x10->compare(*x10b) != SC_EQUIVALENT) || + (adjacent->compare(*adjacent_key) != SC_EQUIVALENT) || + (adjacent_max->compare(*adjacent_max_key) != SC_EQUIVALENT)) + return false; + + const FloatVal probes[] = {FloatVal(8.4), FloatVal(8.5), + FloatVal(8.9), FloatVal(9.0)}; + for (unsigned int i=0; i zero=model(10.0,true); + if (admitted(FloatVal(10.0),*zero,true,0.0) || + !admitted(FloatVal(9.0),*zero,true,0.0) || + admitted(FloatVal(11.0),*x10,false,1.0) || + !admitted(FloatVal(11.1),*x10,false,1.0)) + return false; + + FloatVal before=m9->fv[m9->optVar()].val(); + FloatVal before_other=m10->fv[m10->optVar()].val(); + (void) m9->compare(*m10); + if ((m9->fv[m9->optVar()].val().min() != before.min()) || + (m9->fv[m9->optVar()].val().max() != before.max()) || + (m10->fv[m10->optVar()].val().min() != before_other.min()) || + (m10->fv[m10->optVar()].val().max() != before_other.max())) + return false; + std::unique_ptr different_step=model(10.0,true,0.0); + try { (void) m10->compare(*different_step); return false; } + catch (const DynamicCastFailed&) {} + try { (void) m10->compare(*x10); return false; } + catch (const DynamicCastFailed&) {} + return true; + } + }; + + FloatObjectiveComparison float_objective_comparison; +#endif + /// Verify that statistics do not override an explicit Gist mode. class GistStatisticsMode : public Base { private: diff --git a/test/search.cpp b/test/search.cpp index ce910c9b2f..41d96529db 100644 --- a/test/search.cpp +++ b/test/search.cpp @@ -43,6 +43,7 @@ #include #include +#include #include #include #include @@ -1166,6 +1167,34 @@ namespace Test { virtual IntVar cost(void) const { return x; } }; +#ifdef GECODE_HAS_FLOAT_VARS + /// Scalar float minimization objective used for comparison and cut tests + class FloatMinObjective : public FloatMinimizeSpace { + public: + FloatVar x; + FloatMinObjective(FloatVal v, FloatNum s=0.0) + : FloatMinimizeSpace(s), x(*this,v.min(),v.max()) {} + FloatMinObjective(FloatMinObjective& s) : FloatMinimizeSpace(s) { + x.update(*this,s.x); + } + virtual Space* copy(void) { return new FloatMinObjective(*this); } + virtual FloatVar cost(void) const { return x; } + }; + + /// Scalar float maximization objective used for comparison and cut tests + class FloatMaxObjective : public FloatMaximizeSpace { + public: + FloatVar x; + FloatMaxObjective(FloatVal v, FloatNum s=0.0) + : FloatMaximizeSpace(s), x(*this,v.min(),v.max()) {} + FloatMaxObjective(FloatMaxObjective& s) : FloatMaximizeSpace(s) { + x.update(*this,s.x); + } + virtual Space* copy(void) { return new FloatMaxObjective(*this); } + virtual FloatVar cost(void) const { return x; } + }; +#endif + /// Lexicographic integer objective used for comparison tests class LexObjective : public IntLexMinimizeSpace { public: @@ -1181,6 +1210,76 @@ namespace Test { virtual IntVarArgs cost(void) const { return x; } }; +#ifdef GECODE_HAS_FLOAT_VARS + /// Test float objective ranking and compatibility with stepped cuts + class FloatObjectiveComparison : public Base { + private: + template + static bool admitted(FloatVal candidate, FloatNum step, + const Objective& incumbent) { + Objective c(candidate,step); + c.constrain(incumbent); + return c.status() != SS_FAILED; + } + public: + FloatObjectiveComparison(void) + : Base("Search::FloatObjectiveComparison") {} + virtual bool run(void) { + const FloatNum next = std::nextafter(1.0,2.0); + FloatMinObjective m9(9.5,1.0), m10(10.0,1.0), + m10b(10.0,1.0), mzero(10.0), madj(FloatVal(1.0,next)); + FloatMaxObjective x11(10.5,1.0), x10(10.0,1.0), + x10b(10.0,1.0), xadj(FloatVal(1.0,next)); + if ((m9.compare(m10) != SC_BETTER) || + (m10.compare(m9) != SC_WORSE) || + (m10.compare(m10b) != SC_EQUIVALENT) || + (x11.compare(x10) != SC_BETTER) || + (x10.compare(x11) != SC_WORSE) || + (x10.compare(x10b) != SC_EQUIVALENT) || + (madj.compare(FloatMinObjective(next)) != SC_EQUIVALENT) || + (xadj.compare(FloatMaxObjective(1.0)) != SC_EQUIVALENT)) + return false; + + // Equal keys give identical cuts; better keys only tighten them. + const FloatVal probes[] = {FloatVal(8.4), FloatVal(8.5), + FloatVal(8.9), FloatVal(9.0)}; + for (unsigned int i=0; i(probes[i],1.0,m10) != + admitted(probes[i],1.0,m10b)) + return false; + if (admitted(probes[i],1.0,m9) && + !admitted(probes[i],1.0,m10)) + return false; + } + if (admitted(FloatVal(10.0),0.0,mzero) || + !admitted(FloatVal(9.0),0.0,mzero) || + admitted(FloatVal(11.0),1.0,x10) || + !admitted(FloatVal(11.1),1.0,x10)) + return false; + + FloatVal before_m=m9.cost().val(), before_m_other=m10.cost().val(), + before_x=x11.cost().val(), before_x_other=x10.cost().val(); + (void) m9.compare(m10); (void) x11.compare(x10); + if ((m9.cost().val().min() != before_m.min()) || + (m9.cost().val().max() != before_m.max()) || + (m10.cost().val().min() != before_m_other.min()) || + (m10.cost().val().max() != before_m_other.max()) || + (x11.cost().val().min() != before_x.min()) || + (x11.cost().val().max() != before_x.max()) || + (x10.cost().val().min() != before_x_other.min()) || + (x10.cost().val().max() != before_x_other.max())) + return false; + try { (void) mzero.compare(m10); return false; } + catch (const DynamicCastFailed&) {} + try { (void) m10.compare(x10); return false; } + catch (const DynamicCastFailed&) {} + return true; + } + }; + + FloatObjectiveComparison float_objective_comparison; +#endif + /// Lexicographic maximization objective used for comparison tests class LexMaxObjective : public IntLexMaximizeSpace { public: From d6849933c12c9a1c4c959655dc631cb13de3d091 Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Thu, 10 Sep 2026 12:11:39 +0200 Subject: [PATCH 07/15] comparison: use comparison for sequential incumbents Zdev-Change-Id: Zc9f2540cb25a8b2bc131505b4314c6ea435ced462e808cb28c0b3f57b66f8f34 --- .zdev/comparison/TASKS.md | 8 +- ...quential-bab-and-restart-incumbent-upda.md | 18 ++- changelog.in | 12 ++ examples/car-sequencing.cpp | 12 +- gecode/search/exception.cpp | 3 + gecode/search/exception.hpp | 6 + gecode/search/seq/bab.hpp | 11 +- gecode/search/seq/rbs.cpp | 11 +- gecode/search/support.hh | 17 +++ test/search.cpp | 137 ++++++++++++++++++ 10 files changed, 210 insertions(+), 25 deletions(-) diff --git a/.zdev/comparison/TASKS.md b/.zdev/comparison/TASKS.md index c7240c10c5..29e0bf2c1d 100644 --- a/.zdev/comparison/TASKS.md +++ b/.zdev/comparison/TASKS.md @@ -4,13 +4,13 @@ - Total: 5 - Ready: 1 -- Blocked: 2 -- Done: 2 +- Blocked: 1 +- Done: 3 | ID | Task | State | Blocked by | | --- | --- | --- | --- | | [comparison-001](tasks/001-add-space-comparison-and-integer-objective-implementations.md) | Add Space comparison and integer objective implementations | done | — | | [comparison-002](tasks/002-compare-float-quality-independently-of-the-improvement-step.md) | Compare float quality independently of the improvement step | done | comparison-001 | -| [comparison-003](tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md) | Use comparison for sequential BAB and restart incumbent updates | ready | comparison-001 | -| [comparison-004](tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md) | Use comparison for parallel BAB and safely report comparison failures | blocked | comparison-002, comparison-003 | +| [comparison-003](tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md) | Use comparison for sequential BAB and restart incumbent updates | done | comparison-001 | +| [comparison-004](tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md) | Use comparison for parallel BAB and safely report comparison failures | ready | comparison-002, comparison-003 | | [comparison-005](tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md) | Integrate comparison through portfolios and complete the migration | blocked | comparison-004 | diff --git a/.zdev/comparison/tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md b/.zdev/comparison/tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md index defd9d8a62..6619b89a38 100644 --- a/.zdev/comparison/tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md +++ b/.zdev/comparison/tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md @@ -3,7 +3,7 @@ schema_version = 1 id = "comparison-003" key = "sequential-incumbents" area = "comparison" -status = "open" +status = "done" complexity = "standard" afk = true priority = "normal" @@ -25,10 +25,20 @@ Seq::BAB::constrain in search/seq/bab.hpp and RBS::constrain in search/seq/rbs.c ## Done when -- [ ] Incoming incumbents replace retained ones only when better; equal/worse leave them intact. A valid incomparable outcome is explicitly rejected by the initial engine policy, separately from model comparison errors. -- [ ] Actual working spaces still receive constrain(), and recomputation marks and restart state remain correct. A small internal acceptance helper may keep later BAB/PBS policy handling consistent. -- [ ] Relevant custom optimization test models and examples implement their ordering rather than receiving a generic default ranking. Focused tests cover late better/equal/worse updates, the balanced objective, and direct comparison-policy errors. +- [x] Incoming incumbents replace retained ones only when better; equal/worse leave them intact. A valid incomparable outcome is explicitly rejected by the initial engine policy, separately from model comparison errors. +- [x] Actual working spaces still receive constrain(), and recomputation marks and restart state remain correct. A small internal acceptance helper may keep later BAB/PBS policy handling consistent. +- [x] Relevant custom optimization test models and examples implement their ordering rather than receiving a generic default ranking. Focused tests cover late better/equal/worse updates, the balanced objective, and direct comparison-policy errors. ## Validation - Run focused external-update tests and existing sequential BAB/RBS search tests, including stop/resume. Smoke-check the migrated optimization example. No new tests of unrelated pruning internals. + +## Result + +Implemented and independently verified comparison-based external incumbent updates for sequential BAB and RBS. + +Validation: + +- Search::ExternalIncumbent and focused balanced BAB stop/resume tests passed. +- RBS BAB coverage and car-sequencing smoke run passed. +- Independent review confirmed correct ordering, error separation, and continued pruning/restart behavior. diff --git a/changelog.in b/changelog.in index e757356aa0..0a78532928 100755 --- a/changelog.in +++ b/changelog.in @@ -75,6 +75,18 @@ Date: unreleased [DESCRIPTION] This is the development changelog for the next Gecode release. +[ENTRY] +Module: search +What: change +Rank: major +[DESCRIPTION] +Best-solution search now uses Space::compare() when choosing between an +external incumbent and the solution already retained by sequential BAB or +restart-based search. Optimization spaces with a custom objective must +override compare() as well as constrain(). Incomparable objectives are +reported as unsupported by the current best-solution engines. Adding the +virtual comparison function changes the C++ ABI of Space. + [RELEASE] Version: 6.5.0 Date: 2026-10-31 diff --git a/examples/car-sequencing.cpp b/examples/car-sequencing.cpp index de6cc757cb..9addcda22c 100644 --- a/examples/car-sequencing.cpp +++ b/examples/car-sequencing.cpp @@ -369,6 +369,17 @@ class CarSequencing : public Script { rel(*this, nstall, IRT_LE, best.nstall.val()); } + /// Compare objective values + virtual SpaceComparison compare(const Space& _other) const { + const CarSequencing& other = + dynamic_cast(_other); + if (nstall.val() < other.nstall.val()) + return SC_BETTER; + if (nstall.val() > other.nstall.val()) + return SC_WORSE; + return SC_EQUIVALENT; + } + /// Print solution virtual void print(std::ostream& os) const { @@ -634,4 +645,3 @@ namespace { }; // STATISTICS: example-any - diff --git a/gecode/search/exception.cpp b/gecode/search/exception.cpp index 8d99738996..8e3bcb8974 100644 --- a/gecode/search/exception.cpp +++ b/gecode/search/exception.cpp @@ -64,6 +64,9 @@ namespace Gecode { namespace Search { WorkerControlInUse::WorkerControlInUse(const char* l) : Exception(l,"Worker control is already bound or has unsupported engine placement") {} + Incomparable::Incomparable(const char* l) + : Exception(l,"Incomparable objectives are not supported by best solution search") {} + }} // STATISTICS: search-other diff --git a/gecode/search/exception.hpp b/gecode/search/exception.hpp index e28c3fd9ce..16b65661f1 100644 --- a/gecode/search/exception.hpp +++ b/gecode/search/exception.hpp @@ -80,6 +80,12 @@ namespace Gecode { namespace Search { /// Initialize with location \a l WorkerControlInUse(const char* l); }; + /// %Exception: Incomparable objectives are unsupported by best search + class GECODE_SEARCH_EXPORT Incomparable : public Exception { + public: + /// Initialize with location \a l + Incomparable(const char* l); + }; #ifdef GECODE_HAS_EXPERIMENTAL /// %Exception: Missing guidance for heuristic search class GECODE_SEARCH_EXPORT UninitializedGuidance : public Exception { diff --git a/gecode/search/seq/bab.hpp b/gecode/search/seq/bab.hpp index dade2cb075..fed0129c6a 100644 --- a/gecode/search/seq/bab.hpp +++ b/gecode/search/seq/bab.hpp @@ -161,14 +161,9 @@ namespace Gecode { namespace Search { namespace Seq { template forceinline void BAB::constrain(const Space& b) { - if (best != nullptr) { - // Check whether b is in fact better than best - best->constrain(b); - if (best->status(*this) != SS_FAILED) - return; - else - delete best; - } + if ((best != nullptr) && !Search::better(b,*best,"BAB::constrain")) + return; + delete best; best = b.clone(); if (cur != nullptr) cur->constrain(b); diff --git a/gecode/search/seq/rbs.cpp b/gecode/search/seq/rbs.cpp index 033c525b1d..2f30a96a84 100755 --- a/gecode/search/seq/rbs.cpp +++ b/gecode/search/seq/rbs.cpp @@ -130,14 +130,9 @@ namespace Gecode { namespace Search { namespace Seq { RBS::constrain(const Space& b) { if (!best) throw NoBest("RBS::constrain"); - if (last != nullptr) { - last->constrain(b); - if (last->status() == SS_FAILED) { - delete last; - } else { - return; - } - } + if ((last != nullptr) && !Search::better(b,*last,"RBS::constrain")) + return; + delete last; last = b.clone(); master->constrain(b); e->constrain(b); diff --git a/gecode/search/support.hh b/gecode/search/support.hh index fe25d487b2..fc1e6116d8 100644 --- a/gecode/search/support.hh +++ b/gecode/search/support.hh @@ -38,6 +38,23 @@ namespace Gecode { namespace Search { + /// Whether incoming solution \a s should replace incumbent \a b + forceinline bool + better(const Space& s, const Space& b, const char* l) { + switch (s.compare(b)) { + case SC_BETTER: + return true; + case SC_EQUIVALENT: + case SC_WORSE: + return false; + case SC_INCOMPARABLE: + throw Incomparable(l); + default: + GECODE_NEVER; + } + return false; + } + /// Clone space \a s depending on options \a o forceinline Space* snapshot(Space* s, const Options& o); diff --git a/test/search.cpp b/test/search.cpp index 41d96529db..5d344a1e87 100644 --- a/test/search.cpp +++ b/test/search.cpp @@ -558,6 +558,40 @@ namespace Test { } } } + /// Compare objectives used by best solution search + virtual SpaceComparison compare(const Space& _s) const { + const HasSolutions& s = dynamic_cast(_s); + if (htc != s.htc) + throw DynamicCastFailed("HasSolutions::compare"); + int c=0, sc=0; + switch (htc) { + case HTC_LEX_LE: + case HTC_LEX_GR: + for (int i=0; i s.x[i].val()); + return better ? SC_BETTER : SC_WORSE; + } + return SC_EQUIVALENT; + case HTC_BAL_LE: + case HTC_BAL_GR: + c = std::abs(x[0].val()+x[1].val()+x[2].val()- + x[3].val()-x[4].val()-x[5].val()); + sc = std::abs(s.x[0].val()+s.x[1].val()+s.x[2].val()- + s.x[3].val()-s.x[4].val()-s.x[5].val()); + if (c == sc) + return SC_EQUIVALENT; + return ((htc == HTC_BAL_LE) ? (c < sc) : (c > sc)) ? + SC_BETTER : SC_WORSE; + case HTC_NONE: + return SC_EQUIVALENT; + default: + GECODE_NEVER; + } + return SC_INCOMPARABLE; + } /// Return number of solutions virtual int solutions(void) const { if (htb1 == HTB_NONE) { @@ -1167,6 +1201,109 @@ namespace Test { virtual IntVar cost(void) const { return x; } }; + /// Objective used to test external incumbent updates + class ExternalObjective : public IntMinimizeSpace { + public: + static int constraints; + IntVar x; + ExternalObjective(void) : x(*this,0,10) { + Gecode::branch(*this,x,INT_VAL_MIN()); + } + ExternalObjective(int v) : x(*this,v,v) {} + ExternalObjective(ExternalObjective& s) : IntMinimizeSpace(s) { + x.update(*this,s.x); + } + virtual Space* copy(void) { return new ExternalObjective(*this); } + virtual IntVar cost(void) const { return x; } + virtual void constrain(const Space& s) { + constraints++; + IntMinimizeSpace::constrain(s); + } + }; + + int ExternalObjective::constraints = 0; + + /// Objective with a genuine partial-order result + class IncomparableObjective : public ExternalObjective { + public: + IncomparableObjective(int v) : ExternalObjective(v) {} + IncomparableObjective(IncomparableObjective& s) : ExternalObjective(s) {} + virtual Space* copy(void) { return new IncomparableObjective(*this); } + virtual SpaceComparison compare(const Space&) const { + return SC_INCOMPARABLE; + } + }; + + /// Test sequential BAB and RBS external incumbent arbitration + class ExternalIncumbent : public Base { + private: + template + static bool updates(Engine& e) { + ExternalObjective five(5), equal(5), worse(7), better(3); + int n = ExternalObjective::constraints; + e.constrain(five); + int installed = ExternalObjective::constraints; + e.constrain(equal); + e.constrain(worse); + if ((installed <= n) || (ExternalObjective::constraints != installed)) + return false; + e.constrain(better); + return ExternalObjective::constraints > installed; + } + public: + ExternalIncumbent(void) : Base("Search::ExternalIncumbent") {} + virtual bool run(void) { + Gecode::Search::Options o; + ExternalObjective* bm = new ExternalObjective; + Gecode::Search::Engine* bab = Gecode::Search::babengine(bm,o); + delete bm; + if (!updates(*bab)) { + delete bab; + return false; + } + delete bab; + + o.cutoff = Gecode::Search::Cutoff::constant(10); + ExternalObjective* rm = new ExternalObjective; + Gecode::Search::Engine* rbs = + Gecode::Search::build >(rm,o); + delete rm; + if (!updates(*rbs)) { + delete rbs; + return false; + } + delete rbs; + + IncomparableObjective incomparable(4), incumbent(5); + ExternalObjective* im = new ExternalObjective; + Gecode::Search::Engine* rejecting = Gecode::Search::babengine( + im,Gecode::Search::Options()); + delete im; + rejecting->constrain(incumbent); + try { + rejecting->constrain(incomparable); + delete rejecting; + return false; + } catch (const Gecode::Search::Incomparable&) {} + delete rejecting; + + SolveImmediate unsupported(HTB_NONE,HTB_NONE,HTB_NONE); + Gecode::Search::Engine* missing = Gecode::Search::babengine( + unsupported.clone(),Gecode::Search::Options()); + missing->constrain(unsupported); + try { + missing->constrain(unsupported); + delete missing; + return false; + } catch (const SpaceNoComparison&) {} + delete missing; + return true; + } + }; + + ExternalIncumbent external_incumbent; + #ifdef GECODE_HAS_FLOAT_VARS /// Scalar float minimization objective used for comparison and cut tests class FloatMinObjective : public FloatMinimizeSpace { From 1ea17b6a0116fa93d6e2219ebc6803a5daca2944 Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Thu, 10 Sep 2026 12:59:29 +0200 Subject: [PATCH 08/15] Use comparison in parallel BAB Arbitrate candidates and external incumbents with non-mutating comparison, invalidate superseded queued results, and deliver comparison failures safely through next(). Preserve restart support by clearing failure state during reset, with focused parallel BAB regressions. Zdev-Change-Id: Z7926867238a6c0cfb64f226c42bf1df0d6994fedf653be929d33ef60ec652f92 --- .zdev/comparison/TASKS.md | 8 +- ...rallel-bab-and-safely-report-comparison.md | 21 +- gecode/search/par/bab.hh | 6 + gecode/search/par/bab.hpp | 86 +++++-- test/search.cpp | 214 ++++++++++++++++++ 5 files changed, 306 insertions(+), 29 deletions(-) diff --git a/.zdev/comparison/TASKS.md b/.zdev/comparison/TASKS.md index 29e0bf2c1d..28542801ae 100644 --- a/.zdev/comparison/TASKS.md +++ b/.zdev/comparison/TASKS.md @@ -4,13 +4,13 @@ - Total: 5 - Ready: 1 -- Blocked: 1 -- Done: 3 +- Blocked: 0 +- Done: 4 | ID | Task | State | Blocked by | | --- | --- | --- | --- | | [comparison-001](tasks/001-add-space-comparison-and-integer-objective-implementations.md) | Add Space comparison and integer objective implementations | done | — | | [comparison-002](tasks/002-compare-float-quality-independently-of-the-improvement-step.md) | Compare float quality independently of the improvement step | done | comparison-001 | | [comparison-003](tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md) | Use comparison for sequential BAB and restart incumbent updates | done | comparison-001 | -| [comparison-004](tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md) | Use comparison for parallel BAB and safely report comparison failures | ready | comparison-002, comparison-003 | -| [comparison-005](tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md) | Integrate comparison through portfolios and complete the migration | blocked | comparison-004 | +| [comparison-004](tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md) | Use comparison for parallel BAB and safely report comparison failures | done | comparison-002, comparison-003 | +| [comparison-005](tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md) | Integrate comparison through portfolios and complete the migration | ready | comparison-004 | diff --git a/.zdev/comparison/tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md b/.zdev/comparison/tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md index 0a9fc47445..c7a70d0aaa 100644 --- a/.zdev/comparison/tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md +++ b/.zdev/comparison/tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md @@ -3,7 +3,7 @@ schema_version = 1 id = "comparison-004" key = "parallel-bab" area = "comparison" -status = "open" +status = "done" complexity = "advanced" afk = true priority = "normal" @@ -25,11 +25,22 @@ Par::BAB::solution and constrain in search/par/bab.hpp currently probe with cons ## Done when -- [ ] Candidates and external incumbents use incoming-versus-retained comparison. Only accepted incumbents are cloned/broadcast; completed solutions are never constrained or propagated to rank them. -- [ ] After an external incumbent update, queued results that violate the new incumbent contract are not returned. Accepted queue ordering and ownership remain valid. -- [ ] Missing comparison, model comparison errors, and initial-policy rejection of incomparability wake/block workers safely and reach the controlling call instead of terminating a detached thread, hanging, or reporting normal exhaustion. The engine can be safely destroyed after failure. -- [ ] Focused tests check monotone results and the known optimum for a small custom objective, late external bounds with pending results, sub-step float ranking, and safe failure delivery. +- [x] Candidates and external incumbents use incoming-versus-retained comparison. Only accepted incumbents are cloned/broadcast; completed solutions are never constrained or propagated to rank them. +- [x] After an external incumbent update, queued results that violate the new incumbent contract are not returned. Accepted queue ordering and ownership remain valid. +- [x] Missing comparison, model comparison errors, and initial-policy rejection of incomparability wake/block workers safely and reach the controlling call instead of terminating a detached thread, hanging, or reporting normal exhaustion. The engine can be safely destroyed after failure. +- [x] Focused tests check monotone results and the known optimum for a small custom objective, late external bounds with pending results, sub-step float ranking, and safe failure delivery. ## Validation - Run focused parallel BAB tests and existing BAB stop/resume/recomputation coverage. Use bounded failure tests that detect hangs; do not require deterministic parallel solution counts or schedules. + +## Result + +Parallel BAB now arbitrates incumbents with non-mutating comparison, invalidates superseded queued results, and safely delivers comparison failures with restart support. + +Validation: + +- Built the gecode-test target successfully. +- Search::ParallelBABComparison passed 25 bounded iterations, including all failure categories and reset/reuse. +- Focused float and external-incumbent regressions passed repeatedly. +- Existing parallel BAB coverage passed under bounded execution, and git diff --check passed. diff --git a/gecode/search/par/bab.hh b/gecode/search/par/bab.hh index 4a1911b208..cf5a3d7fb9 100644 --- a/gecode/search/par/bab.hh +++ b/gecode/search/par/bab.hh @@ -35,6 +35,7 @@ #define GECODE_SEARCH_PAR_BAB_HH #include +#include namespace Gecode { namespace Search { namespace Par { @@ -46,6 +47,7 @@ namespace Gecode { namespace Search { namespace Par { using Engine::busy; using Engine::stop; using Engine::block; + using Engine::cmd; using Engine::e_search; using Engine::e_reset_ack_start; using Engine::e_reset_ack_stop; @@ -104,6 +106,8 @@ namespace Gecode { namespace Search { namespace Par { Worker** _worker; /// Best solution so far Space* best; + /// Failure raised while accepting a solution in a worker thread + std::exception_ptr failure; public: /// Provide access to worker \a i Worker* worker(unsigned int i) const; @@ -118,6 +122,8 @@ namespace Gecode { namespace Search { namespace Par { //@{ /// Initialize for space \a s with options \a o BAB(Space* s, const Options& o); + /// Return next solution + virtual Space* next(void); /// Return statistics virtual Statistics statistics(void) const; /// Reset engine to restart at space \a s diff --git a/gecode/search/par/bab.hpp b/gecode/search/par/bab.hpp index deb0ae5f9b..10c43fb536 100755 --- a/gecode/search/par/bab.hpp +++ b/gecode/search/par/bab.hpp @@ -89,7 +89,7 @@ namespace Gecode { namespace Search { namespace Par { template forceinline BAB::BAB(Space* s, const Options& o) - : Engine(o), best(nullptr) { + : Engine(o), best(nullptr), failure(nullptr) { WrapTraceRecorder::engine(o.tracer, SearchTracer::EngineType::DFS, workers()); // Create workers @@ -126,17 +126,33 @@ namespace Gecode { namespace Search { namespace Par { template forceinline void BAB::solution(Space* s) { - m_search.acquire(); - if (best != nullptr) { - s->constrain(*best); - if (s->status() == SS_FAILED) { + Support::Lock lock(m_search); + if (failure != nullptr) { + delete s; + return; + } + try { + if ((best != nullptr) && + !Search::better(*s,*best,"BAB::solution")) { delete s; - m_search.release(); return; - } else { - delete best; - best = s->clone(); } + } catch (...) { + delete s; + while (!solutions.empty()) + delete solutions.pop(); + failure = std::current_exception(); + // A null entry is private to BAB and only wakes Engine::next. That + // call blocks the workers before BAB::next rethrows the failure. + bool bs = signal(); + solutions.push(nullptr); + if (bs) + e_search.signal(); + return; + } + if (best != nullptr) { + delete best; + best = s->clone(); } else { best = s->clone(); } @@ -147,7 +163,6 @@ namespace Gecode { namespace Search { namespace Par { solutions.push(s); if (bs) e_search.signal(); - m_search.release(); } @@ -198,20 +213,45 @@ namespace Gecode { namespace Search { namespace Par { template void BAB::constrain(const Space& b) { - m_search.acquire(); + Support::Lock lock(m_search); + if ((best != nullptr) && + !Search::better(b,*best,"BAB::constrain")) + return; + while (!solutions.empty()) + delete solutions.pop(); if (best != nullptr) { - best->constrain(b); - if (best->status() != SS_FAILED) { - m_search.release(); - return; - } delete best; } best = b.clone(); // Announce better solutions for (unsigned int i=0U; ibetter(best); - m_search.release(); + } + + template + Space* + BAB::next(void) { + std::exception_ptr f; + { + Support::Lock lock(m_search); + f = failure; + } + if (f != nullptr) + std::rethrow_exception(f); + Space* s = Engine::next(); + { + Support::Lock lock(m_search); + f = failure; + } + if (f != nullptr) { + // Engine::next can consume the failure marker through its initial + // nonempty-queue path. Unlike its event-wait path, that path does not + // block workers, so finish the normal lifecycle before reporting. + if (cmd() != C_WAIT) + block(); + std::rethrow_exception(f); + } + return s; } /* @@ -392,9 +432,15 @@ namespace Gecode { namespace Search { namespace Par { // Wait for reset cycle started e_reset_ack_start.wait(); // All workers are marked as busy again - delete best; - best = nullptr; - n_busy = workers(); + { + Support::Lock lock(m_search); + while (!solutions.empty()) + delete solutions.pop(); + failure = nullptr; + delete best; + best = nullptr; + n_busy = workers(); + } for (unsigned int i=1U; ireset(nullptr,0); worker(0)->reset(s,opt().nogoods_limit); diff --git a/test/search.cpp b/test/search.cpp index 5d344a1e87..510e53e688 100644 --- a/test/search.cpp +++ b/test/search.cpp @@ -1223,6 +1223,53 @@ namespace Test { int ExternalObjective::constraints = 0; + /// Small objective whose solutions arrive from worst to best + class ParallelObjective : public IntMinimizeSpace { + public: + IntVar x; + ParallelObjective(void) : x(*this,0,10) { + Gecode::branch(*this,x,INT_VAL_MAX()); + } + ParallelObjective(int v) : x(*this,v,v) {} + ParallelObjective(ParallelObjective& s) : IntMinimizeSpace(s) { + x.update(*this,s.x); + } + virtual Space* copy(void) { return new ParallelObjective(*this); } + virtual IntVar cost(void) const { return x; } + }; + + /// Comparison error raised by a model + class ComparisonError : public Exception { + public: + ComparisonError(void) : Exception("ParallelObjective::compare", + "model comparison failed") {} + }; + + /// Objective used to exercise asynchronous comparison failures + class FailingParallelObjective : public Space { + public: + enum Failure { MISSING, THROWN, INCOMPARABLE }; + IntVar x; + Failure failure; + FailingParallelObjective(Failure f) : x(*this,0,10), failure(f) { + Gecode::branch(*this,x,INT_VAL_MAX()); + } + FailingParallelObjective(FailingParallelObjective& s) + : Space(s), failure(s.failure) { + x.update(*this,s.x); + } + virtual Space* copy(void) { + return new FailingParallelObjective(*this); + } + virtual SpaceComparison compare(const Space& s) const { + if (failure == MISSING) + return Space::compare(s); + if (failure == THROWN) + throw ComparisonError(); + return SC_INCOMPARABLE; + } + }; + /// Objective with a genuine partial-order result class IncomparableObjective : public ExternalObjective { public: @@ -1304,7 +1351,151 @@ namespace Test { ExternalIncumbent external_incumbent; + /// Test parallel BAB solution arbitration and failure delivery + class ParallelBABComparison : public Base { + private: + static Gecode::Search::Options options(void) { + Gecode::Search::Options o; + o.threads = 2; + return o; + } + static bool resetSearch(Gecode::Search::Engine* e) { + e->reset(new ParallelObjective); + int previous = 11; + ParallelObjective* s; + while ((s = static_cast(e->next())) != nullptr) { + int value = s->x.val(); + delete s; + if (value >= previous) + return false; + previous = value; + } + return previous == 0; + } + static bool missingFailure(void) { + Gecode::Search::TimeStop stop(5000); + Gecode::Search::Options o = options(); + o.stop = &stop; + FailingParallelObjective* m = + new FailingParallelObjective(FailingParallelObjective::MISSING); + Gecode::Search::Engine* e = Gecode::Search::babengine(m,o); + delete m; + bool repeated = false; + try { + while (Space* s = e->next()) delete s; + } catch (const SpaceNoComparison&) { + try { (void) e->next(); } + catch (const SpaceNoComparison&) { repeated = true; } + } + if (!repeated) { + delete e; + return false; + } + bool recovered = resetSearch(e); + delete e; + return recovered; + } + static bool thrownFailure(void) { + Gecode::Search::TimeStop stop(5000); + Gecode::Search::Options o = options(); + o.stop = &stop; + FailingParallelObjective* m = + new FailingParallelObjective(FailingParallelObjective::THROWN); + Gecode::Search::Engine* e = Gecode::Search::babengine(m,o); + delete m; + try { + while (Space* s = e->next()) delete s; + } catch (const ComparisonError&) { + bool recovered = resetSearch(e); + delete e; + return recovered; + } + delete e; + return false; + } + static bool incomparableFailure(void) { + Gecode::Search::TimeStop stop(5000); + Gecode::Search::Options o = options(); + o.stop = &stop; + FailingParallelObjective* m = new FailingParallelObjective( + FailingParallelObjective::INCOMPARABLE); + Gecode::Search::Engine* e = Gecode::Search::babengine(m,o); + delete m; + try { + while (Space* s = e->next()) delete s; + } catch (const Gecode::Search::Incomparable&) { + bool recovered = resetSearch(e); + delete e; + return recovered; + } + delete e; + return false; + } + public: + ParallelBABComparison(void) : Base("Search::ParallelBABComparison") {} + virtual bool run(void) { + Gecode::Search::TimeStop stop(5000); + Gecode::Search::Options o = options(); + o.stop = &stop; + ParallelObjective* m = new ParallelObjective; + Gecode::BAB bab(m,o); + delete m; + int previous = 11; + ParallelObjective* s; + while ((s = bab.next()) != nullptr) { + int value = s->x.val(); + delete s; + if (value >= previous) + return false; + previous = value; + } + if (previous != 0) + return false; + + m = new ParallelObjective; + Gecode::Search::Engine* bounded = Gecode::Search::babengine(m,o); + delete m; + if (Space* first = bounded->next()) + delete first; + // This is stronger than every solution in the model, so it also + // supersedes any results queued while the first result was returned. + ParallelObjective bound(-1); + bounded->constrain(bound); + while ((s = static_cast(bounded->next())) != + nullptr) { + int value = s->x.val(); + delete s; + if (value >= -1) { + delete bounded; + return false; + } + } + delete bounded; + return missingFailure() && thrownFailure() && incomparableFailure(); + } + }; + + ParallelBABComparison parallel_bab_comparison; + #ifdef GECODE_HAS_FLOAT_VARS + /// Stepped float objective for parallel BAB admission + class ParallelFloatObjective : public FloatMinimizeSpace { + public: + FloatVar x; + ParallelFloatObjective(void) + : FloatMinimizeSpace(1.0), x(*this,8.0,10.0) { + Gecode::branch(*this,x,FLOAT_VAL_SPLIT_MAX()); + } + ParallelFloatObjective(FloatNum v) + : FloatMinimizeSpace(1.0), x(*this,v,v) {} + ParallelFloatObjective(ParallelFloatObjective& s) + : FloatMinimizeSpace(s) { + x.update(*this,s.x); + } + virtual Space* copy(void) { return new ParallelFloatObjective(*this); } + virtual FloatVar cost(void) const { return x; } + }; + /// Scalar float minimization objective used for comparison and cut tests class FloatMinObjective : public FloatMinimizeSpace { public: @@ -1410,6 +1601,29 @@ namespace Test { catch (const DynamicCastFailed&) {} try { (void) m10.compare(x10); return false; } catch (const DynamicCastFailed&) {} + + // A bound better by less than the step still replaces the incumbent. + Gecode::Search::TimeStop stop(5000); + Gecode::Search::Options o; + o.threads = 2; + o.stop = &stop; + ParallelFloatObjective* root = new ParallelFloatObjective; + std::unique_ptr + bab(Gecode::Search::babengine(root,o)); + delete root; + bab->constrain(ParallelFloatObjective(10.0)); + bab->constrain(ParallelFloatObjective(9.5)); + int found = 0; + bool substep = true; + while (Space* s = bab->next()) { + found++; + substep = substep && + (static_cast(s)->x.val().max() < 8.5); + delete s; + } + substep = substep && (found == 1) && !bab->stopped(); + if (!substep) + return false; return true; } }; From 68c25b9d7394415000224199fe40472dae198602 Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Thu, 10 Sep 2026 13:21:36 +0200 Subject: [PATCH 09/15] Integrate comparison through portfolios Use non-mutating comparison for PBS incumbent arbitration and safely return collector or nested-asset failures after portfolio workers quiesce. Add mixed portfolio regressions and document model-defined comparability, float ranking, migration, and the possible future best-effort incomparability policy. Zdev-Change-Id: Ze012cd6e1d4ae7aebf1483d8aabd40f2204a99b09907e93c1a7e61d6dfc7b911 --- .zdev/comparison/TASKS.md | 6 +- ...through-portfolios-and-complete-the-mig.md | 21 +++- changelog.in | 17 ++- gecode/kernel/core.hpp | 4 + gecode/search.hh | 4 + gecode/search/par/pbs.hh | 5 + gecode/search/par/pbs.hpp | 107 +++++++++++++----- test/search.cpp | 105 +++++++++++++++++ 8 files changed, 224 insertions(+), 45 deletions(-) diff --git a/.zdev/comparison/TASKS.md b/.zdev/comparison/TASKS.md index 28542801ae..e3077d63ed 100644 --- a/.zdev/comparison/TASKS.md +++ b/.zdev/comparison/TASKS.md @@ -3,9 +3,9 @@ # Tasks: comparison - Total: 5 -- Ready: 1 +- Ready: 0 - Blocked: 0 -- Done: 4 +- Done: 5 | ID | Task | State | Blocked by | | --- | --- | --- | --- | @@ -13,4 +13,4 @@ | [comparison-002](tasks/002-compare-float-quality-independently-of-the-improvement-step.md) | Compare float quality independently of the improvement step | done | comparison-001 | | [comparison-003](tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md) | Use comparison for sequential BAB and restart incumbent updates | done | comparison-001 | | [comparison-004](tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md) | Use comparison for parallel BAB and safely report comparison failures | done | comparison-002, comparison-003 | -| [comparison-005](tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md) | Integrate comparison through portfolios and complete the migration | ready | comparison-004 | +| [comparison-005](tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md) | Integrate comparison through portfolios and complete the migration | done | comparison-004 | diff --git a/.zdev/comparison/tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md b/.zdev/comparison/tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md index a750aee650..056d00a8e9 100644 --- a/.zdev/comparison/tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md +++ b/.zdev/comparison/tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md @@ -3,7 +3,7 @@ schema_version = 1 id = "comparison-005" key = "portfolio-comparison" area = "comparison" -status = "open" +status = "done" complexity = "advanced" afk = true priority = "normal" @@ -25,11 +25,22 @@ Par::PBS::CollectBest::add/constrain mutate the retained solution; reporter cont ## Done when -- [ ] CollectBest accepts only better candidates initially, preserves reporter on rejection, and never mutates a solution merely to rank it. External updates and pending results use the same policy as BAB/RBS. -- [ ] Comparison and policy failures from the collector or nested asset calls stop/wake and complete safely, reaching the outer caller; deletion after failure does not hang or race worker completion. -- [ ] Focused direct and mixed PBS/RBS/BAB tests establish expected objective improvement/optimum, sub-step float ranking, equivalent-candidate handling, and safe incomparable/error reporting. Existing satisfaction paths remain functional. -- [ ] API/search documentation and the Gecode 7 changelog explain migration, model-defined comparability, float ranking, and current rejection policy. They explicitly preserve the possible future best-effort rule 'incomparable incoming solution becomes current best' without claiming completeness or requiring a changed comparison interface. +- [x] CollectBest accepts only better candidates initially, preserves reporter on rejection, and never mutates a solution merely to rank it. External updates and pending results use the same policy as BAB/RBS. +- [x] Comparison and policy failures from the collector or nested asset calls stop/wake and complete safely, reaching the outer caller; deletion after failure does not hang or race worker completion. +- [x] Focused direct and mixed PBS/RBS/BAB tests establish expected objective improvement/optimum, sub-step float ranking, equivalent-candidate handling, and safe incomparable/error reporting. Existing satisfaction paths remain functional. +- [x] API/search documentation and the Gecode 7 changelog explain migration, model-defined comparability, float ranking, and current rejection policy. They explicitly preserve the possible future best-effort rule 'incomparable incoming solution becomes current best' without claiming completeness or requiring a changed comparison interface. ## Validation - Run focused portfolio and nested-failure tests, then the existing Search:: suite and relevant FlatZinc checks. Audit remaining constrain/status calls to confirm they are pruning rather than arbitration. Run the existing no-thread build/check path for touched conditional code; no new configuration matrix. + +## Result + +PBS now applies non-mutating comparison consistently, safely returns nested comparison failures, and documents the Gecode 7 migration and future incomparability policy option. + +Validation: + +- Threaded and no-thread gecode-test builds passed. +- The complete Search::PBS:: matrix and focused mixed PBS/RBS/BAB, nested-failure, float, external-incumbent, and FlatZinc checks passed. +- Independent verification confirmed ownership, reporter retention, failure quiescence, satisfaction behavior, and remaining constrain/status pruning uses. +- git diff --check and immutable snapshot comparison passed. diff --git a/changelog.in b/changelog.in index 0a78532928..66d38ea2b7 100755 --- a/changelog.in +++ b/changelog.in @@ -80,12 +80,17 @@ Module: search What: change Rank: major [DESCRIPTION] -Best-solution search now uses Space::compare() when choosing between an -external incumbent and the solution already retained by sequential BAB or -restart-based search. Optimization spaces with a custom objective must -override compare() as well as constrain(). Incomparable objectives are -reported as unsupported by the current best-solution engines. Adding the -virtual comparison function changes the C++ ABI of Space. +Best-solution search now uses Space::compare() when choosing between +independently produced solutions, including external incumbents and portfolio +assets. Optimization spaces with a custom objective must override compare() as +well as constrain(). compare() ranks solutions without changing them; +constrain() still posts improvement restrictions. Comparability is defined by +the model. The built-in floating-point models compare their documented +objective bounds, independently of the improvement step. Current exact engines +reject incomparable incoming solutions. A future best-effort policy could +instead make an incomparable incoming solution current best, without changing +the comparison interface, but that would not by itself guarantee completeness +or maximality. [RELEASE] Version: 6.5.0 diff --git a/gecode/kernel/core.hpp b/gecode/kernel/core.hpp index e1eba7db59..7e333fa08b 100755 --- a/gecode/kernel/core.hpp +++ b/gecode/kernel/core.hpp @@ -2162,9 +2162,13 @@ namespace Gecode { * The comparison is read-only and receiver-relative. Implementations may * require objective information sufficient for the relation they report. * The default implementation throws SpaceNoComparison. + * SC_INCOMPARABLE is a model-defined ordering result, not missing + * information. Current exact best-solution engines reject it. * * Optimization models overriding constrain with a different objective * must also override compare. + * constrain remains responsible for posting improvement restrictions; + * search does not use it to rank two solutions. */ GECODE_KERNEL_EXPORT virtual SpaceComparison compare(const Space& other) const; diff --git a/gecode/search.hh b/gecode/search.hh index 11b40fc458..7508a5a19f 100755 --- a/gecode/search.hh +++ b/gecode/search.hh @@ -1524,6 +1524,10 @@ namespace Gecode { * The engine will run a portfolio with a number of assets as defined * by the options \a o. The engine supports parallel execution of * assets by using the number of threads as defined by the options. + * For best-solution assets, incoming solutions replace the retained + * incumbent only when Space::compare reports SC_BETTER. Equivalent and + * worse solutions are discarded; incomparable solutions are currently + * reported as unsupported. * An external worker control in \a o is supported only for a single * homogeneous asset. Multiple controlled assets require explicit engine * builders, each with its own control and immutable worker capacity. diff --git a/gecode/search/par/pbs.hh b/gecode/search/par/pbs.hh index 40d93e7252..7aabfa9558 100644 --- a/gecode/search/par/pbs.hh +++ b/gecode/search/par/pbs.hh @@ -40,6 +40,7 @@ #include #include +#include namespace Gecode { namespace Search { namespace Par { @@ -190,8 +191,12 @@ namespace Gecode { namespace Search { namespace Par { unsigned int n_busy; /// Signal that number of busy slaves becomes zero Support::Event idle; + /// Failure raised while running or updating a portfolio asset + std::exception_ptr failure; /// Process report from slave, return false if solution was ignored bool report(Slave* slave, Space* s); + /// Process failure from slave + void fail(void); /** * The key invariant of the engine is as follows: * - n_busy is always zero outside the next() function. diff --git a/gecode/search/par/pbs.hpp b/gecode/search/par/pbs.hpp index 0f7f03e769..80005fafaa 100755 --- a/gecode/search/par/pbs.hpp +++ b/gecode/search/par/pbs.hpp @@ -94,29 +94,22 @@ namespace Gecode { namespace Search { namespace Par { : b(nullptr), reporter(nullptr) {} forceinline bool CollectBest::add(Space* s, Slave* r) { - if (b != nullptr) { - b->constrain(*s); - if (b->status() == SS_FAILED) { - delete b; - } else { - delete s; - return false; - } + if ((b != nullptr) && + !Search::better(*s,*b,"PBS::CollectBest::add")) { + delete s; + return false; } + delete b; b = s; reporter = r; return true; } forceinline bool CollectBest::constrain(const Space& s) { - if (b != nullptr) { - b->constrain(s); - if (b->status() == SS_FAILED) { - delete b; - } else { - return false; - } - } + if ((b != nullptr) && + !Search::better(s,*b,"PBS::CollectBest::constrain")) + return false; + delete b; b = s.clone(); reporter = nullptr; return true; @@ -222,8 +215,16 @@ namespace Gecode { namespace Search { namespace Par { bool wake = false; m.acquire(); if (s != nullptr) { - b = solutions.add(s,slave); - if (b) { + try { + b = solutions.add(s,slave); + if (b) { + tostop.store(true, std::memory_order_release); + wake = true; + } + } catch (...) { + delete s; + if (failure == nullptr) + failure = std::current_exception(); tostop.store(true, std::memory_order_release); wake = true; } @@ -258,22 +259,45 @@ namespace Gecode { namespace Search { namespace Par { return b; } + template + forceinline void + PBS::fail(void) { + m.acquire(); + if (failure == nullptr) + failure = std::current_exception(); + tostop.store(true, std::memory_order_release); + for (unsigned int i=0U; iwake(); + if (--n_busy == 0) + idle.signal(); + m.release(); + } + template void Slave::run(void) { - Space* s; - do { - s = slave->next(); - } while (!master->report(this,s)); + try { + Space* s; + do { + s = slave->next(); + } while (!master->report(this,s)); + } catch (...) { + master->fail(); + } } template Space* PBS::next(void) { m.acquire(); + if (failure != nullptr) { + std::exception_ptr f = failure; + m.release(); + std::rethrow_exception(f); + } if (solutions.empty()) slave_stop.store(false, std::memory_order_release); - while (solutions.empty() && (n_active > 0) && + while (solutions.empty() && (n_active > 0) && (failure == nullptr) && !slave_stop.load(std::memory_order_acquire)) { // Clear the internal stop used to interrupt sibling slaves tostop.store(false, std::memory_order_release); @@ -310,10 +334,26 @@ namespace Gecode { namespace Search { namespace Par { } else { Slave* r; s = solutions.get(r); - if (Collect::best) - for (unsigned int i=0U; iconstrain(*s); + if (Collect::best) { + try { + for (unsigned int i=0U; iconstrain(*s); + } catch (...) { + delete s; + failure = std::current_exception(); + std::exception_ptr f = failure; + m.release(); + std::rethrow_exception(f); + } + } + } + + if (failure != nullptr) { + delete s; + std::exception_ptr f = failure; + m.release(); + std::rethrow_exception(f); } m.release(); @@ -342,10 +382,15 @@ namespace Gecode { namespace Search { namespace Par { assert(n_busy == 0); if (!Collect::best) throw NoBest("PBS::constrain"); - if (solutions.constrain(b)) { - // The solution is better - for (unsigned int i=0U; iconstrain(b); + try { + if (solutions.constrain(b)) { + // The solution is better + for (unsigned int i=0U; iconstrain(b); + } + } catch (...) { + failure = std::current_exception(); + std::rethrow_exception(failure); } } diff --git a/test/search.cpp b/test/search.cpp index 510e53e688..03e4e12ccf 100644 --- a/test/search.cpp +++ b/test/search.cpp @@ -153,6 +153,11 @@ namespace Test { ceiling = static_cast(s).node().cost; if (node().cost >= ceiling) fail(); } + SpaceComparison compare(const Space& s) const override { + int a = node().cost; + int b = static_cast(s).node().cost; + return (a < b) ? SC_BETTER : ((a > b) ? SC_WORSE : SC_EQUIVALENT); + } /// Guidance minimizing cost, or maximizing negated cost static Gecode::Search::HeuristicGuidance guidance(bool max, bool ranked, bool dominance, bool keyed) { @@ -465,6 +470,10 @@ namespace Test { virtual void constrain(const Space&) { fail(); } + /// Treat the single solution as equivalent across best-search assets + virtual SpaceComparison compare(const Space&) const { + return SC_EQUIVALENT; + } /// Return number of solutions virtual int solutions(void) const { return 1; @@ -1477,6 +1486,102 @@ namespace Test { ParallelBABComparison parallel_bab_comparison; + /// Test portfolio comparison, external bounds, and nested failures + class PortfolioComparison : public Base { + private: + static Gecode::Search::Options options(void) { + Gecode::Search::Options o; + o.assets = 2; + o.threads = 2; + return o; + } + static bool failure(FailingParallelObjective::Failure f) { + FailingParallelObjective* m = new FailingParallelObjective(f); + Gecode::PBS pbs(m,options()); + delete m; + try { + while (Space* s = pbs.next()) delete s; + } catch (const SpaceNoComparison&) { + if (f != FailingParallelObjective::MISSING) + return false; + try { (void) pbs.next(); } + catch (const SpaceNoComparison&) { return true; } + } catch (const ComparisonError&) { + return f == FailingParallelObjective::THROWN; + } catch (const Gecode::Search::Incomparable&) { + return f == FailingParallelObjective::INCOMPARABLE; + } + return false; + } + public: + PortfolioComparison(void) : Base("Search::PortfolioComparison") {} + virtual bool run(void) { + Gecode::Search::Options o = options(); + ParallelObjective* m = new ParallelObjective; + Gecode::PBS pbs(m,o); + delete m; + int previous = 11; + ParallelObjective* s; + while ((s = pbs.next()) != nullptr) { + int value = s->x.val(); + delete s; + if (value >= previous) + return false; + previous = value; + } + if (previous != 0) + return false; + + ExternalObjective* em = new ExternalObjective; + Gecode::Search::Engine* external = + Gecode::Search::build >(em,o); + delete em; + ExternalObjective five(5), equal(5), worse(7), better(3); + int n = ExternalObjective::constraints; + external->constrain(five); + int installed = ExternalObjective::constraints; + external->constrain(equal); + external->constrain(worse); + if ((installed <= n) || + (ExternalObjective::constraints != installed)) { + delete external; + return false; + } + external->constrain(better); + if (ExternalObjective::constraints <= installed) { + delete external; + return false; + } + delete external; + + using namespace Gecode; + Gecode::Search::Options so; + so.threads = 1; + so.cutoff = Gecode::Search::Cutoff::constant(1000000); + SEBs sebs(2); + sebs[0] = bab(so); + sebs[1] = rbs(so); + m = new ParallelObjective; + Gecode::PBS mixed(m,sebs,o); + delete m; + previous = 11; + while ((s = mixed.next()) != nullptr) { + int value = s->x.val(); + delete s; + if (value >= previous) + return false; + previous = value; + } + return (previous == 0) && + failure(FailingParallelObjective::MISSING) && + failure(FailingParallelObjective::THROWN) && + failure(FailingParallelObjective::INCOMPARABLE); + } + }; + + PortfolioComparison portfolio_comparison; + #ifdef GECODE_HAS_FLOAT_VARS /// Stepped float objective for parallel BAB admission class ParallelFloatObjective : public FloatMinimizeSpace { From 360dd6c2336452806a6c826015fc533652f1d57a Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Tue, 22 Sep 2026 15:27:38 +0200 Subject: [PATCH 10/15] Add tasks to comparison Zdev-Change-Id: Z4e4e39fd1665c0addf3e5726ab9bfb5ef3c6d49a7ca7b636d665bb672dccf5c5 --- .zdev/comparison/TASKS.md | 5 ++- ...n-comparison-ownership-tests-and-public.md | 38 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 .zdev/comparison/tasks/006-fix-review-findings-in-comparison-ownership-tests-and-public.md diff --git a/.zdev/comparison/TASKS.md b/.zdev/comparison/TASKS.md index e3077d63ed..6ea64ffd3d 100644 --- a/.zdev/comparison/TASKS.md +++ b/.zdev/comparison/TASKS.md @@ -2,8 +2,8 @@ # Tasks: comparison -- Total: 5 -- Ready: 0 +- Total: 6 +- Ready: 1 - Blocked: 0 - Done: 5 @@ -14,3 +14,4 @@ | [comparison-003](tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md) | Use comparison for sequential BAB and restart incumbent updates | done | comparison-001 | | [comparison-004](tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md) | Use comparison for parallel BAB and safely report comparison failures | done | comparison-002, comparison-003 | | [comparison-005](tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md) | Integrate comparison through portfolios and complete the migration | done | comparison-004 | +| [comparison-006](tasks/006-fix-review-findings-in-comparison-ownership-tests-and-public.md) | Fix review findings in comparison ownership, tests, and public contract | ready | — | diff --git a/.zdev/comparison/tasks/006-fix-review-findings-in-comparison-ownership-tests-and-public.md b/.zdev/comparison/tasks/006-fix-review-findings-in-comparison-ownership-tests-and-public.md new file mode 100644 index 0000000000..8d8f94f397 --- /dev/null +++ b/.zdev/comparison/tasks/006-fix-review-findings-in-comparison-ownership-tests-and-public.md @@ -0,0 +1,38 @@ ++++ +schema_version = 1 +id = "comparison-006" +key = "review-corrections" +area = "comparison" +status = "open" +complexity = "standard" +blocked_by = [] ++++ +# Fix review findings in comparison ownership, tests, and public contract + +## Outcome + +Comparison failures do not leak delivered solutions; focused fixtures exercise valid models; the public API documents ordering, pruning, and recovery requirements. + +## Context + +User approved the five findings of the branch review. Par::BAB::next loses its popped solution if another worker records a failure; Seq::PBS::next loses its candidate when broadcasting throws. PortfolioObjective fixtures inherit a master hook that kills their branchers, and ExternalIncumbent expects a missing comparison from SolveImmediate even though it now overrides compare. The approved brief already contains ordering and nested-cut laws missing from public documentation. Review probes are available under /tmp/gecode-comparison-review.PIGlZy if still present. + +## Boundaries + +- Keep the comparison API, model-defined readiness, float ranking, and initial incomparability rejection policy unchanged. +- Limit changes to the reviewed ownership paths, focused fixtures/regressions, and public contract/recovery documentation. Avoid a general exception framework. + +## Done when + +- [ ] Parallel BAB releases any popped solution before rethrowing a comparison failure, and sequential PBS releases its candidate when incumbent broadcasting throws. +- [ ] Focused regression tests check space lifetime after comparison failure and engine destruction; portfolio models preserve/reinstall branchers and unsupported-comparison tests use a genuinely unsupported model. +- [ ] Public documentation states ordering laws, exact-search compatibility with constrain, nested/equivalent cut requirements, compatible asset semantics, float bound ranking, and model-owned readiness. +- [ ] Document conservative, accurate post-exception use/reset/destruction guarantees without promising unimplemented recovery. +- [ ] Rebuilt focused comparison, external-incumbent, BAB/PBS failure, float and FlatZinc checks pass; run existing PBS coverage and applicable no-thread focused checks. + +## Validation + +- Build gecode-test in build/comparison and build/comparison-no-thread. +- Run focused Search::Comparison, Search::ExternalIncumbent, Search::ParallelBABComparison, Search::PortfolioComparison, Search::FloatObjectiveComparison and FlatZinc objective comparison tests with confirmed matches and process deadlines. +- Run the existing Search::PBS:: matrix at one iteration and focused no-thread comparison/portfolio checks. Validate owned-space cleanup for both reported leaks. +- Run git diff --check and zdev check comparison. From a25ef9135d776d169e12b851304d7acc0906ba80 Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Tue, 22 Sep 2026 15:48:00 +0200 Subject: [PATCH 11/15] Fix comparison failure ownership and clarify the public contract Zdev-Change-Id: Z779d79cd0461e169a13d69dddf214ef27fe9499b996e28b9f44c29b66ff7cb97 --- .zdev/comparison/TASKS.md | 6 +- ...n-comparison-ownership-tests-and-public.md | 23 +++-- gecode/kernel/core.hpp | 25 ++++++ gecode/search.hh | 11 ++- gecode/search/par/bab.hpp | 1 + gecode/search/seq/pbs.hpp | 13 ++- test/search.cpp | 83 ++++++++++++++----- 7 files changed, 124 insertions(+), 38 deletions(-) diff --git a/.zdev/comparison/TASKS.md b/.zdev/comparison/TASKS.md index 6ea64ffd3d..1e1dadac8b 100644 --- a/.zdev/comparison/TASKS.md +++ b/.zdev/comparison/TASKS.md @@ -3,9 +3,9 @@ # Tasks: comparison - Total: 6 -- Ready: 1 +- Ready: 0 - Blocked: 0 -- Done: 5 +- Done: 6 | ID | Task | State | Blocked by | | --- | --- | --- | --- | @@ -14,4 +14,4 @@ | [comparison-003](tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md) | Use comparison for sequential BAB and restart incumbent updates | done | comparison-001 | | [comparison-004](tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md) | Use comparison for parallel BAB and safely report comparison failures | done | comparison-002, comparison-003 | | [comparison-005](tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md) | Integrate comparison through portfolios and complete the migration | done | comparison-004 | -| [comparison-006](tasks/006-fix-review-findings-in-comparison-ownership-tests-and-public.md) | Fix review findings in comparison ownership, tests, and public contract | ready | — | +| [comparison-006](tasks/006-fix-review-findings-in-comparison-ownership-tests-and-public.md) | Fix review findings in comparison ownership, tests, and public contract | done | — | diff --git a/.zdev/comparison/tasks/006-fix-review-findings-in-comparison-ownership-tests-and-public.md b/.zdev/comparison/tasks/006-fix-review-findings-in-comparison-ownership-tests-and-public.md index 8d8f94f397..6b1b7422d3 100644 --- a/.zdev/comparison/tasks/006-fix-review-findings-in-comparison-ownership-tests-and-public.md +++ b/.zdev/comparison/tasks/006-fix-review-findings-in-comparison-ownership-tests-and-public.md @@ -3,7 +3,7 @@ schema_version = 1 id = "comparison-006" key = "review-corrections" area = "comparison" -status = "open" +status = "done" complexity = "standard" blocked_by = [] +++ @@ -24,11 +24,11 @@ User approved the five findings of the branch review. Par::BAB::next loses its p ## Done when -- [ ] Parallel BAB releases any popped solution before rethrowing a comparison failure, and sequential PBS releases its candidate when incumbent broadcasting throws. -- [ ] Focused regression tests check space lifetime after comparison failure and engine destruction; portfolio models preserve/reinstall branchers and unsupported-comparison tests use a genuinely unsupported model. -- [ ] Public documentation states ordering laws, exact-search compatibility with constrain, nested/equivalent cut requirements, compatible asset semantics, float bound ranking, and model-owned readiness. -- [ ] Document conservative, accurate post-exception use/reset/destruction guarantees without promising unimplemented recovery. -- [ ] Rebuilt focused comparison, external-incumbent, BAB/PBS failure, float and FlatZinc checks pass; run existing PBS coverage and applicable no-thread focused checks. +- [x] Parallel BAB releases any popped solution before rethrowing a comparison failure, and sequential PBS releases its candidate when incumbent broadcasting throws. +- [x] Focused regression tests check space lifetime after comparison failure and engine destruction; portfolio models preserve/reinstall branchers and unsupported-comparison tests use a genuinely unsupported model. +- [x] Public documentation states ordering laws, exact-search compatibility with constrain, nested/equivalent cut requirements, compatible asset semantics, float bound ranking, and model-owned readiness. +- [x] Document conservative, accurate post-exception use/reset/destruction guarantees without promising unimplemented recovery. +- [x] Rebuilt focused comparison, external-incumbent, BAB/PBS failure, float and FlatZinc checks pass; run existing PBS coverage and applicable no-thread focused checks. ## Validation @@ -36,3 +36,14 @@ User approved the five findings of the branch review. Par::BAB::next loses its p - Run focused Search::Comparison, Search::ExternalIncumbent, Search::ParallelBABComparison, Search::PortfolioComparison, Search::FloatObjectiveComparison and FlatZinc objective comparison tests with confirmed matches and process deadlines. - Run the existing Search::PBS:: matrix at one iteration and focused no-thread comparison/portfolio checks. Validate owned-space cleanup for both reported leaks. - Run git diff --check and zdev check comparison. + +## Result + +Fixed solution ownership on comparison failures, repaired regression fixtures, and published ordering, pruning, readiness, float-ranking, and recovery contracts; independently verified. + +Validation: + +- Threaded and no-thread gecode-test builds passed; every applicable focused comparison and FlatZinc test matched and passed. +- All 276 Search::PBS:: cases passed at one iteration. +- Sequential PBS and parallel BAB probes each caught 100 failures with zero surviving spaces after destruction. +- Independent verifier passed immutable snapshot W9a4b02f606b0b537; git diff --check and zdev check comparison passed. diff --git a/gecode/kernel/core.hpp b/gecode/kernel/core.hpp index 7e333fa08b..d9aed09c8a 100755 --- a/gecode/kernel/core.hpp +++ b/gecode/kernel/core.hpp @@ -2161,6 +2161,9 @@ namespace Gecode { * * The comparison is read-only and receiver-relative. Implementations may * require objective information sufficient for the relation they report. + * The model owns this readiness rule: comparison does not require every + * objective variable to be assigned when the model can establish a sound + * result from the available information. * The default implementation throws SpaceNoComparison. * SC_INCOMPARABLE is a model-defined ordering result, not missing * information. Current exact best-solution engines reject it. @@ -2169,6 +2172,28 @@ namespace Gecode { * must also override compare. * constrain remains responsible for posting improvement restrictions; * search does not use it to rank two solutions. + * + * Exact best-solution search requires SC_BETTER to be irreflexive and + * transitive, SC_WORSE to be its reverse, and SC_EQUIVALENT to be + * transitive and preserve comparison with every third solution. On + * objective-complete feasible solutions, SC_BETTER must agree with the + * improvement relation posted by constrain. A replacement incumbent's + * improvement restriction must admit a subset of the solutions admitted + * by the old restriction; equivalent incumbents must post interchangeable + * restrictions. + * + * All assets in a best-solution portfolio must use compatible objective + * direction, component interpretation, and pruning policy. The MiniModel + * float optimization spaces compare the incumbent bound used by their + * cuts: the upper bound for minimization and lower bound for maximization. + * This ranking is independent of the step used for approximate cuts. + * FlatZinc retains its interval-valued cuts and instead ranks the lower + * bound for minimization and upper bound for maximization. + * + * After compare throws during BAB, PBS, or RBS search, destroy and + * recreate the public search engine. An internal engine interface that + * explicitly supports reset can establish a new search after failure; + * other post-exception use has no recovery guarantee. */ GECODE_KERNEL_EXPORT virtual SpaceComparison compare(const Space& other) const; diff --git a/gecode/search.hh b/gecode/search.hh index 7508a5a19f..e7ad6e5524 100755 --- a/gecode/search.hh +++ b/gecode/search.hh @@ -1361,12 +1361,14 @@ namespace Gecode { /** * \brief Depth-first branch-and-bound search engine * - * Additionally, \a s must implement a member function - * \code virtual void constrain(const T& t) \endcode + * Additionally, \a s must implement Space::constrain and Space::compare. * Whenever exploration requires to add a constraint * to the space \a c currently being explored, the engine * executes \c c.constrain(t) where \a t is the so-far * best solution. + * See Space::compare for the ordering and pruning requirements. If objective + * comparison throws, destroy the engine; only an internal engine interface + * that explicitly supports reset can establish a new search after failure. * \ingroup TaskModelSearch */ template @@ -1381,12 +1383,13 @@ namespace Gecode { /** * \brief Perform depth-first branch-and-bound search for subclass \a T of space \a s and options \a o * - * Additionally, \a s must implement a member function - * \code virtual void constrain(const T& t) \endcode + * Additionally, \a s must implement Space::constrain and Space::compare. * Whenever exploration requires to add a constraint * to the space \a c currently being explored, the engine * executes \c c.constrain(t) where \a t is the so-far * best solution. + * See Space::compare for the ordering and pruning requirements. If objective + * comparison throws, destroy the engine rather than continuing to use it. * * \ingroup TaskModelSearch */ diff --git a/gecode/search/par/bab.hpp b/gecode/search/par/bab.hpp index 10c43fb536..0876ca7c9a 100755 --- a/gecode/search/par/bab.hpp +++ b/gecode/search/par/bab.hpp @@ -249,6 +249,7 @@ namespace Gecode { namespace Search { namespace Par { // block workers, so finish the normal lifecycle before reporting. if (cmd() != C_WAIT) block(); + delete s; std::rethrow_exception(f); } return s; diff --git a/gecode/search/seq/pbs.hpp b/gecode/search/seq/pbs.hpp index 4dc77efef1..7dc63372b6 100644 --- a/gecode/search/seq/pbs.hpp +++ b/gecode/search/seq/pbs.hpp @@ -110,10 +110,15 @@ namespace Gecode { namespace Search { namespace Seq { if (Space* s = slaves[cur].next()) { // Constrain other slaves if (best) { - for (unsigned int i=0U; i live; enum Failure { MISSING, THROWN, INCOMPARABLE }; IntVar x; Failure failure; FailingParallelObjective(Failure f) : x(*this,0,10), failure(f) { + live++; Gecode::branch(*this,x,INT_VAL_MAX()); } FailingParallelObjective(FailingParallelObjective& s) : Space(s), failure(s.failure) { + live++; x.update(*this,s.x); } + virtual ~FailingParallelObjective(void) { live--; } virtual Space* copy(void) { return new FailingParallelObjective(*this); } @@ -1277,8 +1286,15 @@ namespace Test { throw ComparisonError(); return SC_INCOMPARABLE; } + virtual bool master(const MetaInfo& mi) { + if (mi.type() == MetaInfo::PORTFOLIO) + return false; + return Space::master(mi); + } }; + std::atomic FailingParallelObjective::live(0); + /// Objective with a genuine partial-order result class IncomparableObjective : public ExternalObjective { public: @@ -1290,6 +1306,14 @@ namespace Test { } }; + /// Search model without objective comparison support + class UnsupportedObjective : public Space { + public: + UnsupportedObjective(void) {} + UnsupportedObjective(UnsupportedObjective& s) : Space(s) {} + virtual Space* copy(void) { return new UnsupportedObjective(*this); } + }; + /// Test sequential BAB and RBS external incumbent arbitration class ExternalIncumbent : public Base { private: @@ -1344,9 +1368,12 @@ namespace Test { } catch (const Gecode::Search::Incomparable&) {} delete rejecting; - SolveImmediate unsupported(HTB_NONE,HTB_NONE,HTB_NONE); + UnsupportedObjective unsupported; + UnsupportedObjective* um = + static_cast(unsupported.clone()); Gecode::Search::Engine* missing = Gecode::Search::babengine( - unsupported.clone(),Gecode::Search::Options()); + um,Gecode::Search::Options()); + delete um; missing->constrain(unsupported); try { missing->constrain(unsupported); @@ -1360,6 +1387,7 @@ namespace Test { ExternalIncumbent external_incumbent; +#ifdef GECODE_HAS_THREADS /// Test parallel BAB solution arbitration and failure delivery class ParallelBABComparison : public Base { private: @@ -1382,6 +1410,7 @@ namespace Test { return previous == 0; } static bool missingFailure(void) { + int live = FailingParallelObjective::live; Gecode::Search::TimeStop stop(5000); Gecode::Search::Options o = options(); o.stop = &stop; @@ -1402,9 +1431,10 @@ namespace Test { } bool recovered = resetSearch(e); delete e; - return recovered; + return recovered && (FailingParallelObjective::live == live); } static bool thrownFailure(void) { + int live = FailingParallelObjective::live; Gecode::Search::TimeStop stop(5000); Gecode::Search::Options o = options(); o.stop = &stop; @@ -1417,12 +1447,13 @@ namespace Test { } catch (const ComparisonError&) { bool recovered = resetSearch(e); delete e; - return recovered; + return recovered && (FailingParallelObjective::live == live); } delete e; return false; } static bool incomparableFailure(void) { + int live = FailingParallelObjective::live; Gecode::Search::TimeStop stop(5000); Gecode::Search::Options o = options(); o.stop = &stop; @@ -1435,7 +1466,7 @@ namespace Test { } catch (const Gecode::Search::Incomparable&) { bool recovered = resetSearch(e); delete e; - return recovered; + return recovered && (FailingParallelObjective::live == live); } delete e; return false; @@ -1485,6 +1516,7 @@ namespace Test { }; ParallelBABComparison parallel_bab_comparison; +#endif /// Test portfolio comparison, external bounds, and nested failures class PortfolioComparison : public Base { @@ -1495,23 +1527,29 @@ namespace Test { o.threads = 2; return o; } - static bool failure(FailingParallelObjective::Failure f) { - FailingParallelObjective* m = new FailingParallelObjective(f); - Gecode::PBS pbs(m,options()); - delete m; - try { - while (Space* s = pbs.next()) delete s; - } catch (const SpaceNoComparison&) { - if (f != FailingParallelObjective::MISSING) - return false; - try { (void) pbs.next(); } - catch (const SpaceNoComparison&) { return true; } - } catch (const ComparisonError&) { - return f == FailingParallelObjective::THROWN; - } catch (const Gecode::Search::Incomparable&) { - return f == FailingParallelObjective::INCOMPARABLE; + static bool failure(FailingParallelObjective::Failure f, + double threads=2.0) { + int live = FailingParallelObjective::live; + bool caught = false; + { + FailingParallelObjective* m = new FailingParallelObjective(f); + Gecode::Search::Options o = options(); + o.threads = threads; + Gecode::PBS pbs(m,o); + delete m; + try { + while (Space* s = pbs.next()) delete s; + } catch (const SpaceNoComparison&) { + if (f != FailingParallelObjective::MISSING) + return false; + caught = true; + } catch (const ComparisonError&) { + caught = (f == FailingParallelObjective::THROWN); + } catch (const Gecode::Search::Incomparable&) { + caught = (f == FailingParallelObjective::INCOMPARABLE); + } } - return false; + return caught && (FailingParallelObjective::live == live); } public: PortfolioComparison(void) : Base("Search::PortfolioComparison") {} @@ -1574,6 +1612,7 @@ namespace Test { previous = value; } return (previous == 0) && + failure(FailingParallelObjective::MISSING,1) && failure(FailingParallelObjective::MISSING) && failure(FailingParallelObjective::THROWN) && failure(FailingParallelObjective::INCOMPARABLE); @@ -1707,6 +1746,7 @@ namespace Test { try { (void) m10.compare(x10); return false; } catch (const DynamicCastFailed&) {} +#ifdef GECODE_HAS_THREADS // A bound better by less than the step still replaces the incumbent. Gecode::Search::TimeStop stop(5000); Gecode::Search::Options o; @@ -1729,6 +1769,7 @@ namespace Test { substep = substep && (found == 1) && !bab->stopped(); if (!substep) return false; +#endif return true; } }; From 0fb065b6b31e871124d59a06f9d71640cbe8c9a9 Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Tue, 3 Nov 2026 11:03:13 +0100 Subject: [PATCH 12/15] Fix search tests that relied on BAB enumeration or shared counters Parallel BAB now discards solutions that compare as equivalent, so BAB tests can no longer enumerate all solutions of a model without an objective. The restart and worker-control pause tests now give BAB a lexicographic objective and check for improving solutions ending with the best one. The restart test model tightens its bound on the last solution when it has an objective. The comparison tests counted spaces and posted constraints in static counters shared by tests that the harness runs concurrently. Give each test its own atomic counter instead. --- test/search.cpp | 128 ++++++++++++++++++++++++++++-------------------- 1 file changed, 76 insertions(+), 52 deletions(-) diff --git a/test/search.cpp b/test/search.cpp index 7642fcfa35..38aefa4d11 100644 --- a/test/search.cpp +++ b/test/search.cpp @@ -649,6 +649,9 @@ namespace Test { virtual bool master(const MetaInfo& mi) { switch (mi.type()) { case MetaInfo::RESTART: + // With an objective, restart from a bound improving on the last solution + if (htc != HTC_NONE) + return Space::master(mi); if (mi.last() != nullptr) { const HasSolutions* s = static_cast(mi.last()); @@ -857,7 +860,9 @@ namespace Test { o.threads = threads; o.worker_control = control; o.stop = &pause; - HasSolutions root(HTB_BINARY,HTB_BINARY,HTB_BINARY); + // Best-solution search needs an objective; it then ends with the best + HasSolutions root(HTB_BINARY,HTB_BINARY,HTB_BINARY, + E::best ? HTC_LEX_LE : HTC_NONE); E engine(&root,o); auto result = std::async(std::launch::async,[&] { return engine.next(); }); auto reached = pause.reached.get_future(); @@ -866,21 +871,22 @@ namespace Test { std::future_status::timeout; control.request(1U); int solutions = 0; - if (HasSolutions* solution = result.get()) { + std::unique_ptr last(result.get()); + if (last) ++solutions; - delete solution; - } // Alternate expansion and shrinking at solution boundaries. This - // exercises each request while preserving complete enumeration. + // exercises each request while preserving complete search. while (true) { control.request((solutions % 2) ? threads : 1U); HasSolutions* solution = engine.next(); if (!solution) break; ++solutions; - delete solution; + last.reset(solution); } - return ok && !engine.stopped() && (solutions == root.solutions()); + bool complete = E::best ? + (last && last->best()) : (solutions == root.solutions()); + return ok && !engine.stopped() && complete; } #ifdef GECODE_HAS_EXPERIMENTAL template class E> @@ -1213,25 +1219,26 @@ namespace Test { /// Objective used to test external incumbent updates class ExternalObjective : public IntMinimizeSpace { public: - static int constraints; + /// Per-test count of posted improvement constraints (optional) + std::atomic* constraints; IntVar x; - ExternalObjective(void) : x(*this,0,10) { + ExternalObjective(std::atomic* c) : constraints(c), x(*this,0,10) { Gecode::branch(*this,x,INT_VAL_MIN()); } - ExternalObjective(int v) : x(*this,v,v) {} - ExternalObjective(ExternalObjective& s) : IntMinimizeSpace(s) { + ExternalObjective(int v) : constraints(nullptr), x(*this,v,v) {} + ExternalObjective(ExternalObjective& s) + : IntMinimizeSpace(s), constraints(s.constraints) { x.update(*this,s.x); } virtual Space* copy(void) { return new ExternalObjective(*this); } virtual IntVar cost(void) const { return x; } virtual void constrain(const Space& s) { - constraints++; + if (constraints != nullptr) + constraints->fetch_add(1); IntMinimizeSpace::constrain(s); } }; - int ExternalObjective::constraints = 0; - /// Small objective whose solutions arrive from worst to best class ParallelObjective : public IntMinimizeSpace { public: @@ -1262,16 +1269,18 @@ namespace Test { /// Objective used to exercise asynchronous comparison failures class FailingParallelObjective : public Space { public: - static std::atomic live; enum Failure { MISSING, THROWN, INCOMPARABLE }; + /// Per-test count of live spaces + std::atomic& live; IntVar x; Failure failure; - FailingParallelObjective(Failure f) : x(*this,0,10), failure(f) { + FailingParallelObjective(Failure f, std::atomic& l) + : live(l), x(*this,0,10), failure(f) { live++; Gecode::branch(*this,x,INT_VAL_MAX()); } FailingParallelObjective(FailingParallelObjective& s) - : Space(s), failure(s.failure) { + : Space(s), live(s.live), failure(s.failure) { live++; x.update(*this,s.x); } @@ -1293,7 +1302,6 @@ namespace Test { } }; - std::atomic FailingParallelObjective::live(0); /// Objective with a genuine partial-order result class IncomparableObjective : public ExternalObjective { @@ -1318,45 +1326,46 @@ namespace Test { class ExternalIncumbent : public Base { private: template - static bool updates(Engine& e) { + static bool updates(Engine& e, const std::atomic& constraints) { ExternalObjective five(5), equal(5), worse(7), better(3); - int n = ExternalObjective::constraints; + int n = constraints; e.constrain(five); - int installed = ExternalObjective::constraints; + int installed = constraints; e.constrain(equal); e.constrain(worse); - if ((installed <= n) || (ExternalObjective::constraints != installed)) + if ((installed <= n) || (constraints != installed)) return false; e.constrain(better); - return ExternalObjective::constraints > installed; + return constraints > installed; } public: ExternalIncumbent(void) : Base("Search::ExternalIncumbent") {} virtual bool run(void) { Gecode::Search::Options o; - ExternalObjective* bm = new ExternalObjective; + std::atomic constraints(0); + ExternalObjective* bm = new ExternalObjective(&constraints); Gecode::Search::Engine* bab = Gecode::Search::babengine(bm,o); delete bm; - if (!updates(*bab)) { + if (!updates(*bab,constraints)) { delete bab; return false; } delete bab; o.cutoff = Gecode::Search::Cutoff::constant(10); - ExternalObjective* rm = new ExternalObjective; + ExternalObjective* rm = new ExternalObjective(&constraints); Gecode::Search::Engine* rbs = Gecode::Search::build >(rm,o); delete rm; - if (!updates(*rbs)) { + if (!updates(*rbs,constraints)) { delete rbs; return false; } delete rbs; IncomparableObjective incomparable(4), incumbent(5); - ExternalObjective* im = new ExternalObjective; + ExternalObjective* im = new ExternalObjective(nullptr); Gecode::Search::Engine* rejecting = Gecode::Search::babengine( im,Gecode::Search::Options()); delete im; @@ -1410,12 +1419,12 @@ namespace Test { return previous == 0; } static bool missingFailure(void) { - int live = FailingParallelObjective::live; + std::atomic live(0); Gecode::Search::TimeStop stop(5000); Gecode::Search::Options o = options(); o.stop = &stop; FailingParallelObjective* m = - new FailingParallelObjective(FailingParallelObjective::MISSING); + new FailingParallelObjective(FailingParallelObjective::MISSING,live); Gecode::Search::Engine* e = Gecode::Search::babengine(m,o); delete m; bool repeated = false; @@ -1431,15 +1440,15 @@ namespace Test { } bool recovered = resetSearch(e); delete e; - return recovered && (FailingParallelObjective::live == live); + return recovered && (live == 0); } static bool thrownFailure(void) { - int live = FailingParallelObjective::live; + std::atomic live(0); Gecode::Search::TimeStop stop(5000); Gecode::Search::Options o = options(); o.stop = &stop; FailingParallelObjective* m = - new FailingParallelObjective(FailingParallelObjective::THROWN); + new FailingParallelObjective(FailingParallelObjective::THROWN,live); Gecode::Search::Engine* e = Gecode::Search::babengine(m,o); delete m; try { @@ -1447,18 +1456,18 @@ namespace Test { } catch (const ComparisonError&) { bool recovered = resetSearch(e); delete e; - return recovered && (FailingParallelObjective::live == live); + return recovered && (live == 0); } delete e; return false; } static bool incomparableFailure(void) { - int live = FailingParallelObjective::live; + std::atomic live(0); Gecode::Search::TimeStop stop(5000); Gecode::Search::Options o = options(); o.stop = &stop; FailingParallelObjective* m = new FailingParallelObjective( - FailingParallelObjective::INCOMPARABLE); + FailingParallelObjective::INCOMPARABLE,live); Gecode::Search::Engine* e = Gecode::Search::babengine(m,o); delete m; try { @@ -1466,7 +1475,7 @@ namespace Test { } catch (const Gecode::Search::Incomparable&) { bool recovered = resetSearch(e); delete e; - return recovered && (FailingParallelObjective::live == live); + return recovered && (live == 0); } delete e; return false; @@ -1529,10 +1538,10 @@ namespace Test { } static bool failure(FailingParallelObjective::Failure f, double threads=2.0) { - int live = FailingParallelObjective::live; + std::atomic live(0); bool caught = false; { - FailingParallelObjective* m = new FailingParallelObjective(f); + FailingParallelObjective* m = new FailingParallelObjective(f,live); Gecode::Search::Options o = options(); o.threads = threads; Gecode::PBS pbs(m,o); @@ -1549,7 +1558,7 @@ namespace Test { caught = (f == FailingParallelObjective::INCOMPARABLE); } } - return caught && (FailingParallelObjective::live == live); + return caught && (live == 0); } public: PortfolioComparison(void) : Base("Search::PortfolioComparison") {} @@ -1570,24 +1579,25 @@ namespace Test { if (previous != 0) return false; - ExternalObjective* em = new ExternalObjective; + std::atomic constraints(0); + ExternalObjective* em = new ExternalObjective(&constraints); Gecode::Search::Engine* external = Gecode::Search::build >(em,o); delete em; ExternalObjective five(5), equal(5), worse(7), better(3); - int n = ExternalObjective::constraints; + int n = constraints; external->constrain(five); - int installed = ExternalObjective::constraints; + int installed = constraints; external->constrain(equal); external->constrain(worse); if ((installed <= n) || - (ExternalObjective::constraints != installed)) { + (constraints != installed)) { delete external; return false; } external->constrain(better); - if (ExternalObjective::constraints <= installed) { + if (constraints <= installed) { delete external; return false; } @@ -1978,13 +1988,13 @@ namespace Test { /// Number of threads unsigned int t; public: - /// Initialize test - RBS(const std::string& e, unsigned int t0) + /// Initialize test, enumerating all solutions without an objective + RBS(const std::string& e, unsigned int t0, HowToConstrain htc0=HTC_NONE) : Test("RBS::"+e+"::"+Model::name()+"::"+str(t0), - HTB_BINARY,HTB_BINARY,HTB_BINARY), t(t0) {} + HTB_BINARY,HTB_BINARY,HTB_BINARY,htc0), t(t0) {} /// Run test virtual bool run(void) { - Model* m = new Model(htb1,htb2,htb3); + Model* m = new Model(htb1,htb2,htb3,htc); Gecode::Search::FailStop f(2); Gecode::Search::Options o; o.threads = t; @@ -1994,16 +2004,30 @@ namespace Test { Gecode::RBS rbs(m,o); int n = m->solutions(); delete m; + // With an objective, every solution must improve on the previous one + Model* b = nullptr; + bool improving = true; while (true) { Model* s = rbs.next(); if (s != nullptr) { - n--; delete s; + n--; + if (htc == HTC_NONE) { + delete s; + } else { + if ((b != nullptr) && (s->compare(*b) != SC_BETTER)) + improving = false; + delete b; b = s; + } } if ((s == nullptr) && !rbs.stopped()) break; f.limit(f.limit()+2); } - return n == 0; + if (htc == HTC_NONE) + return n == 0; + bool ok = improving && (b != nullptr) && b->best(); + delete b; + return ok; } }; @@ -2269,7 +2293,7 @@ namespace Test { for (unsigned int t=1; t<=4; t++) { (void) new RBS("DFS",t); (void) new RBS("LDS",t); - (void) new RBS("BAB",t); + (void) new RBS("BAB",t,HTC_LEX_LE); (void) new RBS("DFS",t); (void) new RBS("LDS",t); (void) new RBS("BAB",t); From 550aa13fc503c184507b108ce4b5be459cf9f9f5 Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Tue, 3 Nov 2026 11:20:46 +0100 Subject: [PATCH 13/15] Rank A* and CABS solutions with comparison A* and CABS chose between solutions by their guidance bound, unlike the other best-solution engines and the portfolios they can run in. They now replace the incumbent only when Space::compare reports SC_BETTER, both for their own solutions and for external incumbents, and report incomparable and missing comparisons like BAB. Parallel workers pass solutions that tie with the incumbent bound on to comparison. The bound still orders and prunes open nodes. --- changelog.in | 6 ++++-- gecode/search.hh | 12 +++++++---- gecode/search/astar.cpp | 9 ++++---- gecode/search/cabs.cpp | 9 ++++---- gecode/search/heuristic.hh | 12 ++++++++++- gecode/search/par/frontier.cpp | 13 ++++++------ gecode/search/par/frontier.hh | 2 +- test/search.cpp | 39 ++++++++++++++++++++++++---------- 8 files changed, 66 insertions(+), 36 deletions(-) diff --git a/changelog.in b/changelog.in index 66d38ea2b7..fd843eb304 100755 --- a/changelog.in +++ b/changelog.in @@ -82,8 +82,10 @@ Rank: major [DESCRIPTION] Best-solution search now uses Space::compare() when choosing between independently produced solutions, including external incumbents and portfolio -assets. Optimization spaces with a custom objective must override compare() as -well as constrain(). compare() ranks solutions without changing them; +assets. The experimental A* and CABS engines also rank solutions with +compare(), while their guidance bound still orders and prunes open nodes. +Optimization spaces with a custom objective must override compare() as well as +constrain(). compare() ranks solutions without changing them; constrain() still posts improvement restrictions. Comparability is defined by the model. The built-in floating-point models compare their documented objective bounds, independently of the improvement step. Current exact engines diff --git a/gecode/search.hh b/gecode/search.hh index e7ad6e5524..4c65967f57 100755 --- a/gecode/search.hh +++ b/gecode/search.hh @@ -1236,8 +1236,10 @@ namespace Gecode { namespace Search { * * The bound is an optimistic objective value for a stable space: a lower * bound when minimizing, an upper bound when maximizing, and the exact - * objective for a solution. It must agree with the model's constrain - * function. The optional rank orders nodes (lower first) but is never + * objective for a solution. It must agree with the model's constrain and + * compare functions. The bound orders and prunes open nodes, while + * solutions replace the incumbent only when Space::compare reports + * SC_BETTER. The optional rank orders nodes (lower first) but is never * used for pruning. * * The optional dominance predicate returns whether every completion of @@ -1287,7 +1289,8 @@ namespace Gecode { * Expands nodes in order of their bound, then their rank, using the * guidance from Search::Options::guidance. Like BAB, next() returns * improving solutions and exhausting the search proves the last - * solution optimal. + * solution optimal. Additionally, \a T must implement Space::constrain + * and Space::compare. * * All open nodes are stored as clones, so c_d and a_d are ignored. * Tracing is not supported. @@ -1325,7 +1328,8 @@ namespace Gecode { * improve the incumbent, the search restarts with twice the width. * The guidance is taken from Search::Options::guidance. Like BAB, * next() returns improving solutions and exhausting the search proves - * the last solution optimal. + * the last solution optimal. Additionally, \a T must implement + * Space::constrain and Space::compare. * * Search stops when the width would exceed Search::Options::max_width or * the stored nodes would exceed Search::Options::frontier_limit. Nodes are diff --git a/gecode/search/astar.cpp b/gecode/search/astar.cpp index 9ed5eeab81..3931e17fc1 100644 --- a/gecode/search/astar.cpp +++ b/gecode/search/astar.cpp @@ -142,9 +142,9 @@ namespace Gecode { namespace Search { namespace Seq { AStar::publish(std::unique_ptr s) { // Delete exhausted branchers before cloning the solution (void) s->choice(); - std::int64_t b = guidance.bound(*s); - if (!best.improves(b)) + if (!best.replaced_by(*s,"AStar::next")) return nullptr; + std::int64_t b = guidance.bound(*s); Space* copy = s->clone(); best.update(s.release(),b); return copy; @@ -214,9 +214,8 @@ namespace Gecode { namespace Search { namespace Seq { void AStar::constrain(const Space& s) { - std::int64_t b = guidance.bound(s); - if (best.improves(b)) - best.update(s.clone(),b); + if (best.replaced_by(s,"AStar::constrain")) + best.update(s.clone(),guidance.bound(s)); } void diff --git a/gecode/search/cabs.cpp b/gecode/search/cabs.cpp index 5616a2e202..233cd489b7 100644 --- a/gecode/search/cabs.cpp +++ b/gecode/search/cabs.cpp @@ -210,9 +210,9 @@ namespace Gecode { namespace Search { namespace Seq { CABS::publish(std::unique_ptr s) { // Delete exhausted branchers before cloning the solution (void) s->choice(); - std::int64_t b = guidance.bound(*s); - if (!best.improves(b)) + if (!best.replaced_by(*s,"CABS::next")) return nullptr; + std::int64_t b = guidance.bound(*s); Space* copy = s->clone(); best.update(s.release(),b); return copy; @@ -316,9 +316,8 @@ namespace Gecode { namespace Search { namespace Seq { void CABS::constrain(const Space& s) { - std::int64_t b = guidance.bound(s); - if (best.improves(b)) - best.update(s.clone(),b); + if (best.replaced_by(s,"CABS::constrain")) + best.update(s.clone(),guidance.bound(s)); } void diff --git a/gecode/search/heuristic.hh b/gecode/search/heuristic.hh index 87fde7c609..7d114f2b4d 100644 --- a/gecode/search/heuristic.hh +++ b/gecode/search/heuristic.hh @@ -35,6 +35,7 @@ #define GECODE_SEARCH_HEURISTIC_HH #include +#include #include #include #include @@ -83,7 +84,12 @@ namespace Gecode { namespace Search { } }; - /// Best solution found so far + /** + * \brief Best solution found so far + * + * Solutions are ranked with Space::compare, like in the other best-solution + * engines. The bound of the incumbent prunes open nodes. + */ class Incumbent { public: /// Objective direction @@ -101,6 +107,10 @@ namespace Gecode { namespace Search { bool improves(std::int64_t b) const { return !space || better(sense,b,bound); } + /// Whether solution \a s should replace the incumbent (location \a l) + bool replaced_by(const Space& s, const char* l) const { + return !space || Search::better(s,*space,l); + } /// Make \a s with objective value \a b the incumbent void update(Space* s, std::int64_t b) { space.reset(s); bound = b; revision++; diff --git a/gecode/search/par/frontier.cpp b/gecode/search/par/frontier.cpp index dacb958871..5977f8827f 100644 --- a/gecode/search/par/frontier.cpp +++ b/gecode/search/par/frontier.cpp @@ -155,12 +155,10 @@ namespace Gecode { namespace Search { namespace Par { void Frontier::publish(Space* s, std::int64_t b) { - if (!best.improves(b)) { - delete s; - return; - } std::unique_ptr incumbent(s); - solution.reset(s->clone()); + if (!best.replaced_by(*incumbent,"Frontier::publish")) + return; + solution.reset(incumbent->clone()); best.update(incumbent.release(),b); running = false; } @@ -281,7 +279,8 @@ namespace Gecode { namespace Search { namespace Par { (void) step.space->choice(); std::int64_t b = guidance.bound(*step.space); if (step.status == SS_SOLVED) { - if (!cutoff || better(guidance.sense,b,*cutoff)) { + // Comparison decides solutions that tie with the incumbent bound + if (!cutoff || !better(guidance.sense,*cutoff,b)) { solved = std::move(step.space); solved_bound = b; break; @@ -375,7 +374,7 @@ namespace Gecode { namespace Search { namespace Par { Frontier::constrain(const Space& s) { std::int64_t b = guidance.bound(s); Support::Lock lock(mutex); - if (best.improves(b)) { + if (best.replaced_by(s,"Frontier::constrain")) { best.update(s.clone(),b); solution.reset(); } diff --git a/gecode/search/par/frontier.hh b/gecode/search/par/frontier.hh index a3825097c9..23a5075a5a 100644 --- a/gecode/search/par/frontier.hh +++ b/gecode/search/par/frontier.hh @@ -143,7 +143,7 @@ namespace Gecode { namespace Search { namespace Par { void push(Task* t); /// Stop all workers (must be called by subclass destructors) void terminate(void); - /// Make \a s with bound \a b the incumbent, if it improves + /// Make \a s with bound \a b the incumbent, if it compares better void publish(Space* s, std::int64_t b); /// Start the threads void launch(void); diff --git a/test/search.cpp b/test/search.cpp index 38aefa4d11..033eaebbb7 100644 --- a/test/search.cpp +++ b/test/search.cpp @@ -1338,6 +1338,18 @@ namespace Test { e.constrain(better); return constraints > installed; } + /// Whether engine \a e reports an incomparable incumbent (deletes \a e) + static bool rejects_incomparable(Gecode::Search::Engine* e) { + std::unique_ptr engine(e); + IncomparableObjective incomparable(4), incumbent(5); + engine->constrain(incumbent); + try { + engine->constrain(incomparable); + } catch (const Gecode::Search::Incomparable&) { + return true; + } + return false; + } public: ExternalIncumbent(void) : Base("Search::ExternalIncumbent") {} virtual bool run(void) { @@ -1364,18 +1376,23 @@ namespace Test { } delete rbs; - IncomparableObjective incomparable(4), incumbent(5); - ExternalObjective* im = new ExternalObjective(nullptr); - Gecode::Search::Engine* rejecting = Gecode::Search::babengine( - im,Gecode::Search::Options()); - delete im; - rejecting->constrain(incumbent); - try { - rejecting->constrain(incomparable); - delete rejecting; + ExternalObjective im(nullptr); + if (!rejects_incomparable(Gecode::Search::babengine(&im,Gecode::Search::Options()))) return false; - } catch (const Gecode::Search::Incomparable&) {} - delete rejecting; +#ifdef GECODE_HAS_EXPERIMENTAL + Gecode::Search::HeuristicGuidance g(Gecode::Search::HeuristicGuidance::MINIMIZE, + [](const Space& s) -> std::int64_t { + return static_cast(s).x.min(); + }); + for (double threads : {1.0,2.0}) { + Gecode::Search::Options ho; + ho.threads = threads; + ho.guidance = &g; + if (!rejects_incomparable(Gecode::Search::build(&im,ho)) || + !rejects_incomparable(Gecode::Search::build(&im,ho))) + return false; + } +#endif UnsupportedObjective unsupported; UnsupportedObjective* um = From 0f02b6d9697dc3cc37cfcb240c218eee6efc7019 Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Tue, 3 Nov 2026 17:15:59 +0100 Subject: [PATCH 14/15] Shorten the comparison documentation Space::compare now says what compare returns, how it must agree with constrain, and what the default does, at about the length of the constrain documentation. Model-specific ranking stays with the MiniModel and FlatZinc classes. The BAB and PBS notes and the changelog entry drop the repeated and speculative parts. --- changelog.in | 20 ++++++------------ gecode/kernel/core.hpp | 48 ++++++++++-------------------------------- gecode/search.hh | 11 ++-------- 3 files changed, 20 insertions(+), 59 deletions(-) diff --git a/changelog.in b/changelog.in index fd843eb304..71072d93ee 100755 --- a/changelog.in +++ b/changelog.in @@ -80,19 +80,13 @@ Module: search What: change Rank: major [DESCRIPTION] -Best-solution search now uses Space::compare() when choosing between -independently produced solutions, including external incumbents and portfolio -assets. The experimental A* and CABS engines also rank solutions with -compare(), while their guidance bound still orders and prunes open nodes. -Optimization spaces with a custom objective must override compare() as well as -constrain(). compare() ranks solutions without changing them; -constrain() still posts improvement restrictions. Comparability is defined by -the model. The built-in floating-point models compare their documented -objective bounds, independently of the improvement step. Current exact engines -reject incomparable incoming solutions. A future best-effort policy could -instead make an incomparable incoming solution current best, without changing -the comparison interface, but that would not by itself guarantee completeness -or maximality. +Add Space::compare(), which ranks two solutions without modifying them. +Best-solution search uses it to choose between solutions, for example in +parallel BAB, portfolios, external incumbents, and the experimental A* and +CABS engines. constrain() still prunes the search. Optimization models with a +custom constrain() must now also override compare(). Search +engines throw Search::Incomparable when compare() reports incomparable +solutions. [RELEASE] Version: 6.5.0 diff --git a/gecode/kernel/core.hpp b/gecode/kernel/core.hpp index d9aed09c8a..a65df466f7 100755 --- a/gecode/kernel/core.hpp +++ b/gecode/kernel/core.hpp @@ -2157,43 +2157,17 @@ namespace Gecode { */ GECODE_KERNEL_EXPORT virtual void constrain(const Space& best); /** - * \brief Compare this space's objective with \a other - * - * The comparison is read-only and receiver-relative. Implementations may - * require objective information sufficient for the relation they report. - * The model owns this readiness rule: comparison does not require every - * objective variable to be assigned when the model can establish a sound - * result from the available information. - * The default implementation throws SpaceNoComparison. - * SC_INCOMPARABLE is a model-defined ordering result, not missing - * information. Current exact best-solution engines reject it. - * - * Optimization models overriding constrain with a different objective - * must also override compare. - * constrain remains responsible for posting improvement restrictions; - * search does not use it to rank two solutions. - * - * Exact best-solution search requires SC_BETTER to be irreflexive and - * transitive, SC_WORSE to be its reverse, and SC_EQUIVALENT to be - * transitive and preserve comparison with every third solution. On - * objective-complete feasible solutions, SC_BETTER must agree with the - * improvement relation posted by constrain. A replacement incumbent's - * improvement restriction must admit a subset of the solutions admitted - * by the old restriction; equivalent incumbents must post interchangeable - * restrictions. - * - * All assets in a best-solution portfolio must use compatible objective - * direction, component interpretation, and pruning policy. The MiniModel - * float optimization spaces compare the incumbent bound used by their - * cuts: the upper bound for minimization and lower bound for maximization. - * This ranking is independent of the step used for approximate cuts. - * FlatZinc retains its interval-valued cuts and instead ranks the lower - * bound for minimization and upper bound for maximization. - * - * After compare throws during BAB, PBS, or RBS search, destroy and - * recreate the public search engine. An internal engine interface that - * explicitly supports reset can establish a new search after failure; - * other post-exception use has no recovery guarantee. + * \brief Compare function for best solution search + * + * Must return whether this solution is better than, equivalent to, + * worse than, or incomparable with solution \a other, without modifying + * either. The result must agree with constrain: solutions admitted by + * constrain(b) are better than b, and a better b admits no more solutions. + * + * The default function throws SpaceNoComparison, so a space overriding + * constrain must also override compare. + * + * \ingroup TaskModelScript */ GECODE_KERNEL_EXPORT virtual SpaceComparison compare(const Space& other) const; diff --git a/gecode/search.hh b/gecode/search.hh index 4c65967f57..1b85153c10 100755 --- a/gecode/search.hh +++ b/gecode/search.hh @@ -1370,9 +1370,6 @@ namespace Gecode { * to the space \a c currently being explored, the engine * executes \c c.constrain(t) where \a t is the so-far * best solution. - * See Space::compare for the ordering and pruning requirements. If objective - * comparison throws, destroy the engine; only an internal engine interface - * that explicitly supports reset can establish a new search after failure. * \ingroup TaskModelSearch */ template @@ -1392,8 +1389,6 @@ namespace Gecode { * to the space \a c currently being explored, the engine * executes \c c.constrain(t) where \a t is the so-far * best solution. - * See Space::compare for the ordering and pruning requirements. If objective - * comparison throws, destroy the engine rather than continuing to use it. * * \ingroup TaskModelSearch */ @@ -1531,10 +1526,8 @@ namespace Gecode { * The engine will run a portfolio with a number of assets as defined * by the options \a o. The engine supports parallel execution of * assets by using the number of threads as defined by the options. - * For best-solution assets, incoming solutions replace the retained - * incumbent only when Space::compare reports SC_BETTER. Equivalent and - * worse solutions are discarded; incomparable solutions are currently - * reported as unsupported. + * For best-solution assets, a solution replaces the incumbent only if + * Space::compare reports SC_BETTER. * An external worker control in \a o is supported only for a single * homogeneous asset. Multiple controlled assets require explicit engine * builders, each with its own control and immutable worker capacity. From 2fdfdc901158057c6b84f2cffbb3d46bc6053403 Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Tue, 3 Nov 2026 17:15:59 +0100 Subject: [PATCH 15/15] Add Sense and compare_int for objective comparison Implementing Space::compare for an integer objective took four lines of value comparisons, against a single rel call in constrain. compare_int compares two values for a Sense, an optimization direction now shared by the kernel, MiniModel, FlatZinc, and the experimental heuristic guidance, which had its own MINIMIZE/MAXIMIZE enum. --- changelog.in | 3 ++- examples/car-sequencing.cpp | 9 ++------- examples/fifteen-puzzle.cpp | 2 +- examples/tsptw.cpp | 2 +- gecode/flatzinc/flatzinc.cpp | 6 ++---- gecode/kernel/core.hpp | 14 ++++++++++++++ gecode/minimodel/optimize.cpp | 15 ++++----------- gecode/search.hh | 2 -- gecode/search/heuristic.hh | 10 +++++----- test/search.cpp | 12 +++++++++--- 10 files changed, 40 insertions(+), 35 deletions(-) diff --git a/changelog.in b/changelog.in index 71072d93ee..95d053c34d 100755 --- a/changelog.in +++ b/changelog.in @@ -84,7 +84,8 @@ Add Space::compare(), which ranks two solutions without modifying them. Best-solution search uses it to choose between solutions, for example in parallel BAB, portfolios, external incumbents, and the experimental A* and CABS engines. constrain() still prunes the search. Optimization models with a -custom constrain() must now also override compare(). Search +custom constrain() must now also override compare(). compare_int() +compares two integer objective values for a given Sense. Search engines throw Search::Incomparable when compare() reports incomparable solutions. diff --git a/examples/car-sequencing.cpp b/examples/car-sequencing.cpp index 9addcda22c..c71ae85354 100644 --- a/examples/car-sequencing.cpp +++ b/examples/car-sequencing.cpp @@ -371,13 +371,8 @@ class CarSequencing : public Script { /// Compare objective values virtual SpaceComparison compare(const Space& _other) const { - const CarSequencing& other = - dynamic_cast(_other); - if (nstall.val() < other.nstall.val()) - return SC_BETTER; - if (nstall.val() > other.nstall.val()) - return SC_WORSE; - return SC_EQUIVALENT; + const CarSequencing& other = static_cast(_other); + return compare_int(nstall.val(), other.nstall.val(), Sense::minimize); } /// Print solution diff --git a/examples/fifteen-puzzle.cpp b/examples/fifteen-puzzle.cpp index 16ef255fb2..f6325be4ce 100644 --- a/examples/fifteen-puzzle.cpp +++ b/examples/fifteen-puzzle.cpp @@ -418,7 +418,7 @@ run(PuzzleOptions& opt) { case PuzzleOptions::SEARCH_ASTAR: case PuzzleOptions::SEARCH_CABS: { - Search::HeuristicGuidance guidance(Search::HeuristicGuidance::MINIMIZE, + Search::HeuristicGuidance guidance(Sense::minimize, [](const Space& home) -> std::int64_t { return static_cast(home).bound(); }); diff --git a/examples/tsptw.cpp b/examples/tsptw.cpp index 2b83ad474b..f40d635388 100644 --- a/examples/tsptw.cpp +++ b/examples/tsptw.cpp @@ -381,7 +381,7 @@ main(int argc, char* argv[]) { case TSPTW::SEARCH_ASTAR: case TSPTW::SEARCH_CABS: { - Search::HeuristicGuidance g(Search::HeuristicGuidance::MINIMIZE, + Search::HeuristicGuidance g(Sense::minimize, [](const Space& s) -> std::int64_t { return static_cast(s).cost().min(); }); diff --git a/gecode/flatzinc/flatzinc.cpp b/gecode/flatzinc/flatzinc.cpp index a7c08401b0..9edaf5a66b 100644 --- a/gecode/flatzinc/flatzinc.cpp +++ b/gecode/flatzinc/flatzinc.cpp @@ -2092,10 +2092,8 @@ namespace Gecode { namespace FlatZinc { ((_method != MIN) && (_method != MAX))) throw DynamicCastFailed("FlatZincSpace::compare"); if (_optVarIsInt) { - int a=iv[_optVar].val(), b=other->iv[other->_optVar].val(); - if (a == b) - return SC_EQUIVALENT; - return ((a < b) == (_method == MIN)) ? SC_BETTER : SC_WORSE; + return compare_int(iv[_optVar].val(),other->iv[other->_optVar].val(), + (_method == MIN) ? Sense::minimize : Sense::maximize); } #ifdef GECODE_HAS_FLOAT_VARS if (step != other->step) diff --git a/gecode/kernel/core.hpp b/gecode/kernel/core.hpp index a65df466f7..78516ce60f 100755 --- a/gecode/kernel/core.hpp +++ b/gecode/kernel/core.hpp @@ -61,6 +61,20 @@ namespace Gecode { SC_INCOMPARABLE ///< Neither objective dominates the other }; + /// Optimization direction + enum class Sense { + minimize, ///< Smaller objective values are better + maximize ///< Larger objective values are better + }; + + /// Compare objective value \a x with \a y for direction \a s + forceinline SpaceComparison + compare_int(int x, int y, Sense s) { + if (x == y) + return SC_EQUIVALENT; + return ((x < y) == (s == Sense::minimize)) ? SC_BETTER : SC_WORSE; + } + class Space; /** diff --git a/gecode/minimodel/optimize.cpp b/gecode/minimodel/optimize.cpp index 0e5253e5c3..2f585c91bf 100755 --- a/gecode/minimodel/optimize.cpp +++ b/gecode/minimodel/optimize.cpp @@ -35,13 +35,6 @@ namespace Gecode { - static SpaceComparison - compare_int(int x, int y, bool minimize) { - if (x == y) - return SC_EQUIVALENT; - return ((x < y) == minimize) ? SC_BETTER : SC_WORSE; - } - void IntMinimizeSpace::constrain(const Space& _best) { const IntMinimizeSpace* best = @@ -56,7 +49,7 @@ namespace Gecode { const IntMinimizeSpace* s = dynamic_cast(&other); if (s == nullptr) throw DynamicCastFailed("IntMinimizeSpace::compare"); - return compare_int(cost().val(),s->cost().val(),true); + return compare_int(cost().val(),s->cost().val(),Sense::minimize); } @@ -74,7 +67,7 @@ namespace Gecode { const IntMaximizeSpace* s = dynamic_cast(&other); if (s == nullptr) throw DynamicCastFailed("IntMaximizeSpace::compare"); - return compare_int(cost().val(),s->cost().val(),false); + return compare_int(cost().val(),s->cost().val(),Sense::maximize); } @@ -103,7 +96,7 @@ namespace Gecode { for (int i=0; i Evaluate; /// Objective direction diff --git a/gecode/search/heuristic.hh b/gecode/search/heuristic.hh index 7d114f2b4d..6502547e51 100644 --- a/gecode/search/heuristic.hh +++ b/gecode/search/heuristic.hh @@ -46,8 +46,8 @@ namespace Gecode { namespace Search { /// Whether bound \a a is better than bound \a b for objective direction \a s forceinline bool - better(HeuristicGuidance::Sense s, std::int64_t a, std::int64_t b) { - return (s == HeuristicGuidance::MINIMIZE) ? (a < b) : (a > b); + better(Sense s, std::int64_t a, std::int64_t b) { + return (s == Sense::minimize) ? (a < b) : (a > b); } /// Open node of a heuristic search engine @@ -71,7 +71,7 @@ namespace Gecode { namespace Search { class Priority { public: /// Objective direction - HeuristicGuidance::Sense sense; + Sense sense; /// Whether to order by rank only (CABS with rank) bool rank_only; /// Test whether \a a is preferred to \a b @@ -93,7 +93,7 @@ namespace Gecode { namespace Search { class Incumbent { public: /// Objective direction - HeuristicGuidance::Sense sense; + Sense sense; /// The solution (if any) std::unique_ptr space; /// Its objective value @@ -101,7 +101,7 @@ namespace Gecode { namespace Search { /// Incremented for every new incumbent unsigned long long int revision; /// Initialize without incumbent - Incumbent(HeuristicGuidance::Sense s) + Incumbent(Sense s) : sense(s), bound(0), revision(0) {} /// Whether a node with bound \a b can improve the incumbent bool improves(std::int64_t b) const { diff --git a/test/search.cpp b/test/search.cpp index 033eaebbb7..3be463df47 100644 --- a/test/search.cpp +++ b/test/search.cpp @@ -162,8 +162,7 @@ namespace Test { static Gecode::Search::HeuristicGuidance guidance(bool max, bool ranked, bool dominance, bool keyed) { using Gecode::Search::HeuristicGuidance; - HeuristicGuidance g(max ? HeuristicGuidance::MAXIMIZE : - HeuristicGuidance::MINIMIZE, + HeuristicGuidance g(max ? Sense::maximize : Sense::minimize, [](const Space& s) -> std::int64_t { const TreeSpace& t = static_cast(s); return t.maximize ? -t.node().bound : t.node().bound; @@ -1380,7 +1379,7 @@ namespace Test { if (!rejects_incomparable(Gecode::Search::babengine(&im,Gecode::Search::Options()))) return false; #ifdef GECODE_HAS_EXPERIMENTAL - Gecode::Search::HeuristicGuidance g(Gecode::Search::HeuristicGuidance::MINIMIZE, + Gecode::Search::HeuristicGuidance g(Gecode::Sense::minimize, [](const Space& s) -> std::int64_t { return static_cast(s).x.min(); }); @@ -1841,6 +1840,13 @@ namespace Test { if ((high.compare(low) != SC_BETTER) || (low.compare(high) != SC_WORSE)) return false; + if ((compare_int(1,2,Sense::minimize) != SC_BETTER) || + (compare_int(2,1,Sense::minimize) != SC_WORSE) || + (compare_int(1,1,Sense::minimize) != SC_EQUIVALENT) || + (compare_int(2,1,Sense::maximize) != SC_BETTER) || + (compare_int(1,2,Sense::maximize) != SC_WORSE) || + (compare_int(1,1,Sense::maximize) != SC_EQUIVALENT)) + return false; LexObjective lp(1,0,2), lq(2,0,2); if ((lp.compare(lq) != SC_BETTER) || (lq.compare(lp) != SC_WORSE))