diff --git a/changelog.in b/changelog.in index 394b936a5d..82f35d7e4e 100755 --- a/changelog.in +++ b/changelog.in @@ -73,6 +73,27 @@ Date: unreleased [DESCRIPTION] This is the development changelog for the next Gecode release. +[ENTRY] +Module: kernel +What: new +Rank: minor +[DESCRIPTION] +Add Space::NoGPI for scoped posting without globally retained propagator +information. Scoped propagators keep their identifiers and groups in the +space, have zero AFC, and copy their information when cloned. Rewriting +preserves the original propagator information. Space::UseGPI enables global +information for selected posting calls inside a NoGPI scope. Both contexts +restore the previous setting when they leave scope. + +[ENTRY] +Module: int +What: bug +Rank: minor +[DESCRIPTION] +Fix iterator invalidation when counting-based search removes expired +propagator cache entries. The invalid iterator could skip remaining entries +and leave the brancher without a choice. + [ENTRY] Module: int diff --git a/gecode/int/branch/cbs.hpp b/gecode/int/branch/cbs.hpp index 6c21bcf1f4..263d437428 100755 --- a/gecode/int/branch/cbs.hpp +++ b/gecode/int/branch/cbs.hpp @@ -191,13 +191,15 @@ namespace Gecode { namespace Int { namespace Branch { // We delete unvisited propagators from the log and look for the highest // solution density across all propagators. - for (const auto& kv : logProp) { - unsigned int prop_id = kv.first; - const PropInfo& info = kv.second; - if (!info.visited) - logProp.erase(prop_id); - else if (info.dens > maxSD.dens) - maxSD = {info.var_id, info.val, info.dens}; + for (auto i = logProp.begin(); i != logProp.end(); ) { + if (!i->second.visited) { + i = logProp.erase(i); + } else { + const PropInfo& info = i->second; + if (info.dens > maxSD.dens) + maxSD = {info.var_id, info.val, info.dens}; + ++i; + } } assert(maxSD.dens != -1); diff --git a/gecode/kernel/core.cpp b/gecode/kernel/core.cpp index 4293cfe6a8..63b03c7594 100644 --- a/gecode/kernel/core.cpp +++ b/gecode/kernel/core.cpp @@ -118,7 +118,7 @@ namespace Gecode { VarImpDisposerBase* Space::vd[AllVarConf::idx_d]; #endif - Space::Space(void) : mm(ssd.data().sm) { + Space::Space(void) : mm(ssd.data().sm), no_gpi(false) { #ifdef GECODE_HAS_CBS var_id_counter = 0; #endif @@ -540,7 +540,8 @@ namespace Gecode { // Process failure failed: // Count failure - ssd.data().gpi.fail(p->gpi()); + if (!p->local_info()) + ssd.data().gpi.fail(p->gpi()); // Mark as failed fail(); // Propagate top priority propagators @@ -727,7 +728,7 @@ namespace Gecode { #ifdef GECODE_HAS_CBS var_id_counter(s.var_id_counter), #endif - d_fst(&Actor::sentinel),d_cur(nullptr),d_lst(nullptr) { + no_gpi(false), d_fst(&Actor::sentinel),d_cur(nullptr),d_lst(nullptr) { #ifdef GECODE_HAS_VAR_DISPOSE for (int i=0; i= 4, + "Propagator information needs two tag bits"); + Kernel::GPI::Info* i = alloc(1); + i->init(ssd.data().gpi.next_pid(),gid); + i->afc = 0.0; + return Support::ptrjoin(i,2); + } + void Space::afc_unshare(void) { if (ssd.data().gpi.unshare()) { for (Propagators ps(*this); ps(); ++ps) { Propagator& p = ps.propagator(); + if (p.local_info()) + continue; Kernel::GPI::Info* gpi = ssd.data().gpi.allocate(p.gpi().pid,p.gpi().gid); if (p.disabled()) diff --git a/gecode/kernel/core.hpp b/gecode/kernel/core.hpp index 169aab6599..7ad7e9abc0 100755 --- a/gecode/kernel/core.hpp +++ b/gecode/kernel/core.hpp @@ -1087,12 +1087,19 @@ namespace Gecode { /// A list of advisors (used during cloning) Gecode::ActorLink* advisors; } u; - /// A tagged pointer combining a pointer to global propagator information and whether the propagator is disabled + /** + * \brief Tagged pointer to propagator information + * + * Bit zero marks disabled propagators; bit one marks space-local + * information, which has no accumulated failure count. + */ void* gpi_disabled; /// Static cast for a non-null pointer (to give a hint to optimizer) static Propagator* cast(ActorLink* al); /// Static cast for a non-null pointer (to give a hint to optimizer) static const Propagator* cast(const ActorLink* al); + /// Whether propagator information is local to the space + bool local_info(void) const; /// Disable propagator void disable(Space& home); /// Enable propagator @@ -1104,7 +1111,7 @@ namespace Gecode { Propagator(Space& home, Propagator& p); /// Return forwarding pointer during copying Propagator* fwd(void) const; - /// Provide access to global propagator information + /// Provide access to propagator information Kernel::GPI::Info& gpi(void); public: @@ -1791,6 +1798,8 @@ namespace Gecode { friend GECODE_KERNEL_EXPORT void trace(Home home, TraceFilter tf, int te, Tracer& t); private: + /// Allocate tagged space-local propagator information + GECODE_KERNEL_EXPORT void* local_info(unsigned int gid); /// Shared data for space Kernel::SharedSpaceData ssd; /// Performs memory management for space @@ -1799,6 +1808,8 @@ namespace Gecode { /// Global counter for variable ids unsigned int var_id_counter; #endif + /// Whether new propagator information is local to this space + bool no_gpi; /// Doubly linked list of all propagators ActorLink pl; /// Doubly linked list of all branchers @@ -2037,6 +2048,68 @@ namespace Gecode { GECODE_KERNEL_EXPORT void ap_ignore_dispose(Actor* a, bool d); public: + /** + * \brief Suppress global propagator information during scoped posting + * \ingroup TaskModelScript + * + * New propagators posted into this space use space-local information + * instead of records retained by all spaces in a search. Their AFC is + * always zero, including after AFC initialization or unsharing. They + * retain normal identifiers and groups, so tracing and counting-based + * search remain supported. Group changes for these propagators are + * local to the space; clones copy the current group. Local records are + * reclaimed with the space, even if their propagators subsume earlier. + * AFC-based branching therefore only counts ordinary propagators. + * + * Rewriting inherits the original propagator's information, regardless + * of this context. Existing propagators are unaffected. Clones retain + * local information but start with ordinary posting enabled. + * + * A nested UseGPI context enables global information for selected + * posting calls. Both contexts restore the previous setting on destruction, + * including exception unwinding. The space must outlive the context, + * and contexts for the same space must be destroyed in reverse order. + */ + class NoGPI { + private: + /// Space in which to suppress global information + Space& home; + /// Previous setting + bool previous; + NoGPI(const NoGPI&) = delete; + NoGPI& operator =(const NoGPI&) = delete; + public: + /// Suppress global information for new propagators in \a home + explicit NoGPI(Space& home); + /// Restore the previous setting + ~NoGPI(void); + }; + + /** + * \brief Enable global propagator information during scoped posting + * \ingroup TaskModelScript + * + * Overrides an enclosing NoGPI context for new propagators posted into + * this space. Rewriting still inherits the original information. + * Destruction restores the previous setting, including exception + * unwinding. The space must outlive the context, and NoGPI and UseGPI + * contexts for the same space must be destroyed in reverse order. + */ + class UseGPI { + private: + /// Space in which to enable global information + Space& home; + /// Previous setting + bool previous; + UseGPI(const UseGPI&) = delete; + UseGPI& operator =(const UseGPI&) = delete; + public: + /// Enable global information for new propagators in \a home + explicit UseGPI(Space& home); + /// Restore the previous setting + ~UseGPI(void); + }; + /** * \brief Default constructor * \ingroup TaskModelScript @@ -3556,6 +3629,28 @@ namespace Gecode { } + forceinline + Space::NoGPI::NoGPI(Space& h) : home(h), previous(h.no_gpi) { + home.no_gpi = true; + } + + forceinline + Space::NoGPI::~NoGPI(void) { + home.no_gpi = previous; + } + + + forceinline + Space::UseGPI::UseGPI(Space& h) : home(h), previous(h.no_gpi) { + home.no_gpi = false; + } + + forceinline + Space::UseGPI::~UseGPI(void) { + home.no_gpi = previous; + } + + /* * Propagator * @@ -3581,6 +3676,11 @@ namespace Gecode { return static_cast(prev()); } + forceinline bool + Propagator::local_info(void) const { + return (reinterpret_cast(gpi_disabled) & 2) != 0; + } + forceinline bool Propagator::disabled(void) const { return Support::marked(gpi_disabled); @@ -3600,7 +3700,8 @@ namespace Gecode { forceinline Kernel::GPI::Info& Propagator::gpi(void) { - return *static_cast(Support::funmark(gpi_disabled)); + return *reinterpret_cast + (reinterpret_cast(gpi_disabled) & ~static_cast(3)); } forceinline @@ -3608,6 +3709,9 @@ namespace Gecode { : gpi_disabled((home.propagator() != nullptr) ? // Inherit propagator information home.propagator()->gpi_disabled : + static_cast(home).no_gpi ? + static_cast(home).local_info + (home.propagatorgroup().gid) : // New propagator information static_cast(home).ssd.data().gpi.allocate (home.propagatorgroup().gid)) { @@ -3617,8 +3721,13 @@ namespace Gecode { } forceinline - Propagator::Propagator(Space&, Propagator& p) + Propagator::Propagator(Space& home, Propagator& p) : gpi_disabled(p.gpi_disabled) { + if (p.local_info()) { + Kernel::GPI::Info* i = home.alloc(1); + *i = p.gpi(); + gpi_disabled = Support::ptrjoin(i, p.disabled() ? 3 : 2); + } u.advisors = nullptr; assert((u.med == 0) && (u.size == 0)); // Set forwarding pointer diff --git a/gecode/kernel/gpi.hpp b/gecode/kernel/gpi.hpp index bca28ef4ee..56d18353eb 100755 --- a/gecode/kernel/gpi.hpp +++ b/gecode/kernel/gpi.hpp @@ -92,6 +92,8 @@ namespace Gecode { namespace Kernel { Info* allocate(unsigned int p, unsigned int gid); /// Allocate new actor info Info* allocate(unsigned int gid); + /// Allocate a new propagator id without allocating information + unsigned int next_pid(void); /// Return next free propagator id unsigned int pid(void) const; /// Provide access to unshare info and set to true @@ -141,6 +143,11 @@ namespace Gecode { namespace Kernel { return d; } + forceinline unsigned int + GPI::next_pid(void) { + return npid.fetch_add(1, std::memory_order_seq_cst); + } + forceinline unsigned int GPI::pid(void) const { return npid.load(std::memory_order_acquire); @@ -186,7 +193,7 @@ namespace Gecode { namespace Kernel { } c = &b->info[--b->free]; m.release(); - c->init(npid.fetch_add(1, std::memory_order_seq_cst),gid); + c->init(next_pid(),gid); return c; } diff --git a/test/afc.cpp b/test/afc.cpp index eceff77137..75c2769555 100644 --- a/test/afc.cpp +++ b/test/afc.cpp @@ -36,6 +36,8 @@ #include "test/test.hh" +#include + namespace Test { /// %Test for %AFC infrastructure @@ -132,6 +134,230 @@ namespace Test { } }; + /// Scoped suppression of global propagator information + class NoGPI : public Test::Base { + /// Space with variables for posting and rewriting relations + class TestSpace : public Gecode::Space { + public: + Gecode::IntVar x, y; + Gecode::BoolVar b; + TestSpace(void) : x(*this,0,10), y(*this,0,10), b(*this,0,1) {} + TestSpace(TestSpace& s) : Space(s) { + x.update(*this,s.x); y.update(*this,s.y); b.update(*this,s.b); + } + virtual Space* copy(void) { return new TestSpace(*this); } + /// Find a surviving propagator by its identifier + Gecode::Propagator* find(unsigned int id) { + for (Propagators ps(*this); ps(); ++ps) + if (ps.propagator().id() == id) + return &ps.propagator(); + return nullptr; + } + /// Post a non-subsumed relation and return its propagator + Gecode::Propagator& post(Gecode::PropagatorGroup g = + Gecode::PropagatorGroup::def) { + Gecode::rel((*this)(g), x, Gecode::IRT_LE, y); + Gecode::Propagator* p = nullptr; + for (Propagators ps(*this); ps(); ++ps) + if ((p == nullptr) || (ps.propagator().id() > p->id())) + p = &ps.propagator(); + return *p; + } + }; + + /// Check nesting, unwinding, isolation, and posting in a clone + bool scopes(void) { + using namespace Gecode; + TestSpace s, other; + if (s.post().afc() != 1.0) + return false; + { + Space::NoGPI outer(s); + if ((s.post().afc() != 0.0) || (other.post().afc() != 1.0)) + return false; + try { + Space::NoGPI inner(s); + if (s.post().afc() != 0.0) + return false; + throw 1; + } catch (int) {} + if (s.post().afc() != 0.0) + return false; + { + Space::UseGPI use(s); + if (s.post().afc() != 1.0) + return false; + { + Space::UseGPI nested(s); + if (s.post().afc() != 1.0) + return false; + } + { + Space::NoGPI suppress(s); + if (s.post().afc() != 0.0) + return false; + } + if (s.post().afc() != 1.0) + return false; + } + if (s.post().afc() != 0.0) + return false; + try { + Space::UseGPI use(s); + if (s.post().afc() != 1.0) + return false; + throw 1; + } catch (int) {} + if (s.post().afc() != 0.0) + return false; + (void) s.status(); + std::unique_ptr c(static_cast(s.clone())); + if (c->post().afc() != 1.0) + return false; + } + try { + Space::NoGPI context(s); + throw 1; + } catch (int) {} + return s.post().afc() == 1.0; + } + + /// Check local groups, disabled pointer tags, and lifetime after cloning + bool lifetime(void) { + using namespace Gecode; + std::unique_ptr s(new TestSpace); + PropagatorGroup a, b; + unsigned int id; + { + Space::NoGPI context(*s); + Propagator& p = s->post(a); + id = p.id(); + if ((p.group() != a) || (s->post().id() == id)) + return false; + } + a.disable(*s); + (void) s->status(); + std::unique_ptr c(static_cast(s->clone())); + // The first call marks the shared pool; the second really unshares. + s->afc_unshare(); + c->afc_unshare(); + Propagator* p = c->find(id); + if (!p || !p->disabled() || (p->afc() != 0.0)) + return false; + b.move(*c,id); + if ((p->group() != b) || (s->find(id)->group() != a)) + return false; + s.reset(); + b.enable(*c); + if (p->disabled() || (c->status() == SS_FAILED)) + return false; + b.kill(*c); + return (c->find(id) == nullptr) && (b.size(*c) == 0); + } + + /// Check AFC failure accounting, initialization, and rescaling + bool failure(bool local) { + using namespace Gecode; + TestSpace s; + Propagator* p; + if (local) { + Space::NoGPI context(s); + p = &s.post(); + } else { + p = &s.post(); + } + (void) s.status(); + std::unique_ptr c(static_cast(s.clone())); + // Force rescaling on failure for the ordinary propagator. + IntAFC afc(*c,IntVarArgs() << c->x << c->y,1.0e-300,false); + c->afc_unshare(); + Propagator* q = c->find(p->id()); + if (!q || (q->afc() != (local ? 0.0 : 1.0))) + return false; + rel(*c,c->x,IRT_EQ,5); + rel(*c,c->y,IRT_EQ,4); + if (c->status() != SS_FAILED) + return false; + return local ? ((q->afc() == 0.0) && (p->afc() == 0.0)) : + ((q->afc() > 1.0) && (p->afc() < 1.0)); + } + + /// A real reified propagator must retain its information when rewritten + bool rewrite(bool local) { + using namespace Gecode; + TestSpace s; + PropagatorGroup g; + if (local) { + Space::NoGPI context(s); + rel(s(g),s.x,IRT_EQ,s.y,Reify(s.b)); + } else { + rel(s(g),s.x,IRT_EQ,s.y,Reify(s.b)); + } + (void) s.status(); + Gecode::Propagators ps(s,PropagatorGroup::all); + unsigned int id = ps.propagator().id(); + rel(s,s.b,IRT_EQ,1); + if (local) { + Space::UseGPI context(s); + (void) s.status(); + } else { + Space::NoGPI context(s); + (void) s.status(); + } + Propagator* p = s.find(id); + if (!p || (p->group() != g) || + (p->afc() != (local ? 0.0 : 1.0))) + return false; + rel(s,s.x,IRT_EQ,3); + return (s.status() == SS_SOLVED) && s.y.assigned() && (s.y.val() == 3); + } + + /// Collect posting counts and propagated identifiers + class Tracer : public Gecode::Tracer { + public: + unsigned int posted = 0; + std::vector ids; + virtual void propagate(const Gecode::Space&, + const Gecode::PropagateTraceInfo& p) { + ids.push_back(p.id()); + } + virtual void commit(const Gecode::Space&, + const Gecode::CommitTraceInfo&) {} + virtual void post(const Gecode::Space&, + const Gecode::PostTraceInfo& p) { + posted += p.propagators(); + } + }; + + /// Posting traces and propagation traces must retain distinct identities + bool tracing(void) { + using namespace Gecode; + Tracer t; + TestSpace s; + PropagatorGroup g; + trace(s,g,TE_POST | TE_PROPAGATE,t); + t.posted = 0; + unsigned int a, b; + { + Space::NoGPI context(s); + a = s.post(g).id(); + b = s.post(g).id(); + } + (void) s.status(); + return (t.posted == 2) && (a != b) && + (std::find(t.ids.begin(),t.ids.end(),a) != t.ids.end()) && + (std::find(t.ids.begin(),t.ids.end(),b) != t.ids.end()); + } + public: + NoGPI(void) : Test::Base("AFC::NoGPI") {} + virtual bool run(void) { + return scopes() && lifetime() && failure(false) && failure(true) && + rewrite(false) && rewrite(true) && tracing(); + } + }; + + NoGPI no_gpi; + AFC afc; } diff --git a/test/branch/int.cpp b/test/branch/int.cpp index 33837242ec..ab0ab575c3 100644 --- a/test/branch/int.cpp +++ b/test/branch/int.cpp @@ -36,6 +36,8 @@ #include #include +#include + namespace Test { namespace Branch { /// Space for testing filtered random tie selection @@ -131,6 +133,62 @@ namespace Test { namespace Branch { FilteredRndTies filtered_rnd_ties; +#ifdef GECODE_HAS_CBS + /// Independent permutations leave one CBS cache entry after another expires + class CBSSpace : public Gecode::Space { + public: + Gecode::IntVarArray x; + CBSSpace(bool local) : x(*this,8,0,3) { + if (local) { + NoGPI suppress(*this); + post(); + } else { + post(); + } + Gecode::cbsbranch(*this,x); + } + void post(void) { + using namespace Gecode; + distinct(*this,IntVarArgs() << x[0] << x[1] << x[2] << x[3],IPL_VAL); + distinct(*this,IntVarArgs() << x[4] << x[5] << x[6] << x[7],IPL_VAL); + } + CBSSpace(CBSSpace& s) : Space(s) { + x.update(*this,s.x); + } + virtual Space* copy(void) { + return new CBSSpace(*this); + } + }; + + /// Cache removal must preserve choices from surviving propagators + class CBS : public Base { + public: + CBS(void) : Base("Int::Branch::CBS",TestTags::check()) {} + virtual bool run(void) { + using namespace Gecode; + for (bool local : {false,true}) { + CBSSpace root(local); + Search::Options o; + o.c_d = 2; + o.a_d = 1; + DFS engine(&root,o); + int n = 0; + while (std::unique_ptr s{engine.next()}) { + for (int i=0; ix.size(); i++) + if (!s->x[i].assigned()) + return false; + n++; + } + if (n != 24*24) + return false; + } + return true; + } + }; + + CBS cbs; +#endif + /// %Test brancher with distinct propagator class Int : public IntTest { public: