From 52f02233b410bf48479a2a63b703b1ed2c6dd2ec Mon Sep 17 00:00:00 2001 From: "lperron@google.com" Date: Mon, 9 May 2011 08:30:19 +0000 Subject: [PATCH] changed protected -> private in local search virtual methods; improved implementation of increasing element --- constraint_solver/constraint_solver.h | 24 ++-- constraint_solver/constraint_solveri.h | 36 +++--- constraint_solver/element.cc | 171 ++++++++++--------------- constraint_solver/local_search.cc | 65 +++++----- constraint_solver/routing.h | 2 + constraint_solver/search.cc | 2 +- 6 files changed, 141 insertions(+), 159 deletions(-) diff --git a/constraint_solver/constraint_solver.h b/constraint_solver/constraint_solver.h index d11e399d94..349116b29c 100644 --- a/constraint_solver/constraint_solver.h +++ b/constraint_solver/constraint_solver.h @@ -58,9 +58,10 @@ #ifndef CONSTRAINT_SOLVER_CONSTRAINT_SOLVER_H_ #define CONSTRAINT_SOLVER_CONSTRAINT_SOLVER_H_ -#include +#include #include #include +#include #include "base/callback.h" #include "base/commandlineflags.h" @@ -76,6 +77,11 @@ #include "base/random.h" class File; +template class ResultCallback2; +template class ResultCallback1; +template class Callback1; +template class ResultCallback; + using operations_research::WallTimer; #define ClockTimer WallTimer @@ -99,13 +105,13 @@ class DomainIntVar; class EqualityVarCstCache; class ExpressionCache; class GreaterEqualCstCache; -class IntervalVar; -class IntervalVarAssignmentProto; -class IntervalVarElement; class IntExpr; class IntVar; class IntVarAssignmentProto; class IntVarElement; +class IntervalVar; +class IntervalVarAssignmentProto; +class IntervalVarElement; class LessEqualCstCache; class LocalSearchFilter; class LocalSearchOperator; @@ -124,10 +130,10 @@ class SolutionCollector; class SolutionPool; class Solver; class SymmetryBreaker; -struct StateInfo; -struct Trail; class UnequalityVarCstCache; class VariableQueueCleaner; +struct StateInfo; +struct Trail; template class SimpleRevFIFO; // This enum is used internally to tag states in the search tree. @@ -709,7 +715,9 @@ class Solver { // Function based element. The constraint takes ownership of // callback. The callback must be monotonic. It must be able to // cope with any possible value in the domain of 'index' - // (potentially negative ones too). + // (potentially negative ones too). Furtermore, monotonicity is not + // checked. Thus giving a non monotonic function, or specifying an + // incorrect increasing parameter will result in undefined behavior. IntExpr* MakeMonotonicElement(IndexEvaluator1* values, bool increasing, IntVar* const index); @@ -3545,7 +3553,7 @@ class Pack : public Constraint { class SolutionPool : public BaseObject { public: SolutionPool() {} - ~SolutionPool() {} + virtual ~SolutionPool() {} // This method is called to initialize the solution pool with the assignment // from the local search. diff --git a/constraint_solver/constraint_solveri.h b/constraint_solver/constraint_solveri.h index f92cd21ac6..6ad8fc13f9 100644 --- a/constraint_solver/constraint_solveri.h +++ b/constraint_solver/constraint_solveri.h @@ -431,10 +431,6 @@ class IntVarLocalSearchOperator : public LocalSearchOperator { IntVar* Var(int64 index) const { return vars_[index]; } virtual bool SkipUnchanged(int index) const { return false; } protected: - // Called by Start() after synchronizing the operator with the current - // assignment. Should be overridden instead of Start() to avoid calling - // IntVarLocalSearchOperator::Start explicitly. - virtual void OnStart() {} int64 OldValue(int64 index) const { return old_values_[index]; } void SetValue(int64 index, int64 value); bool Activated(int64 index) const; @@ -444,6 +440,10 @@ class IntVarLocalSearchOperator : public LocalSearchOperator { void RevertChanges(bool incremental); void AddVars(const IntVar* const* vars, int size); private: + // Called by Start() after synchronizing the operator with the current + // assignment. Should be overridden instead of Start() to avoid calling + // IntVarLocalSearchOperator::Start explicitly. + virtual void OnStart() {} void MarkChange(int64 index); scoped_array vars_; @@ -496,7 +496,7 @@ class BaseLNS : public IntVarLocalSearchOperator { virtual bool MakeNextNeighbor(Assignment* delta, Assignment* deltadelta); virtual void InitFragments(); virtual bool NextFragment(vector* fragment) = 0; - protected: + private: // This method should not be overridden. Override InitFragments() instead. virtual void OnStart(); }; @@ -514,9 +514,9 @@ class ChangeValue : public IntVarLocalSearchOperator { virtual ~ChangeValue(); virtual bool MakeNextNeighbor(Assignment* delta, Assignment* deltadelta); virtual int64 ModifyValue(int64 index, int64 value) = 0; - protected: - virtual void OnStart(); private: + virtual void OnStart(); + int index_; }; @@ -565,11 +565,6 @@ class PathOperator : public IntVarLocalSearchOperator { // Number of next variables. int number_of_nexts() const { return number_of_nexts_; } protected: - virtual void OnStart(); - // Called by OnStart() after initializing node information. Should be - // overriden instead of OnStart() to avoid calling PathOperator::OnStart - // explicitly. - virtual void OnNodeInitialization() {} // Returns the index of the variable corresponding to the ith base node. int64 BaseNode(int i) const { return base_nodes_[i]; } int64 StartNode(int i) const { return path_starts_[base_paths_[i]]; } @@ -620,10 +615,16 @@ class PathOperator : public IntVarLocalSearchOperator { void ResetPosition() { just_started_ = true; } - protected: + const int number_of_nexts_; const bool ignore_path_vars_; private: + virtual void OnStart(); + // Called by OnStart() after initializing node information. Should be + // overriden instead of OnStart() to avoid calling PathOperator::OnStart + // explicitly. + virtual void OnNodeInitialization() {} + bool CheckEnds() const; bool IncrementPosition(); void InitializePathStarts(); @@ -668,13 +669,12 @@ class IntVarLocalSearchFilter : public LocalSearchFilter { public: IntVarLocalSearchFilter(const IntVar* const* vars, int size); ~IntVarLocalSearchFilter(); - protected: - // Add variables to "track" to the filter. - void AddVars(const IntVar* const* vars, int size); // This method should not be overridden. Override OnSynchronize() instead // which is called before exiting this method. virtual void Synchronize(const Assignment* assignment); - virtual void OnSynchronize() {} + protected: + // Add variables to "track" to the filter. + void AddVars(const IntVar* const* vars, int size); bool FindIndex(const IntVar* const var, int64* index) const { DCHECK(index != NULL); return FindCopy(var_to_index_, var, index); @@ -683,6 +683,8 @@ class IntVarLocalSearchFilter : public LocalSearchFilter { IntVar* Var(int index) const { return vars_[index]; } int64 Value(int index) const { return values_[index]; } private: + virtual void OnSynchronize() {} + scoped_array vars_; scoped_array values_; int size_; diff --git a/constraint_solver/element.cc b/constraint_solver/element.cc index d76ddbeec6..220cbede12 100644 --- a/constraint_solver/element.cc +++ b/constraint_solver/element.cc @@ -533,17 +533,13 @@ IntExprFunctionElement::~IntExprFunctionElement() { // ----- Increasing Element ----- -class MonotonicIntExprFunctionElement : public BaseIntExpr { +class IncreasingIntExprFunctionElement : public BaseIntExpr { public: - MonotonicIntExprFunctionElement(Solver* const s, + IncreasingIntExprFunctionElement(Solver* const s, ResultCallback1* values, - bool increasing, - bool to_delete, IntVar* const index) : BaseIntExpr(s), values_(values), - increasing_(increasing), - delete_(to_delete), index_(index) { DCHECK(values); DCHECK(index); @@ -551,124 +547,75 @@ class MonotonicIntExprFunctionElement : public BaseIntExpr { values->CheckIsRepeatable(); } - virtual ~MonotonicIntExprFunctionElement() { - if (delete_) { - delete values_; - } + virtual ~IncreasingIntExprFunctionElement() { + delete values_; } virtual int64 Min() const { - return increasing_? - values_->Run(index_->Min()): - values_->Run(index_->Max()); + return values_->Run(index_->Min()); } virtual void SetMin(int64 m) { const int64 expression_min = index_->Min(); const int64 expression_max = index_->Max(); - if (increasing_) { - if (m > values_->Run(expression_max)) { - solver()->Fail(); - } - int64 nmin = expression_min; - while (nmin <= expression_max && values_->Run(nmin) < m) { - nmin++; - } - DCHECK_LE(nmin, expression_max); - index_->SetMin(nmin); - } else { - if (m > values_->Run(expression_min)) { - solver()->Fail(); - } - int64 nmax = expression_max; - while (nmax >= expression_min && values_->Run(nmax) < m) { - nmax--; - } - DCHECK_GE(nmax, expression_min); - index_->SetMax(nmax); + if (m > values_->Run(expression_max)) { + solver()->Fail(); } + int64 nmin = expression_min; + while (nmin <= expression_max && values_->Run(nmin) < m) { + nmin++; + } + DCHECK_LE(nmin, expression_max); + index_->SetMin(nmin); } virtual int64 Max() const { - return increasing_? - values_->Run(index_->Max()): - values_->Run(index_->Min()); + return values_->Run(index_->Max()); } virtual void SetMax(int64 m) { const int64 expression_min = index_->Min(); const int64 expression_max = index_->Max(); - if (increasing_) { - if (m < values_->Run(expression_min)) { - solver()->Fail(); - } - int64 nmax = expression_max; - while (nmax >= expression_min && values_->Run(nmax) > m) { - nmax--; - } - DCHECK_GE(nmax, expression_min); - index_->SetMax(nmax); - } else { - if (m < values_->Run(expression_max)) { - solver()->Fail(); - } - int64 nmin = expression_min; - while (nmin <= expression_max && values_->Run(nmin) > m) { - nmin++; - } - DCHECK_LE(nmin, expression_max); - index_->SetMin(nmin); + if (m < values_->Run(expression_min)) { + solver()->Fail(); } + int64 nmax = expression_max; + while (nmax >= expression_min && values_->Run(nmax) > m) { + nmax--; + } + DCHECK_GE(nmax, expression_min); + index_->SetMax(nmax); } virtual void SetRange(int64 mi, int64 ma) { const int64 expression_min = index_->Min(); const int64 expression_max = index_->Max(); - if (increasing_) { - if (mi > ma || - ma < values_->Run(expression_min) || - mi > values_->Run(expression_max)) { - solver()->Fail(); - } - int64 nmax = expression_max; - while (nmax >= expression_min && values_->Run(nmax) > ma) { - nmax--; - } - DCHECK_GE(nmax, expression_min); - int64 nmin = expression_min; - while (nmin <= nmax && values_->Run(nmin) < mi) { - nmin++; - } - DCHECK_LE(nmin, nmax); - index_->SetRange(nmin, nmax); - } else { - if (mi > ma || - ma < values_->Run(expression_max) || - mi > values_->Run(expression_min)) { - solver()->Fail(); - } - int64 nmin = expression_min; - while (nmin <= expression_max && values_->Run(nmin) > ma) { - nmin++; - } - DCHECK_LE(nmin, expression_max); - int64 nmax = expression_max; - while (nmax >= expression_min && values_->Run(nmax) < mi) { - nmax--; - } - DCHECK_GE(nmax, nmin); - index_->SetRange(nmin, nmax); + if (mi > ma || + ma < values_->Run(expression_min) || + mi > values_->Run(expression_max)) { + solver()->Fail(); } + int64 nmax = expression_max; + while (nmax >= expression_min && values_->Run(nmax) > ma) { + nmax--; + } + DCHECK_GE(nmax, expression_min); + int64 nmin = expression_min; + while (nmin <= nmax && values_->Run(nmin) < mi) { + nmin++; + } + DCHECK_LE(nmin, nmax); + index_->SetRange(nmin, nmax); } virtual string name() const { - return StringPrintf("MonotonicIntExprFunctionElement(values, %d, %s)", - increasing_, index_->name().c_str()); + return StringPrintf("IncreasingIntExprFunctionElement(values, %s)", + index_->name().c_str()); } virtual string DebugString() const { - return StringPrintf("MonotonicIntExprFunctionElement(values, %d, %s)", - increasing_, index_->DebugString().c_str()); + return StringPrintf("IncreasingIntExprFunctionElement(values, %s)", + index_->DebugString().c_str()); } virtual void WhenRange(Demon* d) { @@ -676,8 +623,6 @@ class MonotonicIntExprFunctionElement : public BaseIntExpr { } private: ResultCallback1* values_; - const bool increasing_; - const bool delete_; IntVar* const index_; }; @@ -687,15 +632,39 @@ IntExpr* Solver::MakeElement(ResultCallback1* values, return RevAlloc(new IntExprFunctionElement(this, values, index, true)); } +namespace { +class OppositeCallback : public BaseObject { + public: + OppositeCallback(ResultCallback1* const values) + : values_(values) { + CHECK_NOTNULL(values_); + values_->CheckIsRepeatable(); + } + + virtual ~OppositeCallback() {} + + int64 Run(int64 index) { + return -values_->Run(index); + } + public: + scoped_ptr > values_; +}; +} // namespace + IntExpr* Solver::MakeMonotonicElement(ResultCallback1* values, bool increasing, IntVar* const index) { CHECK_EQ(this, index->solver()); - return RevAlloc(new MonotonicIntExprFunctionElement(this, - values, - increasing, - true, - index)); + if (increasing) { + return RevAlloc(new IncreasingIntExprFunctionElement(this, values, index)); + } else { + OppositeCallback* const opposite_callback = + RevAlloc(new OppositeCallback(values)); + ResultCallback1* opposite_values = + NewPermanentCallback(opposite_callback, &OppositeCallback::Run); + return MakeOpposite(RevAlloc( + new IncreasingIntExprFunctionElement(this, opposite_values, index))); + } } // ----- IntIntExprFunctionElement ----- diff --git a/constraint_solver/local_search.cc b/constraint_solver/local_search.cc index ac478bc6bc..dfa3f417f3 100644 --- a/constraint_solver/local_search.cc +++ b/constraint_solver/local_search.cc @@ -321,22 +321,6 @@ class MoveTowardTargetLS: public IntVarLocalSearchOperator { } virtual ~MoveTowardTargetLS() {} - virtual void OnStart() { - // Do not change the value of variable_index_: this way, we keep going from - // where we last modified something. This is because we expect that most - // often, the variables we have just checked are less likely to be able - // to be changed to their target values than the ones we have not yet - // checked. - // - // Consider the case where oddly indexed variables can be assigned to their - // target values (no matter in what order they are considered), while even - // indexed ones cannot. Restarting at index 0 each time an odd-indexed - // variable is modified will cause a total of Theta(n^2) neighbors to be - // generated, while not restarting will produce only Theta(n) neighbors. - CHECK_GE(variable_index_, 0); - CHECK_LT(variable_index_, Size()); - num_var_since_last_start_ = 0; - } // Make a neighbor assigning one variable to its target value. virtual bool MakeNextNeighbor(Assignment* delta, Assignment* deltadelta) { @@ -358,6 +342,23 @@ class MoveTowardTargetLS: public IntVarLocalSearchOperator { } private: + virtual void OnStart() { + // Do not change the value of variable_index_: this way, we keep going from + // where we last modified something. This is because we expect that most + // often, the variables we have just checked are less likely to be able + // to be changed to their target values than the ones we have not yet + // checked. + // + // Consider the case where oddly indexed variables can be assigned to their + // target values (no matter in what order they are considered), while even + // indexed ones cannot. Restarting at index 0 each time an odd-indexed + // variable is modified will cause a total of Theta(n^2) neighbors to be + // generated, while not restarting will produce only Theta(n) neighbors. + CHECK_GE(variable_index_, 0); + CHECK_LT(variable_index_, Size()); + num_var_since_last_start_ = 0; + } + // Target values const vector target_; @@ -699,9 +700,9 @@ class TwoOpt : public PathOperator { virtual ~TwoOpt() {} virtual bool MakeNeighbor(); virtual bool IsIncremental() const { return true; } - protected: - virtual void OnNodeInitialization() { last_ = -1; } private: + virtual void OnNodeInitialization() { last_ = -1; } + int64 last_base_; int64 last_; }; @@ -879,9 +880,9 @@ class MakeActiveOperator : public PathOperator { virtual ~MakeActiveOperator() {} virtual bool MakeNextNeighbor(Assignment* delta, Assignment* deltadelta); virtual bool MakeNeighbor(); - protected: - virtual void OnNodeInitialization(); private: + virtual void OnNodeInitialization(); + int inactive_node_; }; @@ -957,9 +958,9 @@ class SwapActiveOperator : public PathOperator { virtual ~SwapActiveOperator() {} virtual bool MakeNextNeighbor(Assignment* delta, Assignment* deltadelta); virtual bool MakeNeighbor(); - protected: - virtual void OnNodeInitialization(); private: + virtual void OnNodeInitialization(); + int inactive_node_; }; @@ -1018,9 +1019,9 @@ class ExtendedSwapActiveOperator : public PathOperator { virtual ~ExtendedSwapActiveOperator() {} virtual bool MakeNextNeighbor(Assignment* delta, Assignment* deltadelta); virtual bool MakeNeighbor(); - protected: - virtual void OnNodeInitialization(); private: + virtual void OnNodeInitialization(); + int inactive_node_; }; @@ -1394,9 +1395,9 @@ class LinKernighan : public PathOperator { bool topt); virtual ~LinKernighan(); virtual bool MakeNeighbor(); - protected: - virtual void OnNodeInitialization(); private: + virtual void OnNodeInitialization(); + static const int kNeighbors; bool InFromOut(int64 in_i, int64 in_j, int64* out, int64* gain); @@ -2143,7 +2144,6 @@ class ObjectiveFilter : public IntVarLocalSearchFilter { LSOperation* op); virtual ~ObjectiveFilter(); virtual bool Accept(const Assignment* delta, const Assignment* deltadelta); - virtual void OnSynchronize(); virtual int64 SynchronizedElementValue(int64 index) = 0; virtual bool EvaluateElementValue(const Assignment::IntContainer& container, int index, @@ -2151,11 +2151,6 @@ class ObjectiveFilter : public IntVarLocalSearchFilter { int64* obj_value) = 0; virtual bool IsIncremental() const { return true; } protected: - int64 Evaluate(const Assignment* delta, - int64 current_value, - const int64* const out_values, - bool cache_delta_values); - const int primary_vars_size_; int64* const cache_; int64* const delta_cache_; @@ -2165,6 +2160,12 @@ class ObjectiveFilter : public IntVarLocalSearchFilter { int64 old_value_; int64 old_delta_value_; bool incremental_; + private: + virtual void OnSynchronize(); + int64 Evaluate(const Assignment* delta, + int64 current_value, + const int64* const out_values, + bool cache_delta_values); }; ObjectiveFilter::ObjectiveFilter(const IntVar* const* vars, diff --git a/constraint_solver/routing.h b/constraint_solver/routing.h index ced6b07715..94389fa2fd 100644 --- a/constraint_solver/routing.h +++ b/constraint_solver/routing.h @@ -26,7 +26,9 @@ #ifndef CONSTRAINT_SOLVER_ROUTING_H_ #define CONSTRAINT_SOLVER_ROUTING_H_ +#include #include +#include #include #include "base/commandlineflags.h" diff --git a/constraint_solver/search.cc b/constraint_solver/search.cc index 85211be988..483b1f5856 100644 --- a/constraint_solver/search.cc +++ b/constraint_solver/search.cc @@ -97,7 +97,7 @@ bool SearchLog::AtSolution() { objective_updated = true; } else if (var_!= NULL) { current = var_->Value(); - StringAppendF(&obj_str, "%" GG_LL_FORMAT "d", current); + StringAppendF(&obj_str, "%" GG_LL_FORMAT "d, ", current); objective_updated = true; } if (objective_updated) {