Skip to content

Commit

Permalink
cleanups
Browse files Browse the repository at this point in the history
  • Loading branch information
lperron committed Oct 16, 2023
1 parent 580ab46 commit d0eb8dd
Show file tree
Hide file tree
Showing 9 changed files with 52 additions and 50 deletions.
33 changes: 33 additions & 0 deletions ortools/algorithms/find_graph_symmetries_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,39 @@ TEST(RecursivelyRefinePartitionByAdjacencyTest, FlowerOfCycles) {
}));
}

TEST(ComputeGraphFingerprintTest, EmptyGraph) {
Graph empty_graph;
empty_graph.Build();
EXPECT_NE(ComputeGraphFingerprint(empty_graph, 10), absl::uint128{0});
}

TEST(GraphSymmetryFinderTest, EmptyGraph) {
for (bool is_undirected : {true, false}) {
SCOPED_TRACE(DUMP_VARS(is_undirected));
Graph empty_graph;
empty_graph.Build();
GraphSymmetryFinder symmetry_finder(empty_graph, is_undirected);

EXPECT_TRUE(symmetry_finder.IsGraphAutomorphism(DynamicPermutation(0)));

std::vector<int> node_equivalence_classes_io;
std::vector<std::unique_ptr<SparsePermutation>> generators;
std::vector<int> factorized_automorphism_group_size;
ASSERT_OK(symmetry_finder.FindSymmetries(
&node_equivalence_classes_io, &generators,
&factorized_automorphism_group_size));
EXPECT_THAT(node_equivalence_classes_io, IsEmpty());
EXPECT_THAT(generators, IsEmpty());
EXPECT_THAT(factorized_automorphism_group_size, IsEmpty());
}
}

TEST(GraphSymmetryFinderTest, EmptyGraphAndDoNothing) {
Graph empty_graph;
empty_graph.Build();
GraphSymmetryFinder symmetry_finder(empty_graph, /*is_undirected=*/true);
}

