Fixes a (major) bug (by cleaning up code)

lib/create_adaptive_distinguishing_sequence.cpp

@@ -1,10 +1,9 @@
 #include "create_adaptive_distinguishing_sequence.hpp"
-#include "logging.hpp"
 #include "splitting_tree.hpp"
 
 #include <algorithm>
+#include <cassert>
 #include <functional>
-#include <iostream>
 #include <queue>
 #include <vector>
 
@@ -27,7 +26,6 @@ result2 create_adaptive_distinguishing_sequence(const result & splitting_tree){
 			work2.pop();
 
 			if(node.CI.size() < 2) continue;
-			if(node.depth > 500) continue;
 
 			vector<bool> states(N, false);
 			for(auto && state : node.CI){
@@ -61,7 +59,7 @@ result2 create_adaptive_distinguishing_sequence(const result & splitting_tree){
 					}
 				}
 
-				// woops. fixme
+				// FIXME: this should/could be done without sorting...
 				sort(begin(new_c.CI), end(new_c.CI));
 
 				if(!new_c.CI.empty()){
@@ -69,11 +67,7 @@ result2 create_adaptive_distinguishing_sequence(const result & splitting_tree){
 				}
 			}
 
-			// FIXME: can not happen????
-			if(node.children.size() == 1) {
-				fire_once([]{ cerr << "WARNING: Only one child in dist seq\n"; });
-				continue;
-			}
+			assert(node.children.size() > 1);
 
 			for(auto & c : node.children) {
 				work2.push(c);

lib/create_splitting_tree.cpp

@@ -1,8 +1,7 @@
 #include "create_splitting_tree.hpp"
-#include "logging.hpp"
 
 #include <functional>
-#include <iostream>
+#include <numeric>
 #include <queue>
 #include <utility>
 
@@ -22,7 +21,6 @@ result create_splitting_tree(const Mealy& g){
 	const auto Q = g.output_indices.size();
 
 	result r(N);
-	auto & part = r.partition;
 	auto & root = r.root;
 	auto & succession = r.successor_cache;
 
@@ -31,33 +29,31 @@ result create_splitting_tree(const Mealy& g){
 	 * tree. We keep track of how many times we did no work. If this is too
 	 * much, there is no complete splitting tree.
 	 */
-	queue<pair<partition_refine::BlockRef, reference_wrapper<splijtboom>>> work;
+	queue<reference_wrapper<splijtboom>> work;
 	size_t days_without_progress = 0;
 
 	// Some lambda functions capturing some state, makes the code a bit easier :)
-	const auto push = [&work](auto br, auto & sp) { work.push({br, sp}); };
-	const auto pop = [&work]() { const auto r = work.front(); work.pop(); return r; };
-	const auto add_push_new_block = [&](auto new_blocks, auto & boom) {
-		const auto nb = distance(new_blocks.first, new_blocks.second);
-		boom.children.assign(nb, splijtboom(0, boom.depth + 1));
+	const auto add_push_new_block = [&work](auto new_blocks, auto & boom) {
+		boom.children.assign(new_blocks.size(), splijtboom(0, boom.depth + 1));
 
 		auto i = 0;
-		while(new_blocks.first != new_blocks.second){
-			for(auto && s : *new_blocks.first){
-				boom.children[i].states.push_back(s);
-			}
-
-			push(new_blocks.first++, boom.children[i++]);
+		for(auto && b : new_blocks){
+			boom.children[i++].states.assign(begin(b), end(b));
 		}
+
+		for(auto && c : boom.children){
+			work.push(c);
+		}
+
+		assert(boom.states.size() == accumulate(begin(boom.children), end(boom.children), 0, [](auto l, auto r) { return l + r.states.size(); }));
 	};
 	const auto is_valid = [N, &g](auto blocks, auto symbol){
 		for(auto && block : blocks) {
-			partition_refine s_part(block);
-			const auto new_blocks = s_part.refine(*s_part.begin(), [symbol, &g](state state){
+			const auto new_blocks = partition_(begin(block), end(block), [symbol, &g](state state){
 				return apply(g, state, symbol).to.base();
 			}, N);
 			for(auto && new_block : new_blocks){
-				if(distance(new_block.begin(), new_block.end()) != 1) return false;
+				if(new_block.size() != 1) return false;
 			}
 		}
 		return true;
@@ -68,18 +64,17 @@ result create_splitting_tree(const Mealy& g){
 	};
 
 	// We'll start with the root, obviously
-	push(part.begin(), root);
+	work.push(root);
 	while(!work.empty()){
-		const auto block_boom = pop();
-		const auto block = block_boom.first;
-		splijtboom & boom = block_boom.second;
+		splijtboom & boom = work.front();
+		work.pop();
 		const auto depth = boom.depth;
 
 		if(boom.states.size() == 1) continue;
 
 		// First try to split on output
 		for(input symbol = 0; symbol < P; ++symbol){
-			const auto new_blocks = part.refine(*block, [symbol, depth, &g, &update_succession](state state){
+			const auto new_blocks = partition_(begin(boom.states), end(boom.states), [symbol, depth, &g, &update_succession](state state){
 				const auto ret = apply(g, state, symbol);
 				update_succession(state, ret.to, depth);
 				return ret.output.base();
@@ -93,8 +88,7 @@ result create_splitting_tree(const Mealy& g){
 
 			// a succesful split, update partition and add the children
 			boom.seperator = {symbol};
-			const auto range = part.replace(block, move(new_blocks));
-			add_push_new_block(range, boom);
+			add_push_new_block(new_blocks, boom);
 
 			goto has_split;
 		}
@@ -102,7 +96,7 @@ result create_splitting_tree(const Mealy& g){
 		// Then try to split on state
 		for(input symbol = 0; symbol < P; ++symbol){
 			vector<bool> successor_states(N, false);
-			for(auto && state : *block){
+			for(auto && state : boom.states){
 				successor_states[apply(g, state, symbol).to.base()] = true;
 			}
 
@@ -115,7 +109,7 @@ result create_splitting_tree(const Mealy& g){
 
 			// possibly a succesful split, construct the children
 			const auto word = concat({symbol}, oboom.seperator);
-			const auto new_blocks = part.refine(*block, [word, depth, &g, &update_succession](state state){
+			const auto new_blocks = partition_(begin(boom.states), end(boom.states), [word, depth, &g, &update_succession](state state){
 				const auto ret = apply(g, state, begin(word), end(word));
 				update_succession(state, ret.to, depth);
 				return ret.output.base();
@@ -124,15 +118,11 @@ result create_splitting_tree(const Mealy& g){
 			// not a valid split -> continue
 			if(!is_valid(new_blocks, symbol)) continue;
 
-			if(new_blocks.size() == 1){
-				fire_once([]{ cerr << "WARNING: Refinement did not give finer partition, can not happen\n"; });
-				continue;
-			}
+			assert(new_blocks.size() > 1);
 
 			// update partition and add the children
 			boom.seperator = word;
-			const auto range = part.replace(block, move(new_blocks));
-			add_push_new_block(range, boom);
+			add_push_new_block(new_blocks, boom);
 
 			goto has_split;
 		}
@@ -142,7 +132,7 @@ result create_splitting_tree(const Mealy& g){
 			r.is_complete = false;
 			return r;
 		}
-		push(block, boom);
+		work.push(boom);
 		continue;
 
 		has_split:

lib/create_splitting_tree.hpp

@@ -9,7 +9,6 @@
 struct result {
 	result(size_t N)
 	: root(N, 0)
-	, partition(N)
 	, successor_cache()
 	, is_complete(true)
 	{}
@@ -17,9 +16,6 @@ struct result {
 	// The splitting tree as described in Lee & Yannakakis
 	splijtboom root;
 
-	// The running partition of states
-	partition_refine partition;
-
 	// Encodes f_u : depth -> state -> state, where only the depth of u is of importance
 	std::vector<std::vector<state>> successor_cache;
 

lib/partition.hpp

@@ -1,99 +1,38 @@
 #pragma once
 
+#include <cassert>
 #include <list>
 #include <numeric>
+#include <stdexcept>
 #include <type_traits>
-#include <utility>
 #include <vector>
 
-using Elements = std::list<size_t>;
+template <typename Iterator, typename Fun>
+auto partition_(Iterator b, Iterator e, Fun && function, size_t output_size) {
+	using namespace std;
+	using T = typename decay<decltype(*b)>::type;
 
-struct Block {
-	Elements::iterator first;
-	Elements::iterator second;
+	list<T> elements(b, e);
+	const auto s = elements.size();
 
-	auto begin() const { return first; }
-	auto end() const { return std::next(second); }
+	list<list<T>> blocks;
+	using ref = typename list<list<T>>::iterator;
+	vector<ref> A(output_size);
 
-	bool operator==(Block const & rh) const {
-		return first == rh.first && second == rh.second;
-	}
-};
+	auto it = begin(elements);
+	auto ed = end(elements);
+	while(it != ed){
+		const auto current = it++;
+		const auto y = function(*current);
+		if(y >= output_size) throw runtime_error("Output is too big");
 
-struct partition_refine {
-	using Blocks = std::list<Block>;
-	using BlockRef = Blocks::iterator;
-
-	partition_refine(size_t n)
-	: elements(n, 0)
-	, blocks{{elements.begin(), --elements.end()}}
-	{
-		std::iota(elements.begin(), elements.end(), 0);
-	}
-
-	partition_refine(Block b)
-	: elements(b.begin(), b.end())
-	, blocks{{elements.begin(), --elements.end()}}
-	{}
-
-	partition_refine() = delete;
-	partition_refine(partition_refine const &) = delete;
-	partition_refine& operator=(partition_refine const &) = delete;
-	partition_refine(partition_refine&&) = default;
-	partition_refine& operator=(partition_refine&&) = default;
-
-	auto size() const { return blocks.size(); }
-	auto begin() { return blocks.begin(); }
-	auto end() { return blocks.end(); }
-
-	// Deprecated since there is no more O(1) lookup
-	auto find(size_t x){
-		for(auto & b : blocks){
-			auto it = std::find(b.begin(), b.end(), x);
-			if(it != b.begin()) return b;
+		auto & ar = A[y];
+		if(ar == ref{}){
+			ar = blocks.insert(blocks.end(), list<T>{});
 		}
-
-		return Block{};
+		ar->splice(ar->end(), elements, current);
 	}
 
-	template <typename F>
-	auto refine(Block br, F && function, size_t output_size){
-		static_assert(std::is_same<decltype(function(0)), size_t>::value, "Function should return size_t");
-		if(br == Block{}) throw std::logic_error("Empty block range");
-		if(br.first == elements.end() || br.second == elements.end()) throw std::logic_error("Invalid block range");
-
-		Blocks new_blocks;
-		std::vector<BlockRef> A(output_size);
-
-		auto it = br.begin();
-		auto ed = br.end();
-		while(it != ed){
-			const auto y = function(*it);
-			if(y >= output_size) throw std::runtime_error("Output is too big");
-
-			auto & ar = A[y];
-			if(ar == BlockRef{}){
-				ar = new_blocks.insert(new_blocks.end(), {it, it});
-				it++;
-			} else {
-				auto current = it++;
-				elements.splice(++ar->second, elements, current);
-				*ar = {ar->first, current};
-			}
-		}
-
-		return new_blocks;
-	}
-
-	auto replace(BlockRef br, Blocks new_blocks){
-		const auto it = blocks.erase(br);
-		const auto b = new_blocks.begin();
-		const auto e = std::prev(new_blocks.end());
-		blocks.splice(it, new_blocks);
-		return make_pair(b, std::next(e));
-	}
-
-private:
-	Elements elements;
-	Blocks blocks;
-};
+	assert(s == accumulate(begin(blocks), end(blocks), 0, [](auto && l, auto && r) { return l + r.size(); }));
+	return blocks;
+}

src/main.cpp

@@ -62,14 +62,7 @@ int main(int argc, char *argv[]){
 			work.push({word, c});
 	}
 
-	for(state i = 0; i < uios.size(); ++i){
-		cout << i << ":\t";
-		if(uios[i.base()].empty()) {
-			cout << "no sequence :(";
-		} else {
-			for(auto && c : uios[i.base()]) cout << c << " ";
-		}
-		cout << endl;
-	}
+	size_t uio_count = count_if(begin(uios), end(uios), [](auto && u){ return !u.empty(); });
+	cout << uio_count << " / " << uios.size() << endl;
 }
 

src/partition_test.cpp

@@ -1,31 +0,0 @@
-#include <partition.hpp>
-
-#include <iostream>
-
-using namespace std;
-
-int main(int argc, char *argv[]){
-	partition_refine p(50);
-
-	auto bs = p.refine(p.find(0), [](size_t x){ return x % 1; }, 1);
-	cout << bs.size() << endl;
-
-	bs = p.refine(p.find(0), [](size_t x){ return x % 2; }, 2);
-	cout << bs.size() << endl;
-
-	bs = p.refine(p.find(0), [](size_t x){ return x % 3; }, 3);
-	cout << bs.size() << endl;
-
-	bs = p.refine(p.find(0), [](size_t x){ return x % 5; }, 5);
-	cout << bs.size() << endl;
-
-	bs = p.refine(p.find(0), [](size_t x){ return x % 7; }, 7);
-	cout << bs.size() << endl;
-
-	for(int i = 0; i < 50; ++i){
-		cout << i << ":\t";
-		const auto block = p.find(i);
-		for(auto && x : block) cout << x << " ";
-		cout << endl;
-	}
-}