From 69cf04d4b494c19e4073bc3d655e5e0dc045aff7 Mon Sep 17 00:00:00 2001 From: "lperron@google.com" Date: Sat, 6 Aug 2011 01:02:14 +0000 Subject: [PATCH] make mincostflow/maxflow friendlier in case of incorrect data --- graph/ebert_graph.h | 54 ++++++++++++++++++++++++--------------------- graph/max_flow.cc | 28 +++++++++++------------ graph/max_flow.h | 6 +++-- util/packed_array.h | 38 ++++++++++++++++++++++--------- 4 files changed, 73 insertions(+), 53 deletions(-) diff --git a/graph/ebert_graph.h b/graph/ebert_graph.h index 8595e42e0b..e98cb4335e 100644 --- a/graph/ebert_graph.h +++ b/graph/ebert_graph.h @@ -145,7 +145,10 @@ template class EbertGraph { next_adjacent_arc_(), first_incident_arc_(), representation_clean_(true) { - Reserve(max_num_nodes, max_num_arcs); + if (!Reserve(max_num_nodes, max_num_arcs)) { + LOG(DFATAL) << "Could not reserve memory for " << max_num_nodes + << " nodes and " << max_num_arcs << " arcs."; + } first_incident_arc_.Assign(kNilArc); next_adjacent_arc_.Assign(kNilArc); node_.Assign(kNilNode); @@ -154,13 +157,16 @@ template class EbertGraph { ~EbertGraph() {} // Reserves memory needed for max_num_nodes nodes and max_num_arcs arcs. + // Returns false if the parameters passed are not OK. // It can be used to enlarge the graph, but does not shrink memory // if called with smaller values. - void Reserve(NodeIndex new_max_num_nodes, ArcIndex new_max_num_arcs) { - CHECK_LE(1, new_max_num_nodes); - CHECK_GE(kMaxNumNodes, new_max_num_nodes); - CHECK_LE(1, new_max_num_arcs); - CHECK_GE(kMaxNumArcs, new_max_num_arcs); + bool Reserve(NodeIndex new_max_num_nodes, ArcIndex new_max_num_arcs) { + if (new_max_num_nodes < 1 || new_max_num_nodes > kMaxNumNodes) { + return false; + } + if (new_max_num_arcs < 1 || new_max_num_arcs > kMaxNumArcs) { + return false; + } node_.Reserve(-new_max_num_arcs, new_max_num_arcs - 1); next_adjacent_arc_.Reserve(-new_max_num_arcs, new_max_num_arcs - 1); for (ArcIndex arc = -new_max_num_arcs; arc < -max_num_arcs_; ++arc) { @@ -177,6 +183,7 @@ template class EbertGraph { } max_num_nodes_ = new_max_num_nodes; max_num_arcs_ = new_max_num_arcs; + return true; } // Returns the number of nodes in the graph. @@ -213,13 +220,18 @@ template class EbertGraph { // over arrays of arc annotation information. ArcIndex max_end_arc_index() const { return kFirstArc + max_num_arcs_; } + // Returns true if node is in the range [kFirstNode .. max_num_nodes_). + bool IsNodeValid(NodeIndex node) const { + return node >= kFirstNode && node < max_num_nodes_; + } + // Adds an arc to the graph and returns its index. + // Returns kNilArc if the arc could not be added. ArcIndex AddArc(NodeIndex tail, NodeIndex head) { - CHECK_LE(kFirstNode, tail); - CHECK_GT(max_num_nodes_, tail); - CHECK_LE(kFirstNode, head); - CHECK_GT(max_num_nodes_, head); - CHECK_GT(max_num_arcs_, num_arcs_); + if (num_arcs_ >= max_num_arcs_ + || !IsNodeValid(tail) || !IsNodeValid(head)) { + return kNilArc; + } num_nodes_ = std::max(num_nodes_, tail + 1); num_nodes_ = std::max(num_nodes_, head + 1); ArcIndex arc = num_arcs_; @@ -412,7 +424,7 @@ template class EbertGraph { if (arc_ == kNilArc) { return true; // This occurs when the iterator has reached the end. } - CHECK(graph_.IsIncident(arc_, node_)); + DCHECK(graph_.IsIncident(arc_, node_)); return true; } // A reference to the current EbertGraph considered. @@ -468,7 +480,7 @@ template class EbertGraph { if (arc_ == kNilArc) { return true; // This occurs when the iterator has reached the end. } - CHECK(graph_.IsIncoming(arc_, node_)); + DCHECK(graph_.IsIncoming(arc_, node_)); return true; } // A reference to the current EbertGraph considered. @@ -519,7 +531,7 @@ template class EbertGraph { if (arc_ == kNilArc) { return true; // This occurs when the iterator has reached the end. } - CHECK(graph_.IsOutgoing(arc_, node_)); + DCHECK(graph_.IsOutgoing(arc_, node_)); return true; } @@ -537,10 +549,7 @@ template class EbertGraph { // It is exported so that users of the EbertGraph class can use it. // To be used in a DCHECK. bool CheckArcBounds(const ArcIndex arc) const { - if (arc == kNilArc) return true; - CHECK_LE(-max_num_arcs_, arc); - CHECK_GT(max_num_arcs_, arc); - return true; + return (arc == kNilArc) || (arc >= -max_num_arcs_ && arc < max_num_arcs_); } // Utility function to check that an arc index is within the bounds AND @@ -548,10 +557,7 @@ template class EbertGraph { // It is exported so that users of the EbertGraph class can use it. // To be used in a DCHECK. bool CheckArcValidity(const ArcIndex arc) const { - CHECK_NE(kNilArc, arc); - CHECK_LE(-max_num_arcs_, arc); - CHECK_GT(max_num_arcs_, arc); - return true; + return (arc != kNilArc) && (arc >= -max_num_arcs_ && arc < max_num_arcs_); } // Utility function to check that a node index is within the bounds AND @@ -559,9 +565,7 @@ template class EbertGraph { // It is exported so that users of the EbertGraph class can use it. // To be used in a DCHECK. bool CheckNodeValidity(const NodeIndex node) const { - CHECK_LE(kFirstNode, node); - CHECK_GT(max_num_nodes_, node); - return true; + return node >= kFirstNode && node < max_num_nodes_; } // Returns the tail or start-node of arc. diff --git a/graph/max_flow.cc b/graph/max_flow.cc index 2cd697fc90..2e94aefb2a 100644 --- a/graph/max_flow.cc +++ b/graph/max_flow.cc @@ -13,8 +13,6 @@ #include "graph/max_flow.h" -#include - #include "base/commandlineflags.h" #include "base/stringprintf.h" @@ -71,7 +69,7 @@ void MaxFlow::SetArcCapacity(ArcIndex arc, FlowQuantity new_capacity) { DCHECK(graph_->CheckArcValidity(arc)); const FlowQuantity free_capacity = residual_arc_capacity_[arc]; const FlowQuantity capacity_delta = new_capacity - Capacity(arc); - VLOG(1) << "Changing capacity on arc " << arc + VLOG(2) << "Changing capacity on arc " << arc << " from " << Capacity(arc) << " to " << new_capacity << ". Current free capacity = " << free_capacity; if (capacity_delta == 0) { @@ -86,13 +84,13 @@ void MaxFlow::SetArcCapacity(ArcIndex arc, FlowQuantity new_capacity) { // reduction is not larger than the free capacity. residual_arc_capacity_.Set(arc, free_capacity + capacity_delta); DCHECK_LE(0, residual_arc_capacity_[arc]); - VLOG(1) << "Now: capacity = " << Capacity(arc) << " flow = " << Flow(arc); + VLOG(2) << "Now: capacity = " << Capacity(arc) << " flow = " << Flow(arc); } else { // We have to reduce the flow on the arc, and update the excesses // accordingly. const FlowQuantity flow = residual_arc_capacity_[Opposite(arc)]; const FlowQuantity flow_excess = flow - new_capacity; - VLOG(1) << "Flow value " << flow << " exceeds new capacity " + VLOG(2) << "Flow value " << flow << " exceeds new capacity " << new_capacity << " by " << flow_excess; residual_arc_capacity_.Set(arc, 0); residual_arc_capacity_.Set(Opposite(arc), new_capacity); @@ -100,7 +98,7 @@ void MaxFlow::SetArcCapacity(ArcIndex arc, FlowQuantity new_capacity) { node_excess_.Set(head, node_excess_[head] + flow_excess); DCHECK_LE(0, residual_arc_capacity_[arc]); DCHECK_LE(0, residual_arc_capacity_[Opposite(arc)]); - VLOG(1) << DebugString("After SetArcCapacity:", arc); + VLOG(2) << DebugString("After SetArcCapacity:", arc); } } @@ -224,14 +222,14 @@ void MaxFlow::InitializePreflow() { residual_arc_capacity_.Set(arc, 0); residual_arc_capacity_.Set(Opposite(arc), arc_capacity); node_excess_.Set(Head(arc), arc_capacity); - VLOG(1) << DebugString("InitializePreflow:", arc); + VLOG(2) << DebugString("InitializePreflow:", arc); } } void MaxFlow::PushFlow(FlowQuantity flow, ArcIndex arc) { DCHECK_GT(residual_arc_capacity_[arc], 0); DCHECK_GT(node_excess_[Tail(arc)], 0); - VLOG(1) << "PushFlow: pushing " << flow << " on arc " << arc + VLOG(2) << "PushFlow: pushing " << flow << " on arc " << arc << " from node " << Tail(arc) << " to node " << Head(arc); // Reduce the residual capacity on arc by flow. residual_arc_capacity_.Set(arc, residual_arc_capacity_[arc] - flow); @@ -243,7 +241,7 @@ void MaxFlow::PushFlow(FlowQuantity flow, ArcIndex arc) { node_excess_.Set(tail, node_excess_[tail] - flow); const NodeIndex head = Head(arc); node_excess_.Set(head, node_excess_[head] + flow); - VLOG(2) << DebugString("PushFlow: ", arc); + VLOG(3) << DebugString("PushFlow: ", arc); } void MaxFlow::InitializeActiveNodeStack() { @@ -252,7 +250,7 @@ void MaxFlow::InitializeActiveNodeStack() { const NodeIndex node = node_it.Index(); if (IsActive(node)) { active_nodes_.push(node); - VLOG(1) << "InitializeActiveNodeStack: node " << node << " added."; + VLOG(2) << "InitializeActiveNodeStack: node " << node << " added."; } } } @@ -263,7 +261,7 @@ void MaxFlow::Refine() { const NodeIndex node = active_nodes_.top(); active_nodes_.pop(); if (IsActive(node)) { - VLOG(1) << "Refine: calling Discharge for node " << node; + VLOG(2) << "Refine: calling Discharge for node " << node; Discharge(node); } } @@ -271,17 +269,17 @@ void MaxFlow::Refine() { void MaxFlow::Discharge(NodeIndex node) { DCHECK(IsActive(node)); - VLOG(1) << "Discharging node " << node << ", excess = " << node_excess_[node]; + VLOG(2) << "Discharging node " << node << ", excess = " << node_excess_[node]; while (IsActive(node)) { for (StarGraph::IncidentArcIterator arc_it(*graph_, node, first_admissible_arc_[node]); arc_it.Ok(); arc_it.Next()) { const ArcIndex arc = arc_it.Index(); - VLOG(2) << DebugString("Discharge: considering", arc); + VLOG(3) << DebugString("Discharge: considering", arc); if (IsAdmissible(arc)) { if (node_excess_[node] != 0) { - VLOG(1) << "Discharge: calling PushFlow."; + VLOG(2) << "Discharge: calling PushFlow."; const NodeIndex head = Head(arc); const bool head_active_before_push = IsActive(head); const FlowQuantity delta = std::min(node_excess_[node], @@ -314,7 +312,7 @@ void MaxFlow::Relabel(NodeIndex node) { min_height = std::min(min_height, node_potential_[Head(arc)]); } } - VLOG(1) << "Relabel: height(" << node << ") relabeled from " + VLOG(2) << "Relabel: height(" << node << ") relabeled from " << node_potential_[node] << " to " << min_height + 1; node_potential_.Set(node, min_height + 1); first_admissible_arc_.Set(node, GetFirstIncidentArc(node)); diff --git a/graph/max_flow.h b/graph/max_flow.h index fd3cc7152d..ebb67cdaf9 100644 --- a/graph/max_flow.h +++ b/graph/max_flow.h @@ -133,6 +133,8 @@ class MaxFlow { MaxFlow(const StarGraph* graph, NodeIndex source, NodeIndex target); + virtual ~MaxFlow() {} + // Returns the graph associated to the current object. const StarGraph* graph() const { return graph_; } @@ -189,7 +191,7 @@ class MaxFlow { } } - private: + protected: // Returns true if arc is admissible. bool IsAdmissible(ArcIndex arc) const { return residual_arc_capacity_[arc] > 0 @@ -234,7 +236,7 @@ class MaxFlow { // Discharges an active node node by saturating its admissible adjacent arcs, // if any, and by relabelling it when it becomes inactive. - void Discharge(NodeIndex node); + virtual void Discharge(NodeIndex node); // Resets the first_admissible_arc_ array to the first incident arc of each // node. diff --git a/util/packed_array.h b/util/packed_array.h index b1bf37539e..f71992aee3 100644 --- a/util/packed_array.h +++ b/util/packed_array.h @@ -62,13 +62,14 @@ template class PackedArrayAllocator { size_in_bytes_(0), storage_() {} // Reserves memory for new minimum and new maximum indices. + // Returns true if the memory could be reserved. // Never shrinks the memory allocated. - void Reserve(int64 new_min_index, int64 new_max_index) { + bool Reserve(int64 new_min_index, int64 new_max_index) { DCHECK_LE(new_min_index, new_max_index); if (base_ != NULL && new_min_index >= min_index_ && new_max_index <= max_index_) { - return; + return true; } DCHECK(base_ == NULL || new_min_index <= min_index_); DCHECK(base_ == NULL || new_max_index >= max_index_); @@ -79,6 +80,9 @@ template class PackedArrayAllocator { const uint64 new_size_in_bytes = new_size * NumBytes + sizeof(int64) - NumBytes; // NOLINT byte* new_storage = new byte[new_size_in_bytes]; + if (new_storage == NULL) { + return false; + } byte* const new_base = new_storage - new_min_index * NumBytes; if (base_ != NULL) { @@ -91,6 +95,7 @@ template class PackedArrayAllocator { max_index_ = new_max_index; size_in_bytes_ = new_size_in_bytes; storage_.reset(reinterpret_cast(new_storage)); + return true; } int64 min_index() const { return min_index_; } @@ -127,7 +132,10 @@ template class PackedArray { // These can be positive or negative, and the value for minimum_index // and for maximun_index can be set and read (i.e. the bounds are inclusive.) PackedArray(int64 min_index, int64 max_index) : allocator_() { - Reserve(min_index, max_index); + if (!Reserve(min_index, max_index)) { + LOG(DFATAL) << "Could not reserve memory for indices ranging from " + << min_index << " to " << max_index; + } } ~PackedArray() {} @@ -197,8 +205,8 @@ template class PackedArray { // Reserves memory for new minimum and new maximum indices. // Never shrinks the memory allocated. - void Reserve(int64 new_min_index, int64 new_max_index) { - allocator_.Reserve(new_min_index, new_max_index); + bool Reserve(int64 new_min_index, int64 new_max_index) { + return allocator_.Reserve(new_min_index, new_max_index); } // Sets all the elements in the array to value. Set is bypassed to maximize @@ -238,7 +246,10 @@ template<> class PackedArray<4> { PackedArray() : allocator_() {} PackedArray(int64 min_index, int64 max_index) : allocator_() { - Reserve(min_index, max_index); + if (!Reserve(min_index, max_index)) { + LOG(DFATAL) << "Could not reserve memory for indices ranging from " + << min_index << " to " << max_index; + } } int64 min_index() const { return allocator_.min_index(); } @@ -269,9 +280,10 @@ template<> class PackedArray<4> { } // Reserves memory for new minimum and new maximum indices. + // Returns true if the memory could be reserved. // Never shrinks the memory allocated. - void Reserve(int64 new_min_index, int64 new_max_index) { - allocator_.Reserve(new_min_index, new_max_index); + bool Reserve(int64 new_min_index, int64 new_max_index) { + return allocator_.Reserve(new_min_index, new_max_index); } // Sets all the elements in the array to value. @@ -300,7 +312,10 @@ template<> class PackedArray<8> { PackedArray() : allocator_() {} PackedArray(int64 min_index, int64 max_index) : allocator_() { - Reserve(min_index, max_index); + if (!Reserve(min_index, max_index)) { + LOG(DFATAL) << "Could not reserve memory for indices ranging from " + << min_index << " to " << max_index; + } } int64 min_index() const { return allocator_.min_index(); } @@ -329,9 +344,10 @@ template<> class PackedArray<8> { } // Reserves memory for new minimum and new maximum indices. + // Returns true if the memory could be reserved. // Never shrinks the memory allocated. - void Reserve(int64 new_min_index, int64 new_max_index) { - allocator_.Reserve(new_min_index, new_max_index); + bool Reserve(int64 new_min_index, int64 new_max_index) { + return allocator_.Reserve(new_min_index, new_max_index); } // Sets all the elements in the array to value.