Skip to content

fix bucket_sorter bug affecting min_degree_ordering #86

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@ kevin-bacon2.dat
random.dot
triangular-fr.dot
triangular-kk.dot

# backup files
*~
13 changes: 1 addition & 12 deletions include/boost/graph/minimum_degree_ordering.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -317,9 +317,6 @@ namespace detail
DegreeListsMarker;
typedef Marker< diff_t, vertex_t, VertexIndexMap > MarkerP;

// Data Members
bool has_no_edges;

// input parameters
Graph& G;
int delta;
Expand All @@ -343,8 +340,7 @@ namespace detail
mmd_impl(Graph& g, size_type n_, int delta, DegreeMap degree,
InversePermutationMap inverse_perm, PermutationMap perm,
SuperNodeMap supernode_size, VertexIndexMap id)
: has_no_edges(true)
, G(g)
: G(g)
, delta(delta)
, degree(degree)
, inverse_perm(inverse_perm)
Expand All @@ -371,8 +367,6 @@ namespace detail
{
typename Traits::degree_size_type d = out_degree(*v, G);
put(degree, *v, d);
if (0 < d)
has_no_edges = false;
degreelists.push(*v);
}
}
Expand All @@ -393,11 +387,6 @@ namespace detail
list_isolated.pop();
}

if (has_no_edges)
{
return;
}

size_type min_degree = 1;
typename DegreeLists::stack list_min_degree
= degreelists[min_degree];
Expand Down
48 changes: 30 additions & 18 deletions include/boost/pending/bucket_sorter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// Revision History:
// 13 June 2001: Changed some names for clarity. (Jeremy Siek)
// 01 April 2001: Modified to use new <boost/limits.hpp> header. (JMaddock)
// 28 Feb 2017: change bucket head, fix bug in remove. (Felix Salfelder)
//
#ifndef BOOST_GRAPH_DETAIL_BUCKET_SORTER_HPP
#define BOOST_GRAPH_DETAIL_BUCKET_SORTER_HPP
Expand Down Expand Up @@ -40,9 +41,9 @@ class bucket_sorter
bucket_sorter(size_type _length, bucket_type _max_bucket,
const Bucket& _bucket = Bucket(),
const ValueIndexMap& _id = ValueIndexMap())
: head(_max_bucket, invalid_value())
, next(_length, invalid_value())
: next(_length+_max_bucket, invalid_value())
, prev(_length, invalid_value())
, head(next.size()?(next.begin()+_length):next.end())
, id_to_value(_length)
, bucket(_bucket)
, id(_id)
Expand All @@ -58,11 +59,8 @@ class bucket_sorter
// check if i is the end of the bucket list
if (next_node != invalid_value())
prev[next_node] = prev_node;
// check if i is the begin of the bucket list
if (prev_node != invalid_value())
next[prev_node] = next_node;
else // need update head of current bucket list
head[bucket[x]] = next_node;
// update predecessor
next[prev_node] = next_node;
}

void push(const value_type& x)
Expand Down Expand Up @@ -95,24 +93,38 @@ class bucket_sorter
public:
stack(bucket_type _bucket_id, Iter h, Iter n, Iter p, IndexValueMap v,
const ValueIndexMap& _id)
: bucket_id(_bucket_id), head(h), next(n), prev(p), value(v), id(_id)
{
}
#if defined(BOOST_CLANG) && (1 == BOOST_CLANG) && defined(__APPLE_CC__)
: bucket_id(_bucket_id), head(), next(), prev(), value(v), id(_id)
{
head = h;
next = n;
prev = p;
}
#else
: bucket_id(_bucket_id), head(h), next(n), prev(p), value(v), id(_id) {}
#endif

// Avoid using default arg for ValueIndexMap so that the default
// constructor of the ValueIndexMap is not required if not used.
stack(bucket_type _bucket_id, Iter h, Iter n, Iter p, IndexValueMap v)
: bucket_id(_bucket_id), head(h), next(n), prev(p), value(v)
{
}
#if defined(BOOST_CLANG) && (1 == BOOST_CLANG) && defined(__APPLE_CC__)
: bucket_id(_bucket_id), head(), next(), prev(), value(v)
{
head = h;
next = n;
prev = p;
}
#else
: bucket_id(_bucket_id), head(h), next(n), prev(p), value(v) {}
#endif

void push(const value_type& x)
{
const size_type new_head = get(id, x);
const size_type current = head[bucket_id];
if (current != invalid_value())
prev[current] = new_head;
prev[new_head] = invalid_value();
prev[new_head] = bucket_id + (head - next);
next[new_head] = current;
head[bucket_id] = new_head;
}
Expand All @@ -122,7 +134,7 @@ class bucket_sorter
size_type next_node = next[current];
head[bucket_id] = next_node;
if (next_node != invalid_value())
prev[next_node] = invalid_value();
prev[next_node] = bucket_id + (head - next);
}
value_type& top() { return value[head[bucket_id]]; }
const value_type& top() const { return value[head[bucket_id]]; }
Expand All @@ -139,15 +151,15 @@ class bucket_sorter

stack operator[](const bucket_type& i)
{
assert(i < head.size());
return stack(i, head.begin(), next.begin(), prev.begin(),
assert(i < next.size());
return stack(i, head, next.begin(), prev.begin(),
id_to_value.begin(), id);
}

protected:
std::vector< size_type > head;
std::vector< size_type > next;
std::vector< size_type > prev;
typename std::vector<size_type>::iterator head;
std::vector< value_type > id_to_value;
Bucket bucket;
ValueIndexMap id;
Expand Down
1 change: 1 addition & 0 deletions test/Jamfile.v2
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ alias graph_test_with_filesystem :
../../filesystem/build
../../system/build
: $(PLANAR_INPUT_FILES) ]
[ run pending/bucket_sorter_2u.cpp ]
;

test-suite graph_test :
Expand Down
53 changes: 53 additions & 0 deletions test/pending/bucket_sorter_2u.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
#include <boost/property_map/property_map.hpp>
#include <boost/pending/bucket_sorter.hpp>
#include <boost/test/minimal.hpp>

#include <vector>

int test_main(int, char**)
{
typedef boost::iterator_property_map<unsigned*,
boost::identity_property_map, unsigned, unsigned&> map_type;
typedef boost::bucket_sorter<unsigned, unsigned,
map_type, boost::identity_property_map > container_type;

std::vector<unsigned> V(10);
map_type M(&V[0], boost::identity_property_map());

container_type B(10, 10, M);

for(unsigned i=4; i<10; ++i){
V[i]=9;
B.push(i);
}
BOOST_CHECK(B[9].top()==9);

// intended use?
V[6]=3;
B.update(6);

while(!B[9].empty()){
B[9].pop();
}

V[0]=0;
B.push(0);

V[2]=1; /// ***
B.push(2);

BOOST_CHECK(B[1].top()==2);

V[0]=1;
// intended use? kills 2 in bucket #1 ***
B.update(0);

container_type::value_type const & top1 = B[1].top();

B[1].pop();
container_type::value_type const &top2 = B[1].top();

BOOST_CHECK(top1 == 2 || top2 == 2);

return 0;
}