diff --git a/.zdev/comparison/TASKS.md b/.zdev/comparison/TASKS.md new file mode 100644 index 0000000000..1e1dadac8b --- /dev/null +++ b/.zdev/comparison/TASKS.md @@ -0,0 +1,17 @@ + + +# Tasks: comparison + +- Total: 6 +- Ready: 0 +- Blocked: 0 +- Done: 6 + +| 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 | done | comparison-002, comparison-003 | +| [comparison-005](tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md) | Integrate comparison through portfolios and complete the migration | done | comparison-004 | +| [comparison-006](tasks/006-fix-review-findings-in-comparison-ownership-tests-and-public.md) | Fix review findings in comparison ownership, tests, and public contract | done | — | 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..8541d9b952 --- /dev/null +++ b/.zdev/comparison/background/comparison-and-search.md @@ -0,0 +1,333 @@ +# 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. +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 +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. + +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. 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, 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. 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, 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 + +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. 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 + +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 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 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. | + +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 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. +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. 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. +- `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..193bbb5b68 --- /dev/null +++ b/.zdev/comparison/brief.md @@ -0,0 +1,326 @@ +# 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. + +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 + +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, 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 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. 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 +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 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. + +Add one virtual member of `Space`, with receiver-relative results: + +```cpp +enum SpaceComparison { + SC_BETTER, + SC_EQUIVALENT, + SC_WORSE, + SC_INCOMPARABLE +}; + +virtual SpaceComparison compare(const Space& other) const; +``` + +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 +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. + +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. + +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. +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 +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 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 +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. + +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 +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. + +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. +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. + +## 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. + +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 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 policy + +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. + +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. + +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. + +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, + 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 + 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. +- 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 + script; a model overriding one overload must not hide the other from Gist. + +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. + +## Validation and completion + +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, +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/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..82598ee382 --- /dev/null +++ b/.zdev/comparison/tasks/001-add-space-comparison-and-integer-objective-implementations.md @@ -0,0 +1,44 @@ ++++ +schema_version = 1 +id = "comparison-001" +key = "integer-comparison" +area = "comparison" +status = "done" +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 + +- [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/.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..6235d3a198 --- /dev/null +++ b/.zdev/comparison/tasks/002-compare-float-quality-independently-of-the-improvement-step.md @@ -0,0 +1,44 @@ ++++ +schema_version = 1 +id = "comparison-002" +key = "float-comparison" +area = "comparison" +status = "done" +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 + +- [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/.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..6619b89a38 --- /dev/null +++ b/.zdev/comparison/tasks/003-use-comparison-for-sequential-bab-and-restart-incumbent-upda.md @@ -0,0 +1,44 @@ ++++ +schema_version = 1 +id = "comparison-003" +key = "sequential-incumbents" +area = "comparison" +status = "done" +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 + +- [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/.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..c7a70d0aaa --- /dev/null +++ b/.zdev/comparison/tasks/004-use-comparison-for-parallel-bab-and-safely-report-comparison.md @@ -0,0 +1,46 @@ ++++ +schema_version = 1 +id = "comparison-004" +key = "parallel-bab" +area = "comparison" +status = "done" +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 + +- [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/.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..056d00a8e9 --- /dev/null +++ b/.zdev/comparison/tasks/005-integrate-comparison-through-portfolios-and-complete-the-mig.md @@ -0,0 +1,46 @@ ++++ +schema_version = 1 +id = "comparison-005" +key = "portfolio-comparison" +area = "comparison" +status = "done" +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 + +- [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/.zdev/comparison/tasks/006-fix-review-findings-in-comparison-ownership-tests-and-public.md b/.zdev/comparison/tasks/006-fix-review-findings-in-comparison-ownership-tests-and-public.md new file mode 100644 index 0000000000..6b1b7422d3 --- /dev/null +++ b/.zdev/comparison/tasks/006-fix-review-findings-in-comparison-ownership-tests-and-public.md @@ -0,0 +1,49 @@ ++++ +schema_version = 1 +id = "comparison-006" +key = "review-corrections" +area = "comparison" +status = "done" +complexity = "standard" +blocked_by = [] ++++ +# Fix review findings in comparison ownership, tests, and public contract + +## Outcome + +Comparison failures do not leak delivered solutions; focused fixtures exercise valid models; the public API documents ordering, pruning, and recovery requirements. + +## Context + +User approved the five findings of the branch review. Par::BAB::next loses its popped solution if another worker records a failure; Seq::PBS::next loses its candidate when broadcasting throws. PortfolioObjective fixtures inherit a master hook that kills their branchers, and ExternalIncumbent expects a missing comparison from SolveImmediate even though it now overrides compare. The approved brief already contains ordering and nested-cut laws missing from public documentation. Review probes are available under /tmp/gecode-comparison-review.PIGlZy if still present. + +## Boundaries + +- Keep the comparison API, model-defined readiness, float ranking, and initial incomparability rejection policy unchanged. +- Limit changes to the reviewed ownership paths, focused fixtures/regressions, and public contract/recovery documentation. Avoid a general exception framework. + +## Done when + +- [x] Parallel BAB releases any popped solution before rethrowing a comparison failure, and sequential PBS releases its candidate when incumbent broadcasting throws. +- [x] Focused regression tests check space lifetime after comparison failure and engine destruction; portfolio models preserve/reinstall branchers and unsupported-comparison tests use a genuinely unsupported model. +- [x] Public documentation states ordering laws, exact-search compatibility with constrain, nested/equivalent cut requirements, compatible asset semantics, float bound ranking, and model-owned readiness. +- [x] Document conservative, accurate post-exception use/reset/destruction guarantees without promising unimplemented recovery. +- [x] Rebuilt focused comparison, external-incumbent, BAB/PBS failure, float and FlatZinc checks pass; run existing PBS coverage and applicable no-thread focused checks. + +## Validation + +- Build gecode-test in build/comparison and build/comparison-no-thread. +- Run focused Search::Comparison, Search::ExternalIncumbent, Search::ParallelBABComparison, Search::PortfolioComparison, Search::FloatObjectiveComparison and FlatZinc objective comparison tests with confirmed matches and process deadlines. +- Run the existing Search::PBS:: matrix at one iteration and focused no-thread comparison/portfolio checks. Validate owned-space cleanup for both reported leaks. +- Run git diff --check and zdev check comparison. + +## Result + +Fixed solution ownership on comparison failures, repaired regression fixtures, and published ordering, pruning, readiness, float-ranking, and recovery contracts; independently verified. + +Validation: + +- Threaded and no-thread gecode-test builds passed; every applicable focused comparison and FlatZinc test matched and passed. +- All 276 Search::PBS:: cases passed at one iteration. +- Sequential PBS and parallel BAB probes each caught 100 failures with zero surviving spaces after destruction. +- Independent verifier passed immutable snapshot W9a4b02f606b0b537; git diff --check and zdev check comparison passed. diff --git a/.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" diff --git a/changelog.in b/changelog.in index e757356aa0..95d053c34d 100755 --- a/changelog.in +++ b/changelog.in @@ -75,6 +75,20 @@ Date: unreleased [DESCRIPTION] This is the development changelog for the next Gecode release. +[ENTRY] +Module: search +What: change +Rank: major +[DESCRIPTION] +Add Space::compare(), which ranks two solutions without modifying them. +Best-solution search uses it to choose between solutions, for example in +parallel BAB, portfolios, external incumbents, and the experimental A* and +CABS engines. constrain() still prunes the search. Optimization models with a +custom constrain() must now also override compare(). compare_int() +compares two integer objective values for a given Sense. Search +engines throw Search::Incomparable when compare() reports incomparable +solutions. + [RELEASE] Version: 6.5.0 Date: 2026-10-31 diff --git a/examples/car-sequencing.cpp b/examples/car-sequencing.cpp index de6cc757cb..c71ae85354 100644 --- a/examples/car-sequencing.cpp +++ b/examples/car-sequencing.cpp @@ -369,6 +369,12 @@ 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 = static_cast(_other); + return compare_int(nstall.val(), other.nstall.val(), Sense::minimize); + } + /// Print solution virtual void print(std::ostream& os) const { @@ -634,4 +640,3 @@ namespace { }; // STATISTICS: example-any - diff --git a/examples/fifteen-puzzle.cpp b/examples/fifteen-puzzle.cpp index 16ef255fb2..f6325be4ce 100644 --- a/examples/fifteen-puzzle.cpp +++ b/examples/fifteen-puzzle.cpp @@ -418,7 +418,7 @@ run(PuzzleOptions& opt) { case PuzzleOptions::SEARCH_ASTAR: case PuzzleOptions::SEARCH_CABS: { - Search::HeuristicGuidance guidance(Search::HeuristicGuidance::MINIMIZE, + Search::HeuristicGuidance guidance(Sense::minimize, [](const Space& home) -> std::int64_t { return static_cast(home).bound(); }); diff --git a/examples/tsptw.cpp b/examples/tsptw.cpp index 2b83ad474b..f40d635388 100644 --- a/examples/tsptw.cpp +++ b/examples/tsptw.cpp @@ -381,7 +381,7 @@ main(int argc, char* argv[]) { case TSPTW::SEARCH_ASTAR: case TSPTW::SEARCH_CABS: { - Search::HeuristicGuidance g(Search::HeuristicGuidance::MINIMIZE, + Search::HeuristicGuidance g(Sense::minimize, [](const Space& s) -> std::int64_t { return static_cast(s).cost().min(); }); diff --git a/gecode/driver.hh b/gecode/driver.hh index 0dd99b4caf..6c4662f859 100755 --- a/gecode/driver.hh +++ b/gecode/driver.hh @@ -881,6 +881,7 @@ namespace Gecode { namespace Driver { template class ScriptBase : public BaseSpace { public: + using BaseSpace::compare; /// Constructor ScriptBase(const Options& opt); /// Constructor used for cloning diff --git a/gecode/flatzinc.hh b/gecode/flatzinc.hh index d4b29dc5da..047a2f6c1b 100755 --- a/gecode/flatzinc.hh +++ b/gecode/flatzinc.hh @@ -576,7 +576,7 @@ namespace Gecode { namespace FlatZinc { Gecode::FloatVarArray fv_aux; /// Indicates whether a float variable is introduced by mzn2fzn std::vector fv_introduced; - /// Step by which a next solution has to have lower cost + /// Step used by float optimization cuts and comparison compatibility Gecode::FloatNum step; #endif /// Whether the introduced variables still need to be copied @@ -626,6 +626,15 @@ 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; + /** + * \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 void compare(const FlatZincSpace& s, std::ostream& out, diff --git a/gecode/flatzinc/flatzinc.cpp b/gecode/flatzinc/flatzinc.cpp index 0d525dd1f0..9edaf5a66b 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(); } @@ -2078,6 +2082,33 @@ 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"); + if (_optVarIsInt) { + return compare_int(iv[_optVar].val(),other->iv[other->_optVar].val(), + (_method == MIN) ? Sense::minimize : Sense::maximize); + } +#ifdef GECODE_HAS_FLOAT_VARS + if (step != other->step) + 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 ((ak < bk) == (_method == MIN)) ? SC_BETTER : SC_WORSE; +#else + throw DynamicCastFailed("FlatZincSpace::compare"); +#endif + } + bool FlatZincSpace::slave(const MetaInfo& mi) { if (mi.type() == MetaInfo::RESTART) { diff --git a/gecode/kernel/core.cpp b/gecode/kernel/core.cpp index 54ab989695..21b8584bfb 100644 --- a/gecode/kernel/core.cpp +++ b/gecode/kernel/core.cpp @@ -896,6 +896,11 @@ namespace Gecode { Space::constrain(const Space&) { } + SpaceComparison + Space::compare(const Space&) const { + throw SpaceNoComparison("Space::compare"); + } + bool Space::master(const MetaInfo& mi) { switch (mi.type()) { diff --git a/gecode/kernel/core.hpp b/gecode/kernel/core.hpp index 08423430ee..78516ce60f 100755 --- a/gecode/kernel/core.hpp +++ b/gecode/kernel/core.hpp @@ -53,6 +53,28 @@ namespace Gecode { + /// Result of comparing two spaces according to their objectives + enum SpaceComparison { + SC_BETTER, ///< The receiver has a better objective + SC_EQUIVALENT, ///< Both objectives are equivalent + SC_WORSE, ///< The receiver has a worse objective + SC_INCOMPARABLE ///< Neither objective dominates the other + }; + + /// Optimization direction + enum class Sense { + minimize, ///< Smaller objective values are better + maximize ///< Larger objective values are better + }; + + /// Compare objective value \a x with \a y for direction \a s + forceinline SpaceComparison + compare_int(int x, int y, Sense s) { + if (x == y) + return SC_EQUIVALENT; + return ((x < y) == (s == Sense::minimize)) ? SC_BETTER : SC_WORSE; + } + class Space; /** @@ -2148,6 +2170,21 @@ namespace Gecode { * \ingroup TaskModelScript */ GECODE_KERNEL_EXPORT virtual void constrain(const Space& best); + /** + * \brief Compare function for best solution search + * + * Must return whether this solution is better than, equivalent to, + * worse than, or incomparable with solution \a other, without modifying + * either. The result must agree with constrain: solutions admitted by + * constrain(b) are better than b, and a better b admits no more solutions. + * + * The default function throws SpaceNoComparison, so a space overriding + * constrain must also override compare. + * + * \ingroup TaskModelScript + */ + GECODE_KERNEL_EXPORT virtual SpaceComparison + compare(const Space& other) const; /** * \brief Master configuration function for meta search engines * diff --git a/gecode/kernel/exception.cpp b/gecode/kernel/exception.cpp index e16c6cd61a..994d76f9c4 100644 --- a/gecode/kernel/exception.cpp +++ b/gecode/kernel/exception.cpp @@ -47,6 +47,9 @@ namespace Gecode { SpaceNoBrancher::SpaceNoBrancher(const char* l) : Exception(l,"Attempt to commit with no brancher") {} + SpaceNoComparison::SpaceNoComparison(const char* l) + : Exception(l,"Space does not support comparison") {} + SpaceIllegalAlternative::SpaceIllegalAlternative(const char* l) : Exception(l,"Attempt to commit with illegal alternative") {} diff --git a/gecode/kernel/exception.hpp b/gecode/kernel/exception.hpp index 792f4975e6..d118b10a9f 100644 --- a/gecode/kernel/exception.hpp +++ b/gecode/kernel/exception.hpp @@ -68,6 +68,13 @@ namespace Gecode { SpaceNoBrancher(const char* l); }; + /// %Exception: Comparison not implemented by space + class GECODE_KERNEL_EXPORT SpaceNoComparison : public Exception { + public: + /// Initialize with location \a l + SpaceNoComparison(const char* l); + }; + /// %Exception: Commit with illegal alternative class GECODE_KERNEL_EXPORT SpaceIllegalAlternative : public Exception { public: diff --git a/gecode/minimodel.hh b/gecode/minimodel.hh index 59c7869e61..117c020367 100755 --- a/gecode/minimodel.hh +++ b/gecode/minimodel.hh @@ -2442,6 +2442,9 @@ namespace Gecode { /// Member function constraining according to decreasing cost GECODE_MINIMODEL_EXPORT virtual void constrain(const Space& best); + /// Compare integer costs (both costs must be assigned) + GECODE_MINIMODEL_EXPORT + virtual SpaceComparison compare(const Space& other) const; /// Return variable with current cost virtual IntVar cost(void) const = 0; }; @@ -2459,6 +2462,9 @@ namespace Gecode { /// Member function constraining according to increasing cost GECODE_MINIMODEL_EXPORT virtual void constrain(const Space& best); + /// Compare integer costs (both costs must be assigned) + GECODE_MINIMODEL_EXPORT + virtual SpaceComparison compare(const Space& other) const; /// Return variable with current cost virtual IntVar cost(void) const = 0; }; @@ -2476,6 +2482,9 @@ namespace Gecode { /// Member function constraining according to decreasing costs GECODE_MINIMODEL_EXPORT virtual void constrain(const Space& best); + /// Compare integer costs lexicographically + GECODE_MINIMODEL_EXPORT + virtual SpaceComparison compare(const Space& other) const; /// Return variables with current costs virtual IntVarArgs cost(void) const = 0; }; @@ -2493,6 +2502,9 @@ namespace Gecode { /// Member function constraining according to increasing costs GECODE_MINIMODEL_EXPORT virtual void constrain(const Space& best); + /// Compare integer costs lexicographically + GECODE_MINIMODEL_EXPORT + virtual SpaceComparison compare(const Space& other) const; /// Return variables with current costs virtual IntVarArgs cost(void) const = 0; }; @@ -2505,6 +2517,9 @@ namespace Gecode { * The class supports using a step value \a step that will make sure * that better solutions must be better by at least the value of * \a step. + * Comparison ranks the upper endpoint of assigned costs independently of + * the step. Both operands must use this base class and the same step. + * Assigned costs may be tight intervals between adjacent float values. * * \ingroup TaskModelMiniModelOptimize */ @@ -2520,6 +2535,9 @@ namespace Gecode { /// Member function constraining according to cost GECODE_MINIMODEL_EXPORT virtual void constrain(const Space& best); + /// Compare upper endpoints of assigned costs with a common step policy + GECODE_MINIMODEL_EXPORT + virtual SpaceComparison compare(const Space& other) const; /// Return variable with current cost virtual FloatVar cost(void) const = 0; }; @@ -2530,6 +2548,9 @@ namespace Gecode { * The class supports using a step value \a step that will make sure * that better solutions must be better by at least the value of * \a step. + * Comparison ranks the lower endpoint of assigned costs independently of + * the step. Both operands must use this base class and the same step. + * Assigned costs may be tight intervals between adjacent float values. * * \ingroup TaskModelMiniModelOptimize */ @@ -2545,6 +2566,9 @@ namespace Gecode { /// Member function constraining according to cost GECODE_MINIMODEL_EXPORT virtual void constrain(const Space& best); + /// Compare lower endpoints of assigned costs with a common step policy + GECODE_MINIMODEL_EXPORT + virtual SpaceComparison compare(const Space& other) const; /// Return variable with current cost virtual FloatVar cost(void) const = 0; }; diff --git a/gecode/minimodel/optimize.cpp b/gecode/minimodel/optimize.cpp index 6442ada179..2f585c91bf 100755 --- a/gecode/minimodel/optimize.cpp +++ b/gecode/minimodel/optimize.cpp @@ -44,6 +44,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(),Sense::minimize); + } + void IntMaximizeSpace::constrain(const Space& _best) { @@ -54,6 +62,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(),Sense::maximize); + } + void IntLexMinimizeSpace::constrain(const Space& _best) { @@ -68,6 +84,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; icost().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) { @@ -102,9 +164,20 @@ 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 } // STATISTICS: minimodel-search - diff --git a/gecode/search.hh b/gecode/search.hh index 11b40fc458..703c844cb7 100755 --- a/gecode/search.hh +++ b/gecode/search.hh @@ -1236,8 +1236,10 @@ namespace Gecode { namespace Search { * * The bound is an optimistic objective value for a stable space: a lower * bound when minimizing, an upper bound when maximizing, and the exact - * objective for a solution. It must agree with the model's constrain - * function. The optional rank orders nodes (lower first) but is never + * objective for a solution. It must agree with the model's constrain and + * compare functions. The bound orders and prunes open nodes, while + * solutions replace the incumbent only when Space::compare reports + * SC_BETTER. The optional rank orders nodes (lower first) but is never * used for pruning. * * The optional dominance predicate returns whether every completion of @@ -1258,8 +1260,6 @@ namespace Gecode { namespace Search { */ class HeuristicGuidance { public: - /// Objective direction - enum Sense { MINIMIZE, MAXIMIZE }; /// Function evaluating a stable space typedef std::function Evaluate; /// Objective direction @@ -1287,7 +1287,8 @@ namespace Gecode { * Expands nodes in order of their bound, then their rank, using the * guidance from Search::Options::guidance. Like BAB, next() returns * improving solutions and exhausting the search proves the last - * solution optimal. + * solution optimal. Additionally, \a T must implement Space::constrain + * and Space::compare. * * All open nodes are stored as clones, so c_d and a_d are ignored. * Tracing is not supported. @@ -1325,7 +1326,8 @@ namespace Gecode { * improve the incumbent, the search restarts with twice the width. * The guidance is taken from Search::Options::guidance. Like BAB, * next() returns improving solutions and exhausting the search proves - * the last solution optimal. + * the last solution optimal. Additionally, \a T must implement + * Space::constrain and Space::compare. * * Search stops when the width would exceed Search::Options::max_width or * the stored nodes would exceed Search::Options::frontier_limit. Nodes are @@ -1361,8 +1363,7 @@ namespace Gecode { /** * \brief Depth-first branch-and-bound search engine * - * Additionally, \a s must implement a member function - * \code virtual void constrain(const T& t) \endcode + * Additionally, \a s must implement Space::constrain and Space::compare. * Whenever exploration requires to add a constraint * to the space \a c currently being explored, the engine * executes \c c.constrain(t) where \a t is the so-far @@ -1381,8 +1382,7 @@ namespace Gecode { /** * \brief Perform depth-first branch-and-bound search for subclass \a T of space \a s and options \a o * - * Additionally, \a s must implement a member function - * \code virtual void constrain(const T& t) \endcode + * Additionally, \a s must implement Space::constrain and Space::compare. * Whenever exploration requires to add a constraint * to the space \a c currently being explored, the engine * executes \c c.constrain(t) where \a t is the so-far @@ -1524,6 +1524,8 @@ namespace Gecode { * The engine will run a portfolio with a number of assets as defined * by the options \a o. The engine supports parallel execution of * assets by using the number of threads as defined by the options. + * For best-solution assets, a solution replaces the incumbent only if + * Space::compare reports SC_BETTER. * An external worker control in \a o is supported only for a single * homogeneous asset. Multiple controlled assets require explicit engine * builders, each with its own control and immutable worker capacity. diff --git a/gecode/search/astar.cpp b/gecode/search/astar.cpp index 9ed5eeab81..3931e17fc1 100644 --- a/gecode/search/astar.cpp +++ b/gecode/search/astar.cpp @@ -142,9 +142,9 @@ namespace Gecode { namespace Search { namespace Seq { AStar::publish(std::unique_ptr s) { // Delete exhausted branchers before cloning the solution (void) s->choice(); - std::int64_t b = guidance.bound(*s); - if (!best.improves(b)) + if (!best.replaced_by(*s,"AStar::next")) return nullptr; + std::int64_t b = guidance.bound(*s); Space* copy = s->clone(); best.update(s.release(),b); return copy; @@ -214,9 +214,8 @@ namespace Gecode { namespace Search { namespace Seq { void AStar::constrain(const Space& s) { - std::int64_t b = guidance.bound(s); - if (best.improves(b)) - best.update(s.clone(),b); + if (best.replaced_by(s,"AStar::constrain")) + best.update(s.clone(),guidance.bound(s)); } void diff --git a/gecode/search/cabs.cpp b/gecode/search/cabs.cpp index 5616a2e202..233cd489b7 100644 --- a/gecode/search/cabs.cpp +++ b/gecode/search/cabs.cpp @@ -210,9 +210,9 @@ namespace Gecode { namespace Search { namespace Seq { CABS::publish(std::unique_ptr s) { // Delete exhausted branchers before cloning the solution (void) s->choice(); - std::int64_t b = guidance.bound(*s); - if (!best.improves(b)) + if (!best.replaced_by(*s,"CABS::next")) return nullptr; + std::int64_t b = guidance.bound(*s); Space* copy = s->clone(); best.update(s.release(),b); return copy; @@ -316,9 +316,8 @@ namespace Gecode { namespace Search { namespace Seq { void CABS::constrain(const Space& s) { - std::int64_t b = guidance.bound(s); - if (best.improves(b)) - best.update(s.clone(),b); + if (best.replaced_by(s,"CABS::constrain")) + best.update(s.clone(),guidance.bound(s)); } void diff --git a/gecode/search/exception.cpp b/gecode/search/exception.cpp index 8d99738996..8e3bcb8974 100644 --- a/gecode/search/exception.cpp +++ b/gecode/search/exception.cpp @@ -64,6 +64,9 @@ namespace Gecode { namespace Search { WorkerControlInUse::WorkerControlInUse(const char* l) : Exception(l,"Worker control is already bound or has unsupported engine placement") {} + Incomparable::Incomparable(const char* l) + : Exception(l,"Incomparable objectives are not supported by best solution search") {} + }} // STATISTICS: search-other diff --git a/gecode/search/exception.hpp b/gecode/search/exception.hpp index e28c3fd9ce..16b65661f1 100644 --- a/gecode/search/exception.hpp +++ b/gecode/search/exception.hpp @@ -80,6 +80,12 @@ namespace Gecode { namespace Search { /// Initialize with location \a l WorkerControlInUse(const char* l); }; + /// %Exception: Incomparable objectives are unsupported by best search + class GECODE_SEARCH_EXPORT Incomparable : public Exception { + public: + /// Initialize with location \a l + Incomparable(const char* l); + }; #ifdef GECODE_HAS_EXPERIMENTAL /// %Exception: Missing guidance for heuristic search class GECODE_SEARCH_EXPORT UninitializedGuidance : public Exception { diff --git a/gecode/search/heuristic.hh b/gecode/search/heuristic.hh index 87fde7c609..6502547e51 100644 --- a/gecode/search/heuristic.hh +++ b/gecode/search/heuristic.hh @@ -35,6 +35,7 @@ #define GECODE_SEARCH_HEURISTIC_HH #include +#include #include #include #include @@ -45,8 +46,8 @@ namespace Gecode { namespace Search { /// Whether bound \a a is better than bound \a b for objective direction \a s forceinline bool - better(HeuristicGuidance::Sense s, std::int64_t a, std::int64_t b) { - return (s == HeuristicGuidance::MINIMIZE) ? (a < b) : (a > b); + better(Sense s, std::int64_t a, std::int64_t b) { + return (s == Sense::minimize) ? (a < b) : (a > b); } /// Open node of a heuristic search engine @@ -70,7 +71,7 @@ namespace Gecode { namespace Search { class Priority { public: /// Objective direction - HeuristicGuidance::Sense sense; + Sense sense; /// Whether to order by rank only (CABS with rank) bool rank_only; /// Test whether \a a is preferred to \a b @@ -83,11 +84,16 @@ namespace Gecode { namespace Search { } }; - /// Best solution found so far + /** + * \brief Best solution found so far + * + * Solutions are ranked with Space::compare, like in the other best-solution + * engines. The bound of the incumbent prunes open nodes. + */ class Incumbent { public: /// Objective direction - HeuristicGuidance::Sense sense; + Sense sense; /// The solution (if any) std::unique_ptr space; /// Its objective value @@ -95,12 +101,16 @@ namespace Gecode { namespace Search { /// Incremented for every new incumbent unsigned long long int revision; /// Initialize without incumbent - Incumbent(HeuristicGuidance::Sense s) + Incumbent(Sense s) : sense(s), bound(0), revision(0) {} /// Whether a node with bound \a b can improve the incumbent bool improves(std::int64_t b) const { return !space || better(sense,b,bound); } + /// Whether solution \a s should replace the incumbent (location \a l) + bool replaced_by(const Space& s, const char* l) const { + return !space || Search::better(s,*space,l); + } /// Make \a s with objective value \a b the incumbent void update(Space* s, std::int64_t b) { space.reset(s); bound = b; revision++; diff --git a/gecode/search/par/bab.hh b/gecode/search/par/bab.hh index 4a1911b208..cf5a3d7fb9 100644 --- a/gecode/search/par/bab.hh +++ b/gecode/search/par/bab.hh @@ -35,6 +35,7 @@ #define GECODE_SEARCH_PAR_BAB_HH #include +#include namespace Gecode { namespace Search { namespace Par { @@ -46,6 +47,7 @@ namespace Gecode { namespace Search { namespace Par { using Engine::busy; using Engine::stop; using Engine::block; + using Engine::cmd; using Engine::e_search; using Engine::e_reset_ack_start; using Engine::e_reset_ack_stop; @@ -104,6 +106,8 @@ namespace Gecode { namespace Search { namespace Par { Worker** _worker; /// Best solution so far Space* best; + /// Failure raised while accepting a solution in a worker thread + std::exception_ptr failure; public: /// Provide access to worker \a i Worker* worker(unsigned int i) const; @@ -118,6 +122,8 @@ namespace Gecode { namespace Search { namespace Par { //@{ /// Initialize for space \a s with options \a o BAB(Space* s, const Options& o); + /// Return next solution + virtual Space* next(void); /// Return statistics virtual Statistics statistics(void) const; /// Reset engine to restart at space \a s diff --git a/gecode/search/par/bab.hpp b/gecode/search/par/bab.hpp index deb0ae5f9b..0876ca7c9a 100755 --- a/gecode/search/par/bab.hpp +++ b/gecode/search/par/bab.hpp @@ -89,7 +89,7 @@ namespace Gecode { namespace Search { namespace Par { template forceinline BAB::BAB(Space* s, const Options& o) - : Engine(o), best(nullptr) { + : Engine(o), best(nullptr), failure(nullptr) { WrapTraceRecorder::engine(o.tracer, SearchTracer::EngineType::DFS, workers()); // Create workers @@ -126,17 +126,33 @@ namespace Gecode { namespace Search { namespace Par { template forceinline void BAB::solution(Space* s) { - m_search.acquire(); - if (best != nullptr) { - s->constrain(*best); - if (s->status() == SS_FAILED) { + Support::Lock lock(m_search); + if (failure != nullptr) { + delete s; + return; + } + try { + if ((best != nullptr) && + !Search::better(*s,*best,"BAB::solution")) { delete s; - m_search.release(); return; - } else { - delete best; - best = s->clone(); } + } catch (...) { + delete s; + while (!solutions.empty()) + delete solutions.pop(); + failure = std::current_exception(); + // A null entry is private to BAB and only wakes Engine::next. That + // call blocks the workers before BAB::next rethrows the failure. + bool bs = signal(); + solutions.push(nullptr); + if (bs) + e_search.signal(); + return; + } + if (best != nullptr) { + delete best; + best = s->clone(); } else { best = s->clone(); } @@ -147,7 +163,6 @@ namespace Gecode { namespace Search { namespace Par { solutions.push(s); if (bs) e_search.signal(); - m_search.release(); } @@ -198,20 +213,46 @@ 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(); + delete s; + std::rethrow_exception(f); + } + return s; } /* @@ -392,9 +433,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/gecode/search/par/frontier.cpp b/gecode/search/par/frontier.cpp index dacb958871..5977f8827f 100644 --- a/gecode/search/par/frontier.cpp +++ b/gecode/search/par/frontier.cpp @@ -155,12 +155,10 @@ namespace Gecode { namespace Search { namespace Par { void Frontier::publish(Space* s, std::int64_t b) { - if (!best.improves(b)) { - delete s; - return; - } std::unique_ptr incumbent(s); - solution.reset(s->clone()); + if (!best.replaced_by(*incumbent,"Frontier::publish")) + return; + solution.reset(incumbent->clone()); best.update(incumbent.release(),b); running = false; } @@ -281,7 +279,8 @@ namespace Gecode { namespace Search { namespace Par { (void) step.space->choice(); std::int64_t b = guidance.bound(*step.space); if (step.status == SS_SOLVED) { - if (!cutoff || better(guidance.sense,b,*cutoff)) { + // Comparison decides solutions that tie with the incumbent bound + if (!cutoff || !better(guidance.sense,*cutoff,b)) { solved = std::move(step.space); solved_bound = b; break; @@ -375,7 +374,7 @@ namespace Gecode { namespace Search { namespace Par { Frontier::constrain(const Space& s) { std::int64_t b = guidance.bound(s); Support::Lock lock(mutex); - if (best.improves(b)) { + if (best.replaced_by(s,"Frontier::constrain")) { best.update(s.clone(),b); solution.reset(); } diff --git a/gecode/search/par/frontier.hh b/gecode/search/par/frontier.hh index a3825097c9..23a5075a5a 100644 --- a/gecode/search/par/frontier.hh +++ b/gecode/search/par/frontier.hh @@ -143,7 +143,7 @@ namespace Gecode { namespace Search { namespace Par { void push(Task* t); /// Stop all workers (must be called by subclass destructors) void terminate(void); - /// Make \a s with bound \a b the incumbent, if it improves + /// Make \a s with bound \a b the incumbent, if it compares better void publish(Space* s, std::int64_t b); /// Start the threads void launch(void); diff --git a/gecode/search/par/pbs.hh b/gecode/search/par/pbs.hh index 40d93e7252..7aabfa9558 100644 --- a/gecode/search/par/pbs.hh +++ b/gecode/search/par/pbs.hh @@ -40,6 +40,7 @@ #include #include +#include namespace Gecode { namespace Search { namespace Par { @@ -190,8 +191,12 @@ namespace Gecode { namespace Search { namespace Par { unsigned int n_busy; /// Signal that number of busy slaves becomes zero Support::Event idle; + /// Failure raised while running or updating a portfolio asset + std::exception_ptr failure; /// Process report from slave, return false if solution was ignored bool report(Slave* slave, Space* s); + /// Process failure from slave + void fail(void); /** * The key invariant of the engine is as follows: * - n_busy is always zero outside the next() function. diff --git a/gecode/search/par/pbs.hpp b/gecode/search/par/pbs.hpp index 0f7f03e769..80005fafaa 100755 --- a/gecode/search/par/pbs.hpp +++ b/gecode/search/par/pbs.hpp @@ -94,29 +94,22 @@ namespace Gecode { namespace Search { namespace Par { : b(nullptr), reporter(nullptr) {} forceinline bool CollectBest::add(Space* s, Slave* r) { - if (b != nullptr) { - b->constrain(*s); - if (b->status() == SS_FAILED) { - delete b; - } else { - delete s; - return false; - } + if ((b != nullptr) && + !Search::better(*s,*b,"PBS::CollectBest::add")) { + delete s; + return false; } + delete b; b = s; reporter = r; return true; } forceinline bool CollectBest::constrain(const Space& s) { - if (b != nullptr) { - b->constrain(s); - if (b->status() == SS_FAILED) { - delete b; - } else { - return false; - } - } + if ((b != nullptr) && + !Search::better(s,*b,"PBS::CollectBest::constrain")) + return false; + delete b; b = s.clone(); reporter = nullptr; return true; @@ -222,8 +215,16 @@ namespace Gecode { namespace Search { namespace Par { bool wake = false; m.acquire(); if (s != nullptr) { - b = solutions.add(s,slave); - if (b) { + try { + b = solutions.add(s,slave); + if (b) { + tostop.store(true, std::memory_order_release); + wake = true; + } + } catch (...) { + delete s; + if (failure == nullptr) + failure = std::current_exception(); tostop.store(true, std::memory_order_release); wake = true; } @@ -258,22 +259,45 @@ namespace Gecode { namespace Search { namespace Par { return b; } + template + forceinline void + PBS::fail(void) { + m.acquire(); + if (failure == nullptr) + failure = std::current_exception(); + tostop.store(true, std::memory_order_release); + for (unsigned int i=0U; iwake(); + if (--n_busy == 0) + idle.signal(); + m.release(); + } + template void Slave::run(void) { - Space* s; - do { - s = slave->next(); - } while (!master->report(this,s)); + try { + Space* s; + do { + s = slave->next(); + } while (!master->report(this,s)); + } catch (...) { + master->fail(); + } } template Space* PBS::next(void) { m.acquire(); + if (failure != nullptr) { + std::exception_ptr f = failure; + m.release(); + std::rethrow_exception(f); + } if (solutions.empty()) slave_stop.store(false, std::memory_order_release); - while (solutions.empty() && (n_active > 0) && + while (solutions.empty() && (n_active > 0) && (failure == nullptr) && !slave_stop.load(std::memory_order_acquire)) { // Clear the internal stop used to interrupt sibling slaves tostop.store(false, std::memory_order_release); @@ -310,10 +334,26 @@ namespace Gecode { namespace Search { namespace Par { } else { Slave* r; s = solutions.get(r); - if (Collect::best) - for (unsigned int i=0U; iconstrain(*s); + if (Collect::best) { + try { + for (unsigned int i=0U; iconstrain(*s); + } catch (...) { + delete s; + failure = std::current_exception(); + std::exception_ptr f = failure; + m.release(); + std::rethrow_exception(f); + } + } + } + + if (failure != nullptr) { + delete s; + std::exception_ptr f = failure; + m.release(); + std::rethrow_exception(f); } m.release(); @@ -342,10 +382,15 @@ namespace Gecode { namespace Search { namespace Par { assert(n_busy == 0); if (!Collect::best) throw NoBest("PBS::constrain"); - if (solutions.constrain(b)) { - // The solution is better - for (unsigned int i=0U; iconstrain(b); + try { + if (solutions.constrain(b)) { + // The solution is better + for (unsigned int i=0U; iconstrain(b); + } + } catch (...) { + failure = std::current_exception(); + std::rethrow_exception(failure); } } diff --git a/gecode/search/seq/bab.hpp b/gecode/search/seq/bab.hpp index dade2cb075..fed0129c6a 100644 --- a/gecode/search/seq/bab.hpp +++ b/gecode/search/seq/bab.hpp @@ -161,14 +161,9 @@ namespace Gecode { namespace Search { namespace Seq { template forceinline void BAB::constrain(const Space& b) { - if (best != nullptr) { - // Check whether b is in fact better than best - best->constrain(b); - if (best->status(*this) != SS_FAILED) - return; - else - delete best; - } + if ((best != nullptr) && !Search::better(b,*best,"BAB::constrain")) + return; + delete best; best = b.clone(); if (cur != nullptr) cur->constrain(b); diff --git a/gecode/search/seq/pbs.hpp b/gecode/search/seq/pbs.hpp index 4dc77efef1..7dc63372b6 100644 --- a/gecode/search/seq/pbs.hpp +++ b/gecode/search/seq/pbs.hpp @@ -110,10 +110,15 @@ namespace Gecode { namespace Search { namespace Seq { if (Space* s = slaves[cur].next()) { // Constrain other slaves if (best) { - for (unsigned int i=0U; iconstrain(b); - if (last->status() == SS_FAILED) { - delete last; - } else { - return; - } - } + if ((last != nullptr) && !Search::better(b,*last,"RBS::constrain")) + return; + delete last; last = b.clone(); master->constrain(b); e->constrain(b); diff --git a/gecode/search/support.hh b/gecode/search/support.hh index fe25d487b2..fc1e6116d8 100644 --- a/gecode/search/support.hh +++ b/gecode/search/support.hh @@ -38,6 +38,23 @@ namespace Gecode { namespace Search { + /// Whether incoming solution \a s should replace incumbent \a b + forceinline bool + better(const Space& s, const Space& b, const char* l) { + switch (s.compare(b)) { + case SC_BETTER: + return true; + case SC_EQUIVALENT: + case SC_WORSE: + return false; + case SC_INCOMPARABLE: + throw Incomparable(l); + default: + GECODE_NEVER; + } + return false; + } + /// Clone space \a s depending on options \a o forceinline Space* snapshot(Space* s, const Options& o); diff --git a/test/flatzinc.cpp b/test/flatzinc.cpp index c1845f8522..3a9dc8798a 100755 --- a/test/flatzinc.cpp +++ b/test/flatzinc.cpp @@ -34,6 +34,9 @@ #include "test/flatzinc.hh" #include +#include +#include +#include namespace Test { namespace FlatZinc { @@ -65,6 +68,157 @@ namespace Test { namespace FlatZinc { TupleSetAutoRepresentation tuple_set_auto_representation; + /// Verify integer objective comparison for FlatZinc spaces. + class IntegerObjectiveComparison : public Base { + private: + static std::unique_ptr + 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; + +#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 b1f0354815..3be463df47 100644 --- a/test/search.cpp +++ b/test/search.cpp @@ -43,6 +43,7 @@ #include #include +#include #include #include #include @@ -152,12 +153,16 @@ namespace Test { ceiling = static_cast(s).node().cost; if (node().cost >= ceiling) fail(); } + SpaceComparison compare(const Space& s) const override { + int a = node().cost; + int b = static_cast(s).node().cost; + return (a < b) ? SC_BETTER : ((a > b) ? SC_WORSE : SC_EQUIVALENT); + } /// Guidance minimizing cost, or maximizing negated cost static Gecode::Search::HeuristicGuidance guidance(bool max, bool ranked, bool dominance, bool keyed) { using Gecode::Search::HeuristicGuidance; - HeuristicGuidance g(max ? HeuristicGuidance::MAXIMIZE : - HeuristicGuidance::MINIMIZE, + HeuristicGuidance g(max ? Sense::maximize : Sense::minimize, [](const Space& s) -> std::int64_t { const TreeSpace& t = static_cast(s); return t.maximize ? -t.node().bound : t.node().bound; @@ -464,6 +469,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; @@ -557,6 +566,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) { @@ -605,6 +648,9 @@ namespace Test { virtual bool master(const MetaInfo& mi) { switch (mi.type()) { case MetaInfo::RESTART: + // With an objective, restart from a bound improving on the last solution + if (htc != HTC_NONE) + return Space::master(mi); if (mi.last() != nullptr) { const HasSolutions* s = static_cast(mi.last()); @@ -813,7 +859,9 @@ namespace Test { o.threads = threads; o.worker_control = control; o.stop = &pause; - HasSolutions root(HTB_BINARY,HTB_BINARY,HTB_BINARY); + // Best-solution search needs an objective; it then ends with the best + HasSolutions root(HTB_BINARY,HTB_BINARY,HTB_BINARY, + E::best ? HTC_LEX_LE : HTC_NONE); E engine(&root,o); auto result = std::async(std::launch::async,[&] { return engine.next(); }); auto reached = pause.reached.get_future(); @@ -822,21 +870,22 @@ namespace Test { std::future_status::timeout; control.request(1U); int solutions = 0; - if (HasSolutions* solution = result.get()) { + std::unique_ptr last(result.get()); + if (last) ++solutions; - delete solution; - } // Alternate expansion and shrinking at solution boundaries. This - // exercises each request while preserving complete enumeration. + // exercises each request while preserving complete search. while (true) { control.request((solutions % 2) ? threads : 1U); HasSolutions* solution = engine.next(); if (!solution) break; ++solutions; - delete solution; + last.reset(solution); } - return ok && !engine.stopped() && (solutions == root.solutions()); + bool complete = E::best ? + (last && last->best()) : (solutions == root.solutions()); + return ok && !engine.stopped() && complete; } #ifdef GECODE_HAS_EXPERIMENTAL template class E> @@ -1142,6 +1191,692 @@ namespace Test { WorkerControlPBSPause worker_control_pbs_pause; + /// Scalar integer objective used for comparison tests + class MinObjective : public IntMinimizeSpace { + public: + IntVar x; + MinObjective(int l, int u) : x(*this,l,u) {} + MinObjective(MinObjective& s) : IntMinimizeSpace(s) { + x.update(*this,s.x); + } + virtual Space* copy(void) { return new MinObjective(*this); } + virtual IntVar cost(void) const { return x; } + }; + + /// Scalar maximization objective used to check objective families + class MaxObjective : public IntMaximizeSpace { + public: + IntVar x; + MaxObjective(int v) : x(*this,v,v) {} + MaxObjective(MaxObjective& s) : IntMaximizeSpace(s) { + x.update(*this,s.x); + } + virtual Space* copy(void) { return new MaxObjective(*this); } + virtual IntVar cost(void) const { return x; } + }; + + /// Objective used to test external incumbent updates + class ExternalObjective : public IntMinimizeSpace { + public: + /// Per-test count of posted improvement constraints (optional) + std::atomic* constraints; + IntVar x; + ExternalObjective(std::atomic* c) : constraints(c), x(*this,0,10) { + Gecode::branch(*this,x,INT_VAL_MIN()); + } + ExternalObjective(int v) : constraints(nullptr), x(*this,v,v) {} + ExternalObjective(ExternalObjective& s) + : IntMinimizeSpace(s), constraints(s.constraints) { + x.update(*this,s.x); + } + virtual Space* copy(void) { return new ExternalObjective(*this); } + virtual IntVar cost(void) const { return x; } + virtual void constrain(const Space& s) { + if (constraints != nullptr) + constraints->fetch_add(1); + IntMinimizeSpace::constrain(s); + } + }; + + /// 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; } + virtual bool master(const MetaInfo& mi) { + if (mi.type() == MetaInfo::PORTFOLIO) + return false; + return Space::master(mi); + } + }; + + /// 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 }; + /// Per-test count of live spaces + std::atomic& live; + IntVar x; + Failure failure; + FailingParallelObjective(Failure f, std::atomic& l) + : live(l), x(*this,0,10), failure(f) { + live++; + Gecode::branch(*this,x,INT_VAL_MAX()); + } + FailingParallelObjective(FailingParallelObjective& s) + : Space(s), live(s.live), failure(s.failure) { + live++; + x.update(*this,s.x); + } + virtual ~FailingParallelObjective(void) { live--; } + 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; + } + virtual bool master(const MetaInfo& mi) { + if (mi.type() == MetaInfo::PORTFOLIO) + return false; + return Space::master(mi); + } + }; + + + /// 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; + } + }; + + /// Search model without objective comparison support + class UnsupportedObjective : public Space { + public: + UnsupportedObjective(void) {} + UnsupportedObjective(UnsupportedObjective& s) : Space(s) {} + virtual Space* copy(void) { return new UnsupportedObjective(*this); } + }; + + /// Test sequential BAB and RBS external incumbent arbitration + class ExternalIncumbent : public Base { + private: + template + static bool updates(Engine& e, const std::atomic& constraints) { + ExternalObjective five(5), equal(5), worse(7), better(3); + int n = constraints; + e.constrain(five); + int installed = constraints; + e.constrain(equal); + e.constrain(worse); + if ((installed <= n) || (constraints != installed)) + return false; + e.constrain(better); + return constraints > installed; + } + /// Whether engine \a e reports an incomparable incumbent (deletes \a e) + static bool rejects_incomparable(Gecode::Search::Engine* e) { + std::unique_ptr engine(e); + IncomparableObjective incomparable(4), incumbent(5); + engine->constrain(incumbent); + try { + engine->constrain(incomparable); + } catch (const Gecode::Search::Incomparable&) { + return true; + } + return false; + } + public: + ExternalIncumbent(void) : Base("Search::ExternalIncumbent") {} + virtual bool run(void) { + Gecode::Search::Options o; + std::atomic constraints(0); + ExternalObjective* bm = new ExternalObjective(&constraints); + Gecode::Search::Engine* bab = Gecode::Search::babengine(bm,o); + delete bm; + if (!updates(*bab,constraints)) { + delete bab; + return false; + } + delete bab; + + o.cutoff = Gecode::Search::Cutoff::constant(10); + ExternalObjective* rm = new ExternalObjective(&constraints); + Gecode::Search::Engine* rbs = + Gecode::Search::build >(rm,o); + delete rm; + if (!updates(*rbs,constraints)) { + delete rbs; + return false; + } + delete rbs; + + ExternalObjective im(nullptr); + if (!rejects_incomparable(Gecode::Search::babengine(&im,Gecode::Search::Options()))) + return false; +#ifdef GECODE_HAS_EXPERIMENTAL + Gecode::Search::HeuristicGuidance g(Gecode::Sense::minimize, + [](const Space& s) -> std::int64_t { + return static_cast(s).x.min(); + }); + for (double threads : {1.0,2.0}) { + Gecode::Search::Options ho; + ho.threads = threads; + ho.guidance = &g; + if (!rejects_incomparable(Gecode::Search::build(&im,ho)) || + !rejects_incomparable(Gecode::Search::build(&im,ho))) + return false; + } +#endif + + UnsupportedObjective unsupported; + UnsupportedObjective* um = + static_cast(unsupported.clone()); + Gecode::Search::Engine* missing = Gecode::Search::babengine( + um,Gecode::Search::Options()); + delete um; + missing->constrain(unsupported); + try { + missing->constrain(unsupported); + delete missing; + return false; + } catch (const SpaceNoComparison&) {} + delete missing; + return true; + } + }; + + ExternalIncumbent external_incumbent; + +#ifdef GECODE_HAS_THREADS + /// 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) { + std::atomic live(0); + Gecode::Search::TimeStop stop(5000); + Gecode::Search::Options o = options(); + o.stop = &stop; + FailingParallelObjective* m = + new FailingParallelObjective(FailingParallelObjective::MISSING,live); + 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 && (live == 0); + } + static bool thrownFailure(void) { + std::atomic live(0); + Gecode::Search::TimeStop stop(5000); + Gecode::Search::Options o = options(); + o.stop = &stop; + FailingParallelObjective* m = + new FailingParallelObjective(FailingParallelObjective::THROWN,live); + 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 && (live == 0); + } + delete e; + return false; + } + static bool incomparableFailure(void) { + std::atomic live(0); + Gecode::Search::TimeStop stop(5000); + Gecode::Search::Options o = options(); + o.stop = &stop; + FailingParallelObjective* m = new FailingParallelObjective( + FailingParallelObjective::INCOMPARABLE,live); + 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 && (live == 0); + } + 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; +#endif + + /// 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, + double threads=2.0) { + std::atomic live(0); + bool caught = false; + { + FailingParallelObjective* m = new FailingParallelObjective(f,live); + Gecode::Search::Options o = options(); + o.threads = threads; + Gecode::PBS pbs(m,o); + delete m; + try { + while (Space* s = pbs.next()) delete s; + } catch (const SpaceNoComparison&) { + if (f != FailingParallelObjective::MISSING) + return false; + caught = true; + } catch (const ComparisonError&) { + caught = (f == FailingParallelObjective::THROWN); + } catch (const Gecode::Search::Incomparable&) { + caught = (f == FailingParallelObjective::INCOMPARABLE); + } + } + return caught && (live == 0); + } + 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; + + std::atomic constraints(0); + ExternalObjective* em = new ExternalObjective(&constraints); + Gecode::Search::Engine* external = + Gecode::Search::build >(em,o); + delete em; + ExternalObjective five(5), equal(5), worse(7), better(3); + int n = constraints; + external->constrain(five); + int installed = constraints; + external->constrain(equal); + external->constrain(worse); + if ((installed <= n) || + (constraints != installed)) { + delete external; + return false; + } + external->constrain(better); + if (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,1) && + 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 { + public: + FloatVar x; + ParallelFloatObjective(void) + : FloatMinimizeSpace(1.0), x(*this,8.0,10.0) { + Gecode::branch(*this,x,FLOAT_VAL_SPLIT_MAX()); + } + ParallelFloatObjective(FloatNum v) + : FloatMinimizeSpace(1.0), x(*this,v,v) {} + ParallelFloatObjective(ParallelFloatObjective& s) + : FloatMinimizeSpace(s) { + x.update(*this,s.x); + } + virtual Space* copy(void) { return new ParallelFloatObjective(*this); } + virtual FloatVar cost(void) const { return x; } + }; + + /// Scalar float minimization objective used for comparison and cut tests + class FloatMinObjective : public FloatMinimizeSpace { + public: + 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: + 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; } + }; + +#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&) {} + +#ifdef GECODE_HAS_THREADS + // A bound better by less than the step still replaces the incumbent. + Gecode::Search::TimeStop stop(5000); + Gecode::Search::Options o; + o.threads = 2; + o.stop = &stop; + ParallelFloatObjective* root = new ParallelFloatObjective; + std::unique_ptr + bab(Gecode::Search::babengine(root,o)); + delete root; + bab->constrain(ParallelFloatObjective(10.0)); + bab->constrain(ParallelFloatObjective(9.5)); + int found = 0; + bool substep = true; + while (Space* s = bab->next()) { + found++; + substep = substep && + (static_cast(s)->x.val().max() < 8.5); + delete s; + } + substep = substep && (found == 1) && !bab->stopped(); + if (!substep) + return false; +#endif + return true; + } + }; + + FloatObjectiveComparison float_objective_comparison; +#endif + + /// 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; + if ((compare_int(1,2,Sense::minimize) != SC_BETTER) || + (compare_int(2,1,Sense::minimize) != SC_WORSE) || + (compare_int(1,1,Sense::minimize) != SC_EQUIVALENT) || + (compare_int(2,1,Sense::maximize) != SC_BETTER) || + (compare_int(1,2,Sense::maximize) != SC_WORSE) || + (compare_int(1,1,Sense::maximize) != SC_EQUIVALENT)) + return false; + LexObjective lp(1,0,2), lq(2,0,2); + if ((lp.compare(lq) != SC_BETTER) || + (lq.compare(lp) != SC_WORSE)) + 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 { @@ -1276,13 +2011,13 @@ namespace Test { /// Number of threads unsigned int t; public: - /// Initialize test - RBS(const std::string& e, unsigned int t0) + /// Initialize test, enumerating all solutions without an objective + RBS(const std::string& e, unsigned int t0, HowToConstrain htc0=HTC_NONE) : Test("RBS::"+e+"::"+Model::name()+"::"+str(t0), - HTB_BINARY,HTB_BINARY,HTB_BINARY), t(t0) {} + HTB_BINARY,HTB_BINARY,HTB_BINARY,htc0), t(t0) {} /// Run test virtual bool run(void) { - Model* m = new Model(htb1,htb2,htb3); + Model* m = new Model(htb1,htb2,htb3,htc); Gecode::Search::FailStop f(2); Gecode::Search::Options o; o.threads = t; @@ -1292,16 +2027,30 @@ namespace Test { Gecode::RBS rbs(m,o); int n = m->solutions(); delete m; + // With an objective, every solution must improve on the previous one + Model* b = nullptr; + bool improving = true; while (true) { Model* s = rbs.next(); if (s != nullptr) { - n--; delete s; + n--; + if (htc == HTC_NONE) { + delete s; + } else { + if ((b != nullptr) && (s->compare(*b) != SC_BETTER)) + improving = false; + delete b; b = s; + } } if ((s == nullptr) && !rbs.stopped()) break; f.limit(f.limit()+2); } - return n == 0; + if (htc == HTC_NONE) + return n == 0; + bool ok = improving && (b != nullptr) && b->best(); + delete b; + return ok; } }; @@ -1496,6 +2245,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++) @@ -1566,7 +2316,7 @@ namespace Test { for (unsigned int t=1; t<=4; t++) { (void) new RBS("DFS",t); (void) new RBS("LDS",t); - (void) new RBS("BAB",t); + (void) new RBS("BAB",t,HTC_LEX_LE); (void) new RBS("DFS",t); (void) new RBS("LDS",t); (void) new RBS("BAB",t);