From 5a284b74bc6d17f96dee39243c9c61b1fca9bf55 Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Thu, 10 Sep 2026 08:22:34 +0200 Subject: [PATCH 1/9] 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 f7bf2c4a5b7b07e589b847079542ccad12292b4f Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Thu, 10 Sep 2026 09:00:04 +0200 Subject: [PATCH 2/9] 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 679d9c824d1a84c74a969aace18f35e28dc9640e Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Thu, 10 Sep 2026 09:41:52 +0200 Subject: [PATCH 3/9] 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 2016acc88dd32a6c25becc93248a461ca96d2a4c Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Thu, 10 Sep 2026 10:05:12 +0200 Subject: [PATCH 4/9] 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 0eb45fb86dd4b526075cb04d751d46c229e9baa9 Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Thu, 10 Sep 2026 10:25:44 +0200 Subject: [PATCH 5/9] 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 | 25 ++++ 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, 294 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 409968d979..d5b2f540f7 100755 --- a/gecode/driver.hh +++ b/gecode/driver.hh @@ -776,6 +776,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 f4d934d3d1..c3743b7b9c 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 e35bd3638c..29aeb990ae 100644 --- a/gecode/flatzinc/flatzinc.cpp +++ b/gecode/flatzinc/flatzinc.cpp @@ -2076,6 +2076,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 4293cfe6a8..b38660acd1 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 169aab6599..cfe83fe05a 100755 --- a/gecode/kernel/core.hpp +++ b/gecode/kernel/core.hpp @@ -53,6 +53,19 @@ namespace Gecode { + /** + * \brief Result of comparing two spaces according to their objectives + * + * Adding Space::compare changes the ordinary C++ ABI of Space by extending + * its virtual function table. + */ + 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; /** @@ -2086,6 +2099,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 5b7bc145d0..f36c5b89ed 100755 --- a/gecode/minimodel.hh +++ b/gecode/minimodel.hh @@ -2424,6 +2424,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; }; @@ -2441,6 +2444,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; }; @@ -2458,6 +2464,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; }; @@ -2475,6 +2484,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 48e0bdfdcb..aa07ba893e 100644 --- a/test/search.cpp +++ b/test/search.cpp @@ -381,6 +381,112 @@ namespace Test { htb1(_htb1), htb2(_htb2), htb3(_htb3), htc(_htc) {} }; + /// 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 { @@ -733,6 +839,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 40d4573b1d634fe9abfcc947ee48035c7f38226a Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Thu, 10 Sep 2026 11:54:45 +0200 Subject: [PATCH 6/9] 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 c3743b7b9c..91e9c5ecef 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 29aeb990ae..8de5b84014 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(); } @@ -2081,14 +2085,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 f36c5b89ed..814585da2d 100755 --- a/gecode/minimodel.hh +++ b/gecode/minimodel.hh @@ -2499,6 +2499,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 */ @@ -2514,6 +2517,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; }; @@ -2524,6 +2530,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 */ @@ -2539,6 +2548,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 e404f691ce..cd078538f0 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 aa07ba893e..585eaa8c2a 100644 --- a/test/search.cpp +++ b/test/search.cpp @@ -41,6 +41,7 @@ #include "test/test.hh" #include +#include static_assert(std::is_copy_constructible::value, "NoGoods must remain copy constructible"); @@ -405,6 +406,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: @@ -420,6 +449,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 3f4977e6ed50134b0b9e76041ebb6511970dfe1a Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Thu, 10 Sep 2026 12:11:39 +0200 Subject: [PATCH 7/9] 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 5356b24303..94d5f39d0c 100755 --- a/changelog.in +++ b/changelog.in @@ -73,6 +73,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.4.0 Date: 2026-07-15 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 eb95353ad8..794f24ffd8 100644 --- a/gecode/search/exception.cpp +++ b/gecode/search/exception.cpp @@ -48,6 +48,9 @@ namespace Gecode { namespace Search { NoBest::NoBest(const char* l) : Exception(l,"Best solution search is not supported") {} + 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 01ff8cbd50..861fda8828 100644 --- a/gecode/search/exception.hpp +++ b/gecode/search/exception.hpp @@ -62,6 +62,12 @@ namespace Gecode { namespace Search { /// Initialize with location \a l NoBest(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); + }; //@} }} diff --git a/gecode/search/seq/bab.hpp b/gecode/search/seq/bab.hpp index 3138fd371e..c5b296a41d 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 1d0fc78172..b15e368c37 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 585eaa8c2a..6feddda70e 100644 --- a/test/search.cpp +++ b/test/search.cpp @@ -269,6 +269,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) { @@ -406,6 +440,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 5e7ecef45cfb328e1e9b92936cfe938277cb2b07 Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Thu, 10 Sep 2026 12:59:29 +0200 Subject: [PATCH 8/9] 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 | 211 ++++++++++++++++++ 5 files changed, 303 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 5284f131cf..a057e4ae24 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; @@ -100,6 +102,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; @@ -114,6 +118,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 8e6d3f047d..5bae67458d 100755 --- a/gecode/search/par/bab.hpp +++ b/gecode/search/par/bab.hpp @@ -83,7 +83,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 @@ -119,17 +119,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(); } @@ -140,7 +156,6 @@ namespace Gecode { namespace Search { namespace Par { solutions.push(s); if (bs) e_search.signal(); - m_search.release(); } @@ -189,20 +204,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; } /* @@ -369,9 +409,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 6feddda70e..399308edcd 100644 --- a/test/search.cpp +++ b/test/search.cpp @@ -37,6 +37,7 @@ #include #include +#include #include "test/test.hh" @@ -462,6 +463,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: @@ -543,7 +591,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,9.5,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: @@ -649,6 +841,25 @@ namespace Test { catch (const DynamicCastFailed&) {} try { (void) m10.compare(x10); return false; } catch (const DynamicCastFailed&) {} + + Gecode::Search::TimeStop stop(5000); + Gecode::Search::Options o; + o.threads = 2; + o.stop = &stop; + Gecode::Search::Par::BAB + bab(nullptr,o); + bab.solution(new ParallelFloatObjective(10.0)); + bab.solution(new ParallelFloatObjective(9.5)); + ParallelFloatObjective* p10 = + static_cast(bab.next()); + ParallelFloatObjective* p95 = + static_cast(bab.next()); + bool substep = (p10 != nullptr) && (p95 != nullptr) && + (p10->x.val().max() == 10.0) && (p95->x.val().max() == 9.5); + delete p10; + delete p95; + if (!substep) + return false; return true; } }; From 87976b234854441fc5d441138c0a29dc1ee9e9bb Mon Sep 17 00:00:00 2001 From: Mikael Zayenz Lagerkvist Date: Thu, 10 Sep 2026 13:21:36 +0200 Subject: [PATCH 9/9] 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 | 18 ++-- gecode/kernel/core.hpp | 4 + gecode/search.hh | 4 + gecode/search/par/pbs.hh | 5 + gecode/search/par/pbs.hpp | 102 ++++++++++++------ test/search.cpp | 100 +++++++++++++++++ 8 files changed, 216 insertions(+), 44 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 94d5f39d0c..3e806156cc 100755 --- a/changelog.in +++ b/changelog.in @@ -78,12 +78,18 @@ 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. Adding the virtual comparison function changes the C++ ABI of +Space. [RELEASE] Version: 6.4.0 diff --git a/gecode/kernel/core.hpp b/gecode/kernel/core.hpp index cfe83fe05a..a3f194587e 100755 --- a/gecode/kernel/core.hpp +++ b/gecode/kernel/core.hpp @@ -2105,9 +2105,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 38668804ad..ebcf53d0ca 100755 --- a/gecode/search.hh +++ b/gecode/search.hh @@ -1265,6 +1265,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. * * The class \a T can implement member functions * \code virtual bool master(const MetaInfo& mi) \endcode diff --git a/gecode/search/par/pbs.hh b/gecode/search/par/pbs.hh index 6bdcf5c82a..6325802301 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 { @@ -180,8 +181,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 1fa593aead..6c1df553db 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; @@ -207,9 +200,16 @@ namespace Gecode { namespace Search { namespace Par { bool b = true; 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); + } catch (...) { + delete s; + if (failure == nullptr) + failure = std::current_exception(); tostop.store(true, std::memory_order_release); + } } else if (slave->stopped()) { if (!tostop.load(std::memory_order_acquire)) slave_stop.store(true, std::memory_order_release); @@ -231,19 +231,40 @@ 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); + 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()) { // Clear all tostop.store(false, std::memory_order_release); @@ -280,10 +301,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(); @@ -312,10 +349,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 399308edcd..d05018b401 100644 --- a/test/search.cpp +++ b/test/search.cpp @@ -177,6 +177,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; @@ -717,6 +721,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 {