class IsGraphAutomorphismTest : public testing::Test {
protected:
void ExpectIsGraphAutomorphism(
Expand Down
8 changes: 4 additions & 4 deletions ortools/base/stl_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ class TemplatedElementDeleter : public BaseDeleter {
STLContainer* container_ptr_;
};

// ElementDeleter is an RAII (go/raii) object that deletes the elements in the
// ElementDeleter is an RAII object that deletes the elements in the
// given container when it goes out of scope. This is similar to
// std::unique_ptr<> except that a container's elements will be deleted rather
// than the container itself.
Expand Down Expand Up @@ -464,7 +464,7 @@ class TemplatedValueDeleter : public BaseDeleter {
STLContainer* container_ptr_;
};

// ValueDeleter is an RAII (go/raii) object that deletes the 'second' member in
// ValueDeleter is an RAII object that deletes the 'second' member in
// the given container of std::pair<>s when it goes out of scope.
//
// Example:
Expand All @@ -488,7 +488,7 @@ class ValueDeleter {
BaseDeleter* deleter_;
};

// RAII (go/raii) object that deletes elements in the given container when it
// RAII object that deletes elements in the given container when it
// goes out of scope. Like ElementDeleter (above) except that this class is
// templated and doesn't have a virtual destructor.
//
Expand All @@ -503,7 +503,7 @@ class STLElementDeleter {
STLContainer* container_ptr_;
};

// RAII (go/raii) object that deletes the values in the given container of
// RAII object that deletes the values in the given container of
// std::pair<>s when it goes out of scope. Like ValueDeleter (above) except that
// this class is templated and doesn't have a virtual destructor.
//
Expand Down
2 changes: 1 addition & 1 deletion ortools/graph/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ std::vector<int> ComputeOnePossibleReverseArcMapping(
std::vector<int> reverse_arc(graph.num_arcs(), -1);
// We need a multi-map since a given (tail,head) may appear several times.
// NOTE(user): It's free, in terms of space, to use InlinedVector<int, 4>
// rather than std::vector<int>. See go/inlined-vector-size.
// rather than std::vector<int>.
absl::flat_hash_map<std::pair</*tail*/ int, /*head*/ int>,
absl::InlinedVector<int, 4>>
arc_map;
Expand Down
2 changes: 0 additions & 2 deletions ortools/linear_solver/linear_solver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1899,14 +1899,12 @@ int64_t MPSolver::global_num_constraints_ = 0;

// static
int64_t MPSolver::global_num_variables() {
// Why not ReaderMutexLock? See go/totw/197#when-are-shared-locks-useful.
absl::MutexLock lock(&global_count_mutex_);
return global_num_variables_;
}

// static
int64_t MPSolver::global_num_constraints() {
// Why not ReaderMutexLock? See go/totw/197#when-are-shared-locks-useful.
absl::MutexLock lock(&global_count_mutex_);
return global_num_constraints_;
}
Expand Down
5 changes: 1 addition & 4 deletions ortools/linear_solver/linear_solver.h
Original file line number Diff line number Diff line change
Expand Up @@ -586,8 +586,7 @@ class MPSolver {
const MPModelRequest::SolverType solver) {
// Interruption requires that MPSolver::InterruptSolve is supported for the
// underlying solver. Interrupting requests using SCIP is also not supported
// as of 2021/08/23, since InterruptSolve is not go/thread-safe
// for SCIP (see e.g. cl/350545631 for details).
// as of 2021/08/23, since InterruptSolve is not thread safe for SCIP.
return solver == MPModelRequest::GLOP_LINEAR_PROGRAMMING ||
solver == MPModelRequest::GUROBI_LINEAR_PROGRAMMING ||
solver == MPModelRequest::GUROBI_MIXED_INTEGER_PROGRAMMING ||
Expand Down Expand Up @@ -828,8 +827,6 @@ class MPSolver {
//
// As of 2019-10-22, only SCIP and Gurobi support Callbacks.
// SCIP does not support suggesting a heuristic solution in the callback.
//
// See go/mpsolver-callbacks for additional documentation.
void SetCallback(MPCallback* mp_callback);
bool SupportsCallbacks() const;

Expand Down
8 changes: 4 additions & 4 deletions ortools/linear_solver/proto_solver/sat_proto_solver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ using google::protobuf::Message;
using google::protobuf::Message;
#endif

// Proto-lite disables some features of protos (see
// go/abp-libraries/proto2-lite) and messages inherit from MessageLite directly
// instead of inheriting from Message (which is itself a specialization of
// MessageLite).
// Proto-lite disables some features of protos and messages inherit from
// MessageLite directly instead of inheriting from Message (which is itself a
// specialization of MessageLite).
// See https://protobuf.dev/reference/cpp/cpp-generated/#message for details.
constexpr bool kProtoLiteSatParameters =
!std::is_base_of<Message, sat::SatParameters>::value;

Expand Down
3 changes: 1 addition & 2 deletions ortools/linear_solver/scip_interface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,7 @@ class SCIPInterface : public MPSolverInterface {
// * We also support SCIP's more general callback interface, built on
// 'constraint handlers'. See ./scip_callback.h and test, these are added
// directly to the underlying SCIP object, bypassing SCIPInterface.
// The former works by calling the latter. See go/scip-callbacks for
// a complete documentation of this design.
// The former works by calling the latter.

// MPCallback API
void SetCallback(MPCallback* mp_callback) override;
Expand Down
19 changes: 8 additions & 11 deletions ortools/sat/cp_model_checker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ bool ExpressionIsFixed(const CpModelProto& model,

int64_t ExpressionFixedValue(const CpModelProto& model,
const LinearExpressionProto& expr) {
CHECK(ExpressionIsFixed(model, expr));
DCHECK(ExpressionIsFixed(model, expr));
return MinOfExpression(model, expr);
}

Expand Down Expand Up @@ -420,8 +420,7 @@ std::string ValidateElementConstraint(const CpModelProto& model,
return "";
}

std::string ValidateTableConstraint(const CpModelProto& /*model*/,
const ConstraintProto& ct) {
std::string ValidateTableConstraint(const ConstraintProto& ct) {
const TableConstraintProto& arg = ct.table();
if (arg.vars().empty()) return "";
if (arg.values().size() % arg.vars().size() != 0) {
Expand All @@ -433,8 +432,7 @@ std::string ValidateTableConstraint(const CpModelProto& /*model*/,
return "";
}

std::string ValidateAutomatonConstraint(const CpModelProto& /*model*/,
const ConstraintProto& ct) {
std::string ValidateAutomatonConstraint(const ConstraintProto& ct) {
const int num_transistions = ct.automaton().transition_tail().size();
if (num_transistions != ct.automaton().transition_head().size() ||
num_transistions != ct.automaton().transition_label().size()) {
Expand Down Expand Up @@ -469,8 +467,7 @@ std::string ValidateAutomatonConstraint(const CpModelProto& /*model*/,
}

template <typename GraphProto>
std::string ValidateGraphInput(bool is_route, const CpModelProto& /*model*/,
const GraphProto& graph) {
std::string ValidateGraphInput(bool is_route, const GraphProto& graph) {
const int size = graph.tails().size();
if (graph.heads().size() != size || graph.literals().size() != size) {
return absl::StrCat("Wrong field sizes in graph: ",
Expand Down Expand Up @@ -519,7 +516,7 @@ std::string ValidateRoutesConstraint(const CpModelProto& model,
"All nodes in a route constraint must have incident arcs");
}

return ValidateGraphInput(/*is_route=*/true, model, ct.routes());
return ValidateGraphInput(/*is_route=*/true, ct.routes());
}

std::string ValidateDomainIsPositive(const CpModelProto& model, int ref,
Expand Down Expand Up @@ -988,14 +985,14 @@ std::string ValidateCpModel(const CpModelProto& model, bool after_presolve) {
RETURN_IF_NOT_EMPTY(ValidateElementConstraint(model, ct));
break;
case ConstraintProto::ConstraintCase::kTable:
RETURN_IF_NOT_EMPTY(ValidateTableConstraint(model, ct));
RETURN_IF_NOT_EMPTY(ValidateTableConstraint(ct));
break;
case ConstraintProto::ConstraintCase::kAutomaton:
RETURN_IF_NOT_EMPTY(ValidateAutomatonConstraint(model, ct));
RETURN_IF_NOT_EMPTY(ValidateAutomatonConstraint(ct));
break;
case ConstraintProto::ConstraintCase::kCircuit:
RETURN_IF_NOT_EMPTY(
ValidateGraphInput(/*is_route=*/false, model, ct.circuit()));
ValidateGraphInput(/*is_route=*/false, ct.circuit()));
break;
case ConstraintProto::ConstraintCase::kRoutes:
RETURN_IF_NOT_EMPTY(ValidateRoutesConstraint(model, ct));
Expand Down
22 changes: 0 additions & 22 deletions ortools/sat/integer_expr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1196,28 +1196,6 @@ DivisionPropagator::DivisionPropagator(AffineExpression num,
// TODO(user): We can propagate a bit more if min_div = 0:
// (min_num > -min_denom).
bool DivisionPropagator::Propagate() {
// Direct propagation if num_ and denom_ are fixed.
if (integer_trail_->IsFixed(num_) && integer_trail_->IsFixed(denom_)) {
const IntegerValue num_value = integer_trail_->FixedValue(num_);
const IntegerValue denom_value = integer_trail_->FixedValue(denom_);
const IntegerValue div_value = num_value / denom_value;
if (!integer_trail_->SafeEnqueue(
div_.LowerOrEqual(div_value),
{num_.LowerOrEqual(num_value), num_.GreaterOrEqual(num_value),
denom_.LowerOrEqual(denom_value),
denom_.GreaterOrEqual(denom_value)})) {
return false;
}
if (!integer_trail_->SafeEnqueue(
div_.GreaterOrEqual(div_value),
{num_.LowerOrEqual(num_value), num_.GreaterOrEqual(num_value),
denom_.LowerOrEqual(denom_value),
denom_.GreaterOrEqual(denom_value)})) {
return false;
}
return true;
}

if (integer_trail_->LowerBound(denom_) < 0 &&
integer_trail_->UpperBound(denom_) > 0) {
return true;
Expand Down

0 comments on commit d0eb8dd

Please sign in to comment.