diff --git a/ortools/algorithms/dense_doubly_linked_list.h b/ortools/algorithms/dense_doubly_linked_list.h index f741fbcc13..c318e1507a 100644 --- a/ortools/algorithms/dense_doubly_linked_list.h +++ b/ortools/algorithms/dense_doubly_linked_list.h @@ -15,6 +15,7 @@ #define OR_TOOLS_ALGORITHMS_DENSE_DOUBLY_LINKED_LIST_H_ #include + #include "ortools/base/logging.h" namespace operations_research { diff --git a/ortools/algorithms/dynamic_partition.h b/ortools/algorithms/dynamic_partition.h index 6bf5ab04e4..851ebaa075 100644 --- a/ortools/algorithms/dynamic_partition.h +++ b/ortools/algorithms/dynamic_partition.h @@ -32,6 +32,7 @@ #include #include + #include "ortools/base/logging.h" namespace operations_research { diff --git a/ortools/algorithms/dynamic_permutation.cc b/ortools/algorithms/dynamic_permutation.cc index 326f58d3bf..afcceb525a 100644 --- a/ortools/algorithms/dynamic_permutation.cc +++ b/ortools/algorithms/dynamic_permutation.cc @@ -14,6 +14,7 @@ #include "ortools/algorithms/dynamic_permutation.h" #include + #include "ortools/algorithms/sparse_permutation.h" namespace operations_research { diff --git a/ortools/algorithms/hungarian.h b/ortools/algorithms/hungarian.h index 3ec8c8a20d..6b7711edef 100644 --- a/ortools/algorithms/hungarian.h +++ b/ortools/algorithms/hungarian.h @@ -34,6 +34,7 @@ #define OR_TOOLS_ALGORITHMS_HUNGARIAN_H_ #include + #include "absl/container/flat_hash_map.h" namespace operations_research { diff --git a/ortools/algorithms/knapsack_solver.h b/ortools/algorithms/knapsack_solver.h index dfdc729d9f..32ce835ad4 100644 --- a/ortools/algorithms/knapsack_solver.h +++ b/ortools/algorithms/knapsack_solver.h @@ -58,6 +58,7 @@ #define OR_TOOLS_ALGORITHMS_KNAPSACK_SOLVER_H_ #include + #include #include #include diff --git a/ortools/algorithms/sparse_permutation.cc b/ortools/algorithms/sparse_permutation.cc index bf216c2190..cdae55d18c 100644 --- a/ortools/algorithms/sparse_permutation.cc +++ b/ortools/algorithms/sparse_permutation.cc @@ -14,6 +14,7 @@ #include "ortools/algorithms/sparse_permutation.h" #include + #include "absl/strings/str_join.h" #include "ortools/base/logging.h" diff --git a/ortools/base/adjustable_priority_queue.h b/ortools/base/adjustable_priority_queue.h index e3503f4b24..2b6ee858dc 100644 --- a/ortools/base/adjustable_priority_queue.h +++ b/ortools/base/adjustable_priority_queue.h @@ -14,6 +14,7 @@ #include #include #include + #include "ortools/base/basictypes.h" #include "ortools/base/logging.h" #include "ortools/base/macros.h" diff --git a/ortools/base/bitmap.h b/ortools/base/bitmap.h index ae94f3cb6b..d5e1a6f783 100644 --- a/ortools/base/bitmap.h +++ b/ortools/base/bitmap.h @@ -15,6 +15,7 @@ #define OR_TOOLS_BASE_BITMAP_H_ #include + #include "ortools/base/basictypes.h" namespace operations_research { diff --git a/ortools/base/file.cc b/ortools/base/file.cc index 1ed1945db5..766cc1dc9f 100644 --- a/ortools/base/file.cc +++ b/ortools/base/file.cc @@ -13,6 +13,7 @@ #include #include + #include "absl/strings/str_cat.h" #if defined(_MSC_VER) #include diff --git a/ortools/base/int_type.h b/ortools/base/int_type.h index 082a1b0079..aa2ddc5fc5 100644 --- a/ortools/base/int_type.h +++ b/ortools/base/int_type.h @@ -145,6 +145,7 @@ #define OR_TOOLS_BASE_INT_TYPE_H_ #include + #include #include #include // NOLINT diff --git a/ortools/base/int_type_indexed_vector.h b/ortools/base/int_type_indexed_vector.h index 203302f01a..e23dbc78c2 100644 --- a/ortools/base/int_type_indexed_vector.h +++ b/ortools/base/int_type_indexed_vector.h @@ -59,6 +59,7 @@ #define OR_TOOLS_BASE_INT_TYPE_INDEXED_VECTOR_H_ #include + #include #include #include diff --git a/ortools/base/jniutil.h b/ortools/base/jniutil.h index 586b576244..a3cf3c9d67 100644 --- a/ortools/base/jniutil.h +++ b/ortools/base/jniutil.h @@ -15,7 +15,9 @@ #define OR_TOOLS_BASE_JNIUTIL_H_ #include + #include + #include "ortools/base/logging.h" class JNIUtil { diff --git a/ortools/base/map_util.h b/ortools/base/map_util.h index b83aa0a462..d8dc3651f7 100644 --- a/ortools/base/map_util.h +++ b/ortools/base/map_util.h @@ -15,6 +15,7 @@ #define OR_TOOLS_BASE_MAP_UTIL_H_ #include + #include "ortools/base/logging.h" namespace gtl { diff --git a/ortools/base/mathutil.h b/ortools/base/mathutil.h index 5c4c4e7f71..71032eee34 100644 --- a/ortools/base/mathutil.h +++ b/ortools/base/mathutil.h @@ -15,6 +15,7 @@ #define OR_TOOLS_BASE_MATHUTIL_H_ #include + #include #include diff --git a/ortools/base/random.h b/ortools/base/random.h index f954ab63af..94958902c4 100644 --- a/ortools/base/random.h +++ b/ortools/base/random.h @@ -16,6 +16,7 @@ #include #include + #include "ortools/base/basictypes.h" namespace operations_research { diff --git a/ortools/base/recordio.cc b/ortools/base/recordio.cc index d009580577..cf9835cf24 100644 --- a/ortools/base/recordio.cc +++ b/ortools/base/recordio.cc @@ -12,9 +12,12 @@ // limitations under the License. #include "ortools/base/recordio.h" + #include + #include #include + #include "ortools/base/logging.h" namespace recordio { diff --git a/ortools/base/recordio.h b/ortools/base/recordio.h index 219771bf15..de6a61017e 100644 --- a/ortools/base/recordio.h +++ b/ortools/base/recordio.h @@ -16,6 +16,7 @@ #include #include + #include "ortools/base/file.h" // This file defines some IO interfaces to compatible with Google diff --git a/ortools/base/status.h b/ortools/base/status.h index 41220eb8b0..18ff56995e 100644 --- a/ortools/base/status.h +++ b/ortools/base/status.h @@ -15,6 +15,7 @@ #define OR_TOOLS_BASE_STATUS_H_ #include + #include "absl/strings/str_cat.h" #include "ortools/base/logging.h" diff --git a/ortools/bop/integral_solver.cc b/ortools/bop/integral_solver.cc index c9b7756ea4..745ed82ca0 100644 --- a/ortools/bop/integral_solver.cc +++ b/ortools/bop/integral_solver.cc @@ -14,7 +14,9 @@ #include "ortools/bop/integral_solver.h" #include + #include + #include "ortools/bop/bop_solver.h" #include "ortools/lp_data/lp_decomposer.h" diff --git a/ortools/flatzinc/reporting.cc b/ortools/flatzinc/reporting.cc index 94a0733b54..41876b66e9 100644 --- a/ortools/flatzinc/reporting.cc +++ b/ortools/flatzinc/reporting.cc @@ -14,8 +14,8 @@ #include "ortools/flatzinc/reporting.h" #include // NOLINT - #include + #include "absl/strings/str_format.h" #include "absl/synchronization/mutex.h" #include "ortools/base/integral_types.h" diff --git a/ortools/flatzinc/reporting.h b/ortools/flatzinc/reporting.h index 5326381f13..2f24fcf895 100644 --- a/ortools/flatzinc/reporting.h +++ b/ortools/flatzinc/reporting.h @@ -15,6 +15,7 @@ #define OR_TOOLS_FLATZINC_REPORTING_H_ #include + #include "absl/synchronization/mutex.h" #include "ortools/base/integral_types.h" #include "ortools/constraint_solver/constraint_solver.h" diff --git a/ortools/glop/initial_basis.cc b/ortools/glop/initial_basis.cc index 7709b95a49..ce7e7c3966 100644 --- a/ortools/glop/initial_basis.cc +++ b/ortools/glop/initial_basis.cc @@ -12,6 +12,7 @@ // limitations under the License. #include "ortools/glop/initial_basis.h" + #include #include "ortools/glop/markowitz.h" diff --git a/ortools/glop/lp_solver.cc b/ortools/glop/lp_solver.cc index 1a3f593140..ed357b343e 100644 --- a/ortools/glop/lp_solver.cc +++ b/ortools/glop/lp_solver.cc @@ -17,13 +17,12 @@ #include #include -#include "ortools/base/commandlineflags.h" -#include "ortools/base/integral_types.h" -#include "ortools/base/timer.h" - #include "absl/memory/memory.h" #include "absl/strings/match.h" #include "absl/strings/str_format.h" +#include "ortools/base/commandlineflags.h" +#include "ortools/base/integral_types.h" +#include "ortools/base/timer.h" #include "ortools/glop/preprocessor.h" #include "ortools/glop/status.h" #include "ortools/lp_data/lp_types.h" diff --git a/ortools/glop/lu_factorization.cc b/ortools/glop/lu_factorization.cc index ec0dbf32c5..a622206f8f 100644 --- a/ortools/glop/lu_factorization.cc +++ b/ortools/glop/lu_factorization.cc @@ -12,6 +12,7 @@ // limitations under the License. #include "ortools/glop/lu_factorization.h" + #include "ortools/lp_data/lp_utils.h" namespace operations_research { diff --git a/ortools/glop/markowitz.cc b/ortools/glop/markowitz.cc index f084c5083b..85feba6b50 100644 --- a/ortools/glop/markowitz.cc +++ b/ortools/glop/markowitz.cc @@ -14,6 +14,7 @@ #include "ortools/glop/markowitz.h" #include + #include "absl/strings/str_format.h" #include "ortools/lp_data/lp_utils.h" diff --git a/ortools/glop/preprocessor.cc b/ortools/glop/preprocessor.cc index dc6f75895b..ce054b593e 100644 --- a/ortools/glop/preprocessor.cc +++ b/ortools/glop/preprocessor.cc @@ -1573,6 +1573,7 @@ bool DoubletonFreeColumnPreprocessor::Run(LinearProgram* lp) { // column r.col where the coefficient will be left unchanged. r.deleted_row_as_column.AddMultipleToSparseVectorAndIgnoreCommonIndex( -r.coeff[MODIFIED] / r.coeff[DELETED], ColToRowIndex(r.col), + parameters_.drop_tolerance(), transpose->mutable_column(RowToColIndex(r.row[MODIFIED]))); // We also need to correct the objective value of the variables involved in @@ -1589,9 +1590,7 @@ bool DoubletonFreeColumnPreprocessor::Run(LinearProgram* lp) { // the numerical error in the formula above, we have a really low // objective instead. The logic is the same as in // AddMultipleToSparseVectorAndIgnoreCommonIndex(). - if (std::abs(new_objective) > - std::numeric_limits::epsilon() * 2.0 * - std::abs(lp->objective_coefficients()[col])) { + if (std::abs(new_objective) > parameters_.drop_tolerance()) { lp->SetObjectiveCoefficient(col, new_objective); } else { lp->SetObjectiveCoefficient(col, 0.0); @@ -3002,15 +3001,22 @@ bool DoubletonEqualityRowPreprocessor::Run(LinearProgram* lp) { break; } r.column[DELETED].AddMultipleToSparseVectorAndDeleteCommonIndex( - substitution_factor, row, lp->GetMutableSparseColumn(r.col[MODIFIED])); + substitution_factor, row, parameters_.drop_tolerance(), + lp->GetMutableSparseColumn(r.col[MODIFIED])); // Apply similar operations on the objective coefficients. // Note that the offset is being updated by // SubtractColumnMultipleFromConstraintBound() below. - lp->SetObjectiveCoefficient( - r.col[MODIFIED], - r.objective_coefficient[MODIFIED] + - substitution_factor * r.objective_coefficient[DELETED]); + { + const Fractional new_objective = + r.objective_coefficient[MODIFIED] + + substitution_factor * r.objective_coefficient[DELETED]; + if (std::abs(new_objective) > parameters_.drop_tolerance()) { + lp->SetObjectiveCoefficient(r.col[MODIFIED], new_objective); + } else { + lp->SetObjectiveCoefficient(r.col[MODIFIED], 0.0); + } + } // Carry over the constant factor of the substitution as well. // TODO(user): rename that method to reflect the fact that it also updates diff --git a/ortools/graph/connected_components.cc b/ortools/graph/connected_components.cc index d2ae0abfa9..9ab1b24dad 100644 --- a/ortools/graph/connected_components.cc +++ b/ortools/graph/connected_components.cc @@ -27,10 +27,10 @@ // The following uses disjoint-sets algorithms, see: // https://en.wikipedia.org/wiki/Disjoint-set_data_structure#Disjoint-set_forests -#include - #include "ortools/graph/connected_components.h" +#include + void DenseConnectedComponentsFinder::SetNumberOfNodes(int num_nodes) { const int old_num_nodes = GetNumberOfNodes(); if (num_nodes == old_num_nodes) { diff --git a/ortools/graph/one_tree_lower_bound.h b/ortools/graph/one_tree_lower_bound.h index ec3a0d2e5d..921c4eb3eb 100644 --- a/ortools/graph/one_tree_lower_bound.h +++ b/ortools/graph/one_tree_lower_bound.h @@ -122,6 +122,7 @@ #define OR_TOOLS_GRAPH_ONE_TREE_LOWER_BOUND_H_ #include + #include #include diff --git a/ortools/lp_data/lp_data.cc b/ortools/lp_data/lp_data.cc index e0fc561f0d..df39d443a3 100644 --- a/ortools/lp_data/lp_data.cc +++ b/ortools/lp_data/lp_data.cc @@ -17,8 +17,8 @@ #include #include #include -#include "absl/container/flat_hash_map.h" +#include "absl/container/flat_hash_map.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "ortools/base/logging.h" diff --git a/ortools/lp_data/lp_data.h b/ortools/lp_data/lp_data.h index 1508a3317f..8204732346 100644 --- a/ortools/lp_data/lp_data.h +++ b/ortools/lp_data/lp_data.h @@ -28,9 +28,9 @@ #include #include // for std::string #include // for vector + #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" - #include "ortools/base/hash.h" #include "ortools/base/int_type.h" #include "ortools/base/int_type_indexed_vector.h" diff --git a/ortools/lp_data/lp_print_utils.cc b/ortools/lp_data/lp_print_utils.cc index 5340fc861a..a94936ecc0 100644 --- a/ortools/lp_data/lp_print_utils.cc +++ b/ortools/lp_data/lp_print_utils.cc @@ -12,6 +12,7 @@ // limitations under the License. #include "ortools/lp_data/lp_print_utils.h" + #include #include #include diff --git a/ortools/lp_data/lp_types.h b/ortools/lp_data/lp_types.h index 9cd24f9ecc..2d21296b5e 100644 --- a/ortools/lp_data/lp_types.h +++ b/ortools/lp_data/lp_types.h @@ -18,6 +18,7 @@ #include #include + #include "ortools/base/basictypes.h" #include "ortools/base/int_type.h" #include "ortools/base/int_type_indexed_vector.h" diff --git a/ortools/lp_data/matrix_utils.cc b/ortools/lp_data/matrix_utils.cc index 65cf81ca6b..898cf9e0a3 100644 --- a/ortools/lp_data/matrix_utils.cc +++ b/ortools/lp_data/matrix_utils.cc @@ -12,7 +12,9 @@ // limitations under the License. #include "ortools/lp_data/matrix_utils.h" + #include + #include "ortools/base/hash.h" namespace operations_research { diff --git a/ortools/lp_data/sparse.cc b/ortools/lp_data/sparse.cc index b7e5e9021b..8d1581896e 100644 --- a/ortools/lp_data/sparse.cc +++ b/ortools/lp_data/sparse.cc @@ -12,6 +12,7 @@ // limitations under the License. #include "ortools/lp_data/sparse.h" + #include #include "absl/strings/str_format.h" diff --git a/ortools/lp_data/sparse_column.cc b/ortools/lp_data/sparse_column.cc index 2812f84b1d..2bfb15cb2f 100644 --- a/ortools/lp_data/sparse_column.cc +++ b/ortools/lp_data/sparse_column.cc @@ -12,6 +12,7 @@ // limitations under the License. #include "ortools/lp_data/sparse_column.h" + #include #include "ortools/lp_data/lp_types.h" diff --git a/ortools/lp_data/sparse_vector.h b/ortools/lp_data/sparse_vector.h index 571cccf9df..97cbeed0dd 100644 --- a/ortools/lp_data/sparse_vector.h +++ b/ortools/lp_data/sparse_vector.h @@ -241,17 +241,17 @@ class SparseVector { // WARNING: BOTH vectors (the current and the destination) MUST be "clean", // i.e. sorted and without duplicates. // Performs the operation accumulator_vector += multiplier * this, removing - // a given index which must be in both vectors, and pruning new entries that - // are zero with a relative precision of 2 * std::numeric_limits::epsilon(). + // a given index which must be in both vectors, and pruning new entries whose + // absolute value are under the given drop_tolerance. void AddMultipleToSparseVectorAndDeleteCommonIndex( Fractional multiplier, Index removed_common_index, - SparseVector* accumulator_vector) const; + Fractional drop_tolerance, SparseVector* accumulator_vector) const; // Same as AddMultipleToSparseVectorAndDeleteCommonIndex() but instead of // deleting the common index, leave it unchanged. void AddMultipleToSparseVectorAndIgnoreCommonIndex( Fractional multiplier, Index removed_common_index, - SparseVector* accumulator_vector) const; + Fractional drop_tolerance, SparseVector* accumulator_vector) const; // Applies the index permutation to all entries: index = index_perm[index]; void ApplyIndexPermutation(const IndexPermutation& index_perm); @@ -396,7 +396,7 @@ class SparseVector { // and AddMultipleToSparseVectorAndIgnoreCommonIndex() which is shared. void AddMultipleToSparseVectorInternal( bool delete_common_index, Fractional multiplier, Index common_index, - SparseVector* accumulator_vector) const; + Fractional drop_tolerance, SparseVector* accumulator_vector) const; }; // -------------------------------------------------------- @@ -859,24 +859,24 @@ template void SparseVector:: AddMultipleToSparseVectorAndDeleteCommonIndex( Fractional multiplier, Index removed_common_index, - SparseVector* accumulator_vector) const { + Fractional drop_tolerance, SparseVector* accumulator_vector) const { AddMultipleToSparseVectorInternal(true, multiplier, removed_common_index, - accumulator_vector); + drop_tolerance, accumulator_vector); } template void SparseVector:: AddMultipleToSparseVectorAndIgnoreCommonIndex( Fractional multiplier, Index removed_common_index, - SparseVector* accumulator_vector) const { + Fractional drop_tolerance, SparseVector* accumulator_vector) const { AddMultipleToSparseVectorInternal(false, multiplier, removed_common_index, - accumulator_vector); + drop_tolerance, accumulator_vector); } template void SparseVector::AddMultipleToSparseVectorInternal( bool delete_common_index, Fractional multiplier, Index common_index, - SparseVector* accumulator_vector) const { + Fractional drop_tolerance, SparseVector* accumulator_vector) const { // DCHECK that the input is correct. DCHECK(IsCleanedUp()); DCHECK(accumulator_vector->IsCleanedUp()); @@ -912,10 +912,10 @@ void SparseVector::AddMultipleToSparseVectorInternal( const Fractional a_coeff_mul = multiplier * a.GetCoefficient(ia); const Fractional b_coeff = b.GetCoefficient(ib); const Fractional sum = a_coeff_mul + b_coeff; - // We use the factor 2.0 because the error can be slightly greater than - // 1ulp, and we don't want to leave such near zero entries. - if (fabs(sum) > 2.0 * std::numeric_limits::epsilon() * - std::max(fabs(a_coeff_mul), fabs(b_coeff))) { + + // We do not want to leave near-zero entries. + // TODO(user): expose the tolerance used here. + if (std::abs(sum) > drop_tolerance) { c.MutableIndex(ic) = index_a; c.MutableCoefficient(ic) = sum; ++ic; diff --git a/ortools/port/file_nonport.cc b/ortools/port/file_nonport.cc index 1b6579df62..2984ba8683 100644 --- a/ortools/port/file_nonport.cc +++ b/ortools/port/file_nonport.cc @@ -11,9 +11,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "ortools/port/file.h" - #include "ortools/base/file.h" +#include "ortools/port/file.h" namespace operations_research { diff --git a/ortools/port/sysinfo_nonport.cc b/ortools/port/sysinfo_nonport.cc index 26d0bc6a24..1ef2cb3862 100644 --- a/ortools/port/sysinfo_nonport.cc +++ b/ortools/port/sysinfo_nonport.cc @@ -11,9 +11,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "ortools/port/sysinfo.h" - #include "ortools/base/sysinfo.h" +#include "ortools/port/sysinfo.h" namespace operations_research { namespace sysinfo { diff --git a/ortools/sat/boolean_problem.cc b/ortools/sat/boolean_problem.cc index 9f970a7028..d0c8fd8a32 100644 --- a/ortools/sat/boolean_problem.cc +++ b/ortools/sat/boolean_problem.cc @@ -18,8 +18,8 @@ #include #include #include -#include "absl/container/flat_hash_map.h" +#include "absl/container/flat_hash_map.h" #include "absl/strings/str_format.h" #include "ortools/base/commandlineflags.h" #include "ortools/base/integral_types.h" diff --git a/ortools/sat/clause.h b/ortools/sat/clause.h index 7af3976a23..86d78c474d 100644 --- a/ortools/sat/clause.h +++ b/ortools/sat/clause.h @@ -21,9 +21,9 @@ #include #include #include + #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" - #include "absl/container/inlined_vector.h" #include "absl/types/span.h" #include "ortools/base/hash.h" diff --git a/ortools/sat/cp_model.cc b/ortools/sat/cp_model.cc index 183743e7de..b489ed9298 100644 --- a/ortools/sat/cp_model.cc +++ b/ortools/sat/cp_model.cc @@ -12,6 +12,7 @@ // limitations under the License. #include "ortools/sat/cp_model.h" + #include "absl/strings/str_format.h" #include "ortools/base/map_util.h" #include "ortools/sat/cp_model.pb.h" diff --git a/ortools/sat/cp_model.h b/ortools/sat/cp_model.h index edb864dc9e..078f6658ed 100644 --- a/ortools/sat/cp_model.h +++ b/ortools/sat/cp_model.h @@ -35,6 +35,7 @@ #define OR_TOOLS_SAT_CP_MODEL_H_ #include + #include "absl/container/flat_hash_map.h" #include "absl/types/span.h" #include "ortools/sat/cp_model.pb.h" diff --git a/ortools/sat/cp_model_solver.cc b/ortools/sat/cp_model_solver.cc index eedde96966..a96cc678c4 100644 --- a/ortools/sat/cp_model_solver.cc +++ b/ortools/sat/cp_model_solver.cc @@ -108,6 +108,7 @@ DEFINE_bool(cp_model_check_intermediate_solutions, false, namespace operations_research { namespace sat { + namespace { // ============================================================================= diff --git a/ortools/sat/drat_checker.cc b/ortools/sat/drat_checker.cc index 528f12058d..492aadfa57 100644 --- a/ortools/sat/drat_checker.cc +++ b/ortools/sat/drat_checker.cc @@ -15,6 +15,7 @@ #include #include + #include "absl/strings/numbers.h" #include "absl/strings/str_split.h" #include "absl/time/clock.h" diff --git a/ortools/sat/linear_constraint_manager.cc b/ortools/sat/linear_constraint_manager.cc index a03c0dd91d..19d3bfb94f 100644 --- a/ortools/sat/linear_constraint_manager.cc +++ b/ortools/sat/linear_constraint_manager.cc @@ -12,8 +12,10 @@ // limitations under the License. #include "ortools/sat/linear_constraint_manager.h" + #include #include + #include "ortools/sat/integer.h" namespace operations_research { @@ -23,13 +25,20 @@ namespace { const LinearConstraintManager::ConstraintIndex kInvalidConstraintIndex(-1); +size_t ComputeHashOfTerms(const LinearConstraint& ct) { + DCHECK(std::is_sorted(ct.vars.begin(), ct.vars.end())); + size_t hash = 0; + const int num_terms = ct.vars.size(); + for (int i = 0; i < num_terms; ++i) { + hash = util_hash::Hash(ct.vars[i].value(), hash); + hash = util_hash::Hash(ct.coeffs[i].value(), hash); + } + return hash; +} + // TODO(user): it would be better if LinearConstraint natively supported // term and not two separated vectors. Fix? -// -// TODO(user): Divide by gcd? perform coefficient strengthening (note that as -// the search progress the variable bounds get tighter)? -std::vector> -CanonicalizeConstraintAndGetTerms(LinearConstraint* ct) { +void CanonicalizeConstraint(LinearConstraint* ct) { std::vector> terms; const int size = ct->vars.size(); @@ -48,7 +57,6 @@ CanonicalizeConstraintAndGetTerms(LinearConstraint* ct) { ct->vars.push_back(term.first); ct->coeffs.push_back(term.second); } - return terms; } } // namespace @@ -93,60 +101,48 @@ void LinearConstraintManager::RemoveMarkedConstraints() { // Because sometimes we split a == constraint in two (>= and <=), it makes sense // to detect duplicate constraints and merge bounds. This is also relevant if // we regenerate identical cuts for some reason. -// -// TODO(user): Also compute GCD and divide by it. -// TODO(user): Call SimplifyConstraint() too. -void LinearConstraintManager::Add(const LinearConstraint& ct) { - LinearConstraint canonicalized = ct; - const Terms terms = CanonicalizeConstraintAndGetTerms(&canonicalized); - CHECK(!terms.empty()); +LinearConstraintManager::ConstraintIndex LinearConstraintManager::Add( + LinearConstraint ct) { + CHECK(!ct.vars.empty()); + SimplifyConstraint(&ct); + DivideByGCD(&ct); + CanonicalizeConstraint(&ct); - if (gtl::ContainsKey(equiv_constraints_, terms)) { - const ConstraintIndex index( - gtl::FindOrDieNoPrint(equiv_constraints_, terms)); - if (canonicalized.lb > constraints_[index].lb) { - if (constraint_is_in_lp_[index]) { - some_lp_constraint_bounds_changed_ = true; + // If an identical constraint exists, only updates its bound. + const size_t key = ComputeHashOfTerms(ct); + if (gtl::ContainsKey(equiv_constraints_, key)) { + const ConstraintIndex ct_index = equiv_constraints_[key]; + if (constraints_[ct_index].vars == ct.vars && + constraints_[ct_index].coeffs == ct.coeffs) { + if (ct.lb > constraints_[ct_index].lb) { + if (constraint_is_in_lp_[ct_index]) current_lp_is_changed_ = true; + constraints_[ct_index].lb = ct.lb; } - constraints_[index].lb = canonicalized.lb; - } - if (canonicalized.ub < constraints_[index].ub) { - if (constraint_is_in_lp_[index]) { - some_lp_constraint_bounds_changed_ = true; + if (ct.ub < constraints_[ct_index].ub) { + if (constraint_is_in_lp_[ct_index]) current_lp_is_changed_ = true; + constraints_[ct_index].ub = ct.ub; } - constraints_[index].ub = canonicalized.ub; + ++num_merged_constraints_; + return ct_index; } - ++num_merged_constraints_; - } else { - constraint_l2_norms_.push_back(ComputeL2Norm(canonicalized)); - equiv_constraints_[terms] = ConstraintIndex(constraints_.size()); - constraint_is_in_lp_.push_back(false); - constraint_is_cut_.push_back(false); - constraint_inactive_count_.push_back(0); - constraint_permanently_removed_.push_back(false); - constraints_.push_back(std::move(canonicalized)); } -} -void LinearConstraintManager::RemoveConstraintFromEquivTable( - ConstraintIndex ct_index) { - const Terms terms = - CanonicalizeConstraintAndGetTerms(&constraints_[ct_index]); - equiv_constraints_.erase(terms); -} - -void LinearConstraintManager::NotifyConstraintChanged( - ConstraintIndex ct_index) { - const Terms terms = - CanonicalizeConstraintAndGetTerms(&constraints_[ct_index]); - equiv_constraints_[terms] = ct_index; - constraint_l2_norms_[ct_index] = ComputeL2Norm(constraints_[ct_index]); + const ConstraintIndex ct_index(constraints_.size()); + constraint_l2_norms_.push_back(ComputeL2Norm(ct)); + constraint_is_in_lp_.push_back(false); + constraint_is_cut_.push_back(false); + constraint_inactive_count_.push_back(0); + constraint_permanently_removed_.push_back(false); + equiv_constraints_[key] = ct_index; + constraint_hashes_.push_back(key); + constraints_.push_back(std::move(ct)); + return ct_index; } // Same as Add(), but logs some information about the newly added constraint. // Cuts are also handled slightly differently than normal constraints. void LinearConstraintManager::AddCut( - const LinearConstraint& ct, std::string type_name, + LinearConstraint ct, std::string type_name, const gtl::ITIVector& lp_solution) { if (ct.vars.empty()) return; @@ -158,28 +154,32 @@ void LinearConstraintManager::AddCut( // Only add cut with sufficient efficacy. if (violation / l2_norm < 1e-5) return; - Add(ct); - constraint_is_cut_.back() = true; - num_cuts_++; - type_to_num_cuts_[type_name]++; - VLOG(2) << "Cut '" << type_name << "'" << " size=" << ct.vars.size() << " max_magnitude=" << ComputeInfinityNorm(ct) << " norm=" << l2_norm << " violation=" << violation << " eff=" << violation / l2_norm; + + // Add the constraint. We only mark the constraint as a cut if it is not an + // update of an already existing one. + const ConstraintIndex ct_index = Add(std::move(ct)); + if (ct_index + 1 == constraints_.size()) { + num_cuts_++; + type_to_num_cuts_[type_name]++; + constraint_is_cut_[ct_index] = true; + } } -void LinearConstraintManager::SimplifyConstraint(ConstraintIndex ct_index) { - LinearConstraint& ct = constraints_[ct_index]; +bool LinearConstraintManager::SimplifyConstraint(LinearConstraint* ct) { + bool term_changed = false; IntegerValue min_sum(0); IntegerValue max_sum(0); IntegerValue max_magnitude(0); int new_size = 0; - const int num_terms = ct.vars.size(); + const int num_terms = ct->vars.size(); for (int i = 0; i < num_terms; ++i) { - const IntegerVariable var = ct.vars[i]; - const IntegerValue coeff = ct.coeffs[i]; + const IntegerVariable var = ct->vars[i]; + const IntegerValue coeff = ct->coeffs[i]; const IntegerValue lb = integer_trail_.LevelZeroLowerBound(var); const IntegerValue ub = integer_trail_.LevelZeroUpperBound(var); @@ -200,44 +200,39 @@ void LinearConstraintManager::SimplifyConstraint(ConstraintIndex ct_index) { // Shorten the constraint if needed. if (new_size < num_terms) { + term_changed = true; ++num_shortened_constraints_; - RemoveConstraintFromEquivTable(ct_index); - new_size = 0; for (int i = 0; i < num_terms; ++i) { - const IntegerVariable var = ct.vars[i]; - const IntegerValue coeff = ct.coeffs[i]; + const IntegerVariable var = ct->vars[i]; + const IntegerValue coeff = ct->coeffs[i]; const IntegerValue lb = integer_trail_.LevelZeroLowerBound(var); const IntegerValue ub = integer_trail_.LevelZeroUpperBound(var); if (lb == ub) { const IntegerValue rhs_adjust = lb * coeff; - if (ct.lb > kMinIntegerValue) ct.lb -= rhs_adjust; - if (ct.ub < kMaxIntegerValue) ct.ub -= rhs_adjust; + if (ct->lb > kMinIntegerValue) ct->lb -= rhs_adjust; + if (ct->ub < kMaxIntegerValue) ct->ub -= rhs_adjust; continue; } - ct.vars[new_size] = var; - ct.coeffs[new_size] = coeff; + ct->vars[new_size] = var; + ct->coeffs[new_size] = coeff; ++new_size; } - ct.vars.resize(new_size); - ct.coeffs.resize(new_size); - - NotifyConstraintChanged(ct_index); + ct->vars.resize(new_size); + ct->coeffs.resize(new_size); } // Relax the bound if needed, note that this doesn't require a change to // the equiv map. - if (min_sum >= ct.lb) ct.lb = kMinIntegerValue; - if (max_sum <= ct.ub) ct.ub = kMaxIntegerValue; + if (min_sum >= ct->lb) ct->lb = kMinIntegerValue; + if (max_sum <= ct->ub) ct->ub = kMaxIntegerValue; // Clear constraints that are always true. // We rely on the deletion code to remove them eventually. - if (ct.lb == kMinIntegerValue && ct.ub == kMaxIntegerValue) { - RemoveConstraintFromEquivTable(ct_index); - ct.vars.clear(); - ct.coeffs.clear(); - constraint_l2_norms_[ct_index] = 0.0; - return; + if (ct->lb == kMinIntegerValue && ct->ub == kMaxIntegerValue) { + ct->vars.clear(); + ct->coeffs.clear(); + return true; } // TODO(user): Split constraint in two if it is boxed and there is possible @@ -246,67 +241,61 @@ void LinearConstraintManager::SimplifyConstraint(ConstraintIndex ct_index) { // TODO(user): Make sure there cannot be any overflow. They should't, but // I am not sure all the generated cuts are safe regarding min/max sum // computation. We should check this. - if (ct.ub != kMaxIntegerValue && max_magnitude > max_sum - ct.ub) { - if (ct.lb != kMinIntegerValue) { + if (ct->ub != kMaxIntegerValue && max_magnitude > max_sum - ct->ub) { + if (ct->lb != kMinIntegerValue) { ++num_splitted_constraints_; } else { + term_changed = true; ++num_coeff_strenghtening_; - RemoveConstraintFromEquivTable(ct_index); - - const int num_terms = ct.vars.size(); - const IntegerValue target = max_sum - ct.ub; + const int num_terms = ct->vars.size(); + const IntegerValue target = max_sum - ct->ub; for (int i = 0; i < num_terms; ++i) { - const IntegerValue coeff = ct.coeffs[i]; + const IntegerValue coeff = ct->coeffs[i]; if (coeff > target) { - const IntegerVariable var = ct.vars[i]; + const IntegerVariable var = ct->vars[i]; const IntegerValue ub = integer_trail_.LevelZeroUpperBound(var); - ct.coeffs[i] = target; - ct.ub -= (coeff - target) * ub; + ct->coeffs[i] = target; + ct->ub -= (coeff - target) * ub; } else if (coeff < -target) { - const IntegerVariable var = ct.vars[i]; + const IntegerVariable var = ct->vars[i]; const IntegerValue lb = integer_trail_.LevelZeroLowerBound(var); - ct.coeffs[i] = -target; - ct.ub += (-target - coeff) * lb; + ct->coeffs[i] = -target; + ct->ub += (-target - coeff) * lb; } } - - NotifyConstraintChanged(ct_index); } } - if (ct.lb != kMinIntegerValue && max_magnitude > ct.lb - min_sum) { - if (ct.ub != kMaxIntegerValue) { + if (ct->lb != kMinIntegerValue && max_magnitude > ct->lb - min_sum) { + if (ct->ub != kMaxIntegerValue) { ++num_splitted_constraints_; } else { + term_changed = true; ++num_coeff_strenghtening_; - RemoveConstraintFromEquivTable(ct_index); - - const int num_terms = ct.vars.size(); - const IntegerValue target = ct.lb - min_sum; + const int num_terms = ct->vars.size(); + const IntegerValue target = ct->lb - min_sum; for (int i = 0; i < num_terms; ++i) { - const IntegerValue coeff = ct.coeffs[i]; + const IntegerValue coeff = ct->coeffs[i]; if (coeff > target) { - const IntegerVariable var = ct.vars[i]; + const IntegerVariable var = ct->vars[i]; const IntegerValue lb = integer_trail_.LevelZeroLowerBound(var); - ct.coeffs[i] = target; - ct.lb -= (coeff - target) * lb; + ct->coeffs[i] = target; + ct->lb -= (coeff - target) * lb; } else if (coeff < -target) { - const IntegerVariable var = ct.vars[i]; + const IntegerVariable var = ct->vars[i]; const IntegerValue ub = integer_trail_.LevelZeroUpperBound(var); - ct.coeffs[i] = -target; - ct.lb += (-target - coeff) * ub; + ct->coeffs[i] = -target; + ct->lb += (-target - coeff) * ub; } } - - NotifyConstraintChanged(ct_index); } } + + return term_changed; } bool LinearConstraintManager::ChangeLp( const gtl::ITIVector& lp_solution) { - const int old_num_constraints = lp_constraints_.size(); - std::vector new_constraints; std::vector new_constraints_efficacies; std::vector new_constraints_orthogonalities; @@ -322,7 +311,15 @@ bool LinearConstraintManager::ChangeLp( if (constraint_permanently_removed_[i]) continue; // Inprocessing of the constraint. - if (simplify_constraints) SimplifyConstraint(i); + if (simplify_constraints && SimplifyConstraint(&constraints_[i])) { + DivideByGCD(&constraints_[i]); + constraint_l2_norms_[i] = ComputeL2Norm(constraints_[i]); + + if (constraint_is_in_lp_[i]) current_lp_is_changed_ = true; + equiv_constraints_.erase(constraint_hashes_[i]); + constraint_hashes_[i] = ComputeHashOfTerms(constraints_[i]); + equiv_constraints_[constraint_hashes_[i]] = i; + } // TODO(user,user): Use constraint status from GLOP for constraints // already in LP. @@ -407,17 +404,16 @@ bool LinearConstraintManager::ChangeLp( // Note that it is important for LP incremental solving that the old // constraints stays at the same position in this list (and thus in the // returned GetLp()). + current_lp_is_changed_ = true; lp_constraints_.push_back(best_candidate); last_added_candidate = best_candidate; } } - // The LP changed only if we added new constraints or if the bounds of some - // constraints already in the LP changed because of parallel constraints - // merging during Add(). - if (some_lp_constraint_bounds_changed_ || - lp_constraints_.size() > old_num_constraints) { - some_lp_constraint_bounds_changed_ = false; + // The LP changed only if we added new constraints or if some constraints + // already inside changed (simplification or tighter bounds). + if (current_lp_is_changed_) { + current_lp_is_changed_ = false; return true; } return false; diff --git a/ortools/sat/linear_constraint_manager.h b/ortools/sat/linear_constraint_manager.h index 3b63ac14f9..f808aba3d3 100644 --- a/ortools/sat/linear_constraint_manager.h +++ b/ortools/sat/linear_constraint_manager.h @@ -45,12 +45,14 @@ class LinearConstraintManager { ~LinearConstraintManager(); // Add a new constraint to the manager. Note that we canonicalize constraints - // and merge the bounds of constraints with the same terms. - void Add(const LinearConstraint& ct); + // and merge the bounds of constraints with the same terms. We also perform + // basic preprocessing. + DEFINE_INT_TYPE(ConstraintIndex, int32); + ConstraintIndex Add(LinearConstraint ct); // Same as Add(), but logs some information about the newly added constraint. // Cuts are also handled slightly differently than normal constraints. - void AddCut(const LinearConstraint& ct, std::string type_name, + void AddCut(LinearConstraint ct, std::string type_name, const gtl::ITIVector& lp_solution); // Heuristic to decides what LP is best solved next. The given lp_solution @@ -65,7 +67,6 @@ class LinearConstraintManager { void AddAllConstraintsToLp(); // All the constraints managed by this class. - DEFINE_INT_TYPE(ConstraintIndex, int32); const gtl::ITIVector& AllConstraints() const { return constraints_; @@ -77,40 +78,29 @@ class LinearConstraintManager { return lp_constraints_; } - int num_cuts() const { return num_cuts_; } + int64 num_cuts() const { return num_cuts_; } int64 num_shortened_constraints() const { return num_shortened_constraints_; } int64 num_coeff_strenghtening() const { return num_coeff_strenghtening_; } - // Visible for testing. - // - // Apply basic inprocessing simplification rules: - // - remove fixed variable - // - reduce large coefficient (i.e. coeff strenghtenning or big-M reduction). - // This uses level-zero bounds. - void SimplifyConstraint(ConstraintIndex index); - private: // Removes the marked constraints from the LP. void RemoveMarkedConstraints(); - // Important: before modifying a constraint in equiv_constraints_, one - // must call RemoveConstraintFromEquivTable() and when done, call - // NotifyConstraintChanged(). - // - // TODO(user): That might be a bit slow and brittle. Redesign the equiv map - // so that we compare actual constraints (so we never merge different ones), - // and we don't need to keep duplicate of the constraints in memory. - void RemoveConstraintFromEquivTable(ConstraintIndex ct_index); - void NotifyConstraintChanged(ConstraintIndex ct_index); + // Apply basic inprocessing simplification rules: + // - remove fixed variable + // - reduce large coefficient (i.e. coeff strenghtenning or big-M reduction). + // This uses level-zero bounds. + // Returns true if the terms of the constraint changed. + bool SimplifyConstraint(LinearConstraint* ct); const SatParameters& sat_parameters_; const IntegerTrail& integer_trail_; - // Set at true by Add() and at false by ChangeLp(). - bool some_lp_constraint_bounds_changed_ = false; + // Set at true by Add()/SimplifyConstraint() and at false by ChangeLp(). + bool current_lp_is_changed_ = false; // Optimization to avoid calling SimplifyConstraint() when not needed. - int64 last_simplification_timestamp_ = -1; + int64 last_simplification_timestamp_ = 0; // TODO(user): Merge all the constraint related info in a struct and store // a vector of struct instead. The global list of constraint. @@ -132,29 +122,20 @@ class LinearConstraintManager { gtl::ITIVector constraint_is_in_lp_; std::vector lp_constraints_; - // For each constraint "terms", equiv_constraints_ indicates the index of a - // constraint with the same terms in constraints_. This way, when a - // "duplicate" constraint is added, we can just update its bound. - using Terms = std::vector>; - struct TermsHash { - std::size_t operator()(const Terms& terms) const { - size_t hash = 0; - for (const auto& term : terms) { - const size_t pair_hash = - util_hash::Hash(term.first.value(), term.second.value()); - hash = util_hash::Hash(pair_hash, hash); - } - return hash; - } - }; - absl::flat_hash_map equiv_constraints_; + // We keep a map from the hash of our constraint terms to their position in + // constraints_. This is an optimization to detect duplicate constraints. We + // are robust to collisions because we always relies on the ground truth + // contained in constraints_ and the code is still okay if we do not merge the + // constraints. + absl::flat_hash_map equiv_constraints_; + gtl::ITIVector constraint_hashes_; int64 num_merged_constraints_ = 0; int64 num_shortened_constraints_ = 0; int64 num_splitted_constraints_ = 0; int64 num_coeff_strenghtening_ = 0; - int num_cuts_ = 0; + int64 num_cuts_ = 0; std::map type_to_num_cuts_; }; diff --git a/ortools/sat/sat_decision.h b/ortools/sat/sat_decision.h index 008c518ed8..0e74debfff 100644 --- a/ortools/sat/sat_decision.h +++ b/ortools/sat/sat_decision.h @@ -15,6 +15,7 @@ #define OR_TOOLS_SAT_SAT_DECISION_H_ #include + #include "ortools/base/integral_types.h" #include "ortools/sat/model.h" #include "ortools/sat/pb_constraint.h" diff --git a/ortools/util/bitset.h b/ortools/util/bitset.h index cc97e47f9a..58a00edb77 100644 --- a/ortools/util/bitset.h +++ b/ortools/util/bitset.h @@ -17,6 +17,7 @@ #define OR_TOOLS_UTIL_BITSET_H_ #include + #include #include diff --git a/ortools/util/cached_log.h b/ortools/util/cached_log.h index 8c88e72d4b..d0dbe2014a 100644 --- a/ortools/util/cached_log.h +++ b/ortools/util/cached_log.h @@ -15,6 +15,7 @@ #define OR_TOOLS_UTIL_CACHED_LOG_H_ #include + #include #include "ortools/base/basictypes.h" diff --git a/ortools/util/functions_swig_test_helpers.h b/ortools/util/functions_swig_test_helpers.h index d86b4a08d3..a94d24a7f7 100644 --- a/ortools/util/functions_swig_test_helpers.h +++ b/ortools/util/functions_swig_test_helpers.h @@ -24,6 +24,7 @@ #include #include #include + #include "ortools/base/integral_types.h" namespace operations_research { diff --git a/ortools/util/piecewise_linear_function.h b/ortools/util/piecewise_linear_function.h index e4d8c0abdd..a9eea9d567 100644 --- a/ortools/util/piecewise_linear_function.h +++ b/ortools/util/piecewise_linear_function.h @@ -29,7 +29,6 @@ #include "ortools/base/basictypes.h" #include "ortools/base/integral_types.h" #include "ortools/base/macros.h" - #include "ortools/util/saturated_arithmetic.h" namespace operations_research { diff --git a/ortools/util/rational_approximation.cc b/ortools/util/rational_approximation.cc index b3a70bb2ba..a629985bc5 100644 --- a/ortools/util/rational_approximation.cc +++ b/ortools/util/rational_approximation.cc @@ -12,6 +12,7 @@ // limitations under the License. #include "ortools/util/rational_approximation.h" + #include #include diff --git a/ortools/util/saturated_arithmetic.h b/ortools/util/saturated_arithmetic.h index 44ad2772d4..266cfcc1cd 100644 --- a/ortools/util/saturated_arithmetic.h +++ b/ortools/util/saturated_arithmetic.h @@ -14,9 +14,8 @@ #ifndef OR_TOOLS_UTIL_SATURATED_ARITHMETIC_H_ #define OR_TOOLS_UTIL_SATURATED_ARITHMETIC_H_ -#include "ortools/base/integral_types.h" - #include "absl/base/casts.h" +#include "ortools/base/integral_types.h" #include "ortools/util/bitset.h" namespace operations_research { diff --git a/ortools/util/tuple_set.h b/ortools/util/tuple_set.h index ce4f1c7940..4e734a4416 100644 --- a/ortools/util/tuple_set.h +++ b/ortools/util/tuple_set.h @@ -35,9 +35,9 @@ #include #include + #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" - #include "ortools/base/hash.h" #include "ortools/base/integral_types.h" #include "ortools/base/logging.h" diff --git a/ortools/util/vector_map.h b/ortools/util/vector_map.h index 7a21d2d457..9a97c3aff0 100644 --- a/ortools/util/vector_map.h +++ b/ortools/util/vector_map.h @@ -17,6 +17,7 @@ #define OR_TOOLS_UTIL_VECTOR_MAP_H_ #include + #include "absl/container/flat_hash_map.h" #include "ortools/base/map_util.h"