#1531 Breadth-first search and maintain insertion order on remove_deep()

src/ifcopenshell-python/ifcopenshell/file.py

@@ -359,19 +359,27 @@ class file(object):
             return [entity_instance(e, self) for e in self.wrapped_data.by_type(type)]
         return [entity_instance(e, self) for e in self.wrapped_data.by_type_excl_subtypes(type)]
 
-    def traverse(self, inst, max_levels=None):
+    def traverse(self, inst, max_levels=None, breadth_first=False):
         """Get a list of all referenced instances for a particular instance including itself
 
         :param inst: The entity instance to get all sub instances
         :type inst: ifcopenshell.entity_instance.entity_instance
         :param max_levels: How far deep to recursively fetch sub instances. None or -1 means infinite.
         :type max_levels: None|int
+        :param breadth_first: Whether to use breadth-first search, the default is depth-first.
+        :type max_levels: bool
         :returns: A list of ifcopenshell.entity_instance.entity_instance objects
         :rtype: list
         """
         if max_levels is None:
             max_levels = -1
-        return [entity_instance(e, self) for e in self.wrapped_data.traverse(inst.wrapped_data, max_levels)]
+        
+        if breadth_first:
+            fn = self.wrapped_data.traverse_breadth_first
+        else:
+            fn = self.wrapped_data.traverse
+        
+        return [entity_instance(e, self) for e in fn(inst.wrapped_data, max_levels)]
 
     def get_inverse(self, inst):
         """Return a list of entities that reference this entity

src/ifcopenshell-python/ifcopenshell/util/element.py

@@ -104,7 +104,7 @@ def has_element_reference(value, element):
 def remove_deep(ifc_file, element):
     # @todo maybe some sort of try-finally mechanism.
     ifc_file.batch()
-    subgraph = list(ifc_file.traverse(element))
+    subgraph = list(ifc_file.traverse(element, breadth_first=True))
     subgraph_set = set(subgraph)
     for ref in subgraph[::-1]:
         if ref.id() and len(set(ifc_file.get_inverse(ref)) - subgraph_set) == 0:

src/ifcparse/IfcFile.h

@@ -24,6 +24,10 @@
 #include <set>
 #include <iterator>
 #include <boost/unordered_map.hpp>
+#include <boost/multi_index_container.hpp>
+#include <boost/multi_index/sequenced_index.hpp>
+#include <boost/multi_index/ordered_index.hpp>
+#include <boost/multi_index/random_access_index.hpp>
 
 #include "ifc_parse_api.h"
 
@@ -132,7 +136,16 @@ private:
 
 	void build_inverses_(IfcUtil::IfcBaseClass*);
 
-	std::set<int> batch_deletion_ids_;
+	typedef boost::multi_index_container<
+		int,
+		boost::multi_index::indexed_by<
+			boost::multi_index::sequenced<>,
+			boost::multi_index::ordered_unique<
+				boost::multi_index::identity<int>
+			>
+		>
+	> batch_deletion_ids_t;
+	batch_deletion_ids_t batch_deletion_ids_;
 	bool batch_mode_ = false;
 	void process_deletion_();
 
@@ -220,6 +233,10 @@ public:
 	/// in the first function argument.
 	IfcEntityList::ptr traverse(IfcUtil::IfcBaseClass* instance, int max_level=-1);
 
+	/// Same as traverse() but maintains topological order by using a
+	/// breadth-first search
+	IfcEntityList::ptr traverse_breadth_first(IfcUtil::IfcBaseClass* instance, int max_level = -1);
+
 	IfcEntityList::ptr getInverse(int instance_id, const IfcParse::declaration* type, int attribute_index);
 
 	/// Marks entity as modified so that potential cache for it is invalidated.

src/ifcparse/IfcParse.cpp

@@ -1506,15 +1506,52 @@ void IfcFile::recalculate_id_counter() {
 	MaxId = (unsigned int)k;
 }
 
+class traversal_recorder {
+	IfcEntityList::ptr list_;
+	std::map<int, IfcEntityList::ptr> instances_by_level_;
+	int mode_;
+
+public:
+	traversal_recorder(int mode) : mode_(mode) {
+		if (mode == 0) {
+			list_.reset(new IfcEntityList);
+		}
+	};
+
+	void push_back(int level, IfcUtil::IfcBaseClass* instance) {
+		if (mode_ == 0) {
+			list_->push(instance);
+		} else {
+			auto& l = instances_by_level_[level];
+			if (!l) {
+				l.reset(new IfcEntityList);
+			}
+			l->push(instance);
+		}
+	}
+
+	IfcEntityList::ptr get_list() const {
+		if (mode_ == 0) {
+			return list_;
+		} else {
+			IfcEntityList::ptr l(new IfcEntityList);
+			for (auto& p : instances_by_level_) {
+				l->push(p.second);
+			}
+			return l;
+		}
+	}
+};
+
 class traversal_visitor {
 private:
 	std::set<IfcUtil::IfcBaseClass*>& visited_;
-	IfcEntityList::ptr& list_;
+	traversal_recorder& list_;
 	int level_;
 	int max_level_;
 
 public:
-	traversal_visitor(std::set<IfcUtil::IfcBaseClass*>& visited, IfcEntityList::ptr& list, int level, int max_level)
+	traversal_visitor(std::set<IfcUtil::IfcBaseClass*>& visited, traversal_recorder& list, int level, int max_level)
 		: visited_(visited)
 		, list_(list)
 		, level_(level)
@@ -1524,12 +1561,12 @@ public:
 	void operator()(IfcUtil::IfcBaseClass* inst);
 };
 
-void traverse_(IfcUtil::IfcBaseClass* instance, std::set<IfcUtil::IfcBaseClass*>& visited, IfcEntityList::ptr list, int level, int max_level) {
+void traverse_(IfcUtil::IfcBaseClass* instance, std::set<IfcUtil::IfcBaseClass*>& visited, traversal_recorder& list, int level, int max_level) {
 	if (visited.find(instance) != visited.end()) {
 		return;
 	}
 	visited.insert(instance);
-	list->push(instance);
+	list.push_back(level, instance);
 
 	if (level >= max_level && max_level > 0) return;
 
@@ -1543,9 +1580,18 @@ void traversal_visitor::operator()(IfcUtil::IfcBaseClass* inst) {
 
 IfcEntityList::ptr IfcParse::traverse(IfcUtil::IfcBaseClass* instance, int max_level) {
 	std::set<IfcUtil::IfcBaseClass*> visited;
-	IfcEntityList::ptr return_value(new IfcEntityList);
-	traverse_(instance, visited, return_value, 0, max_level);
-	return return_value;
+	traversal_recorder r(0);
+	traverse_(instance, visited, r, 0, max_level);
+	return r.get_list();
+}
+
+// I'm cheating this isn't breadth-first, but rather we record visited instances
+// keeping track of their rank and return a list ordered by rank. Is this equivalent?
+IfcEntityList::ptr IfcParse::traverse_breadth_first(IfcUtil::IfcBaseClass* instance, int max_level) {
+	std::set<IfcUtil::IfcBaseClass*> visited;
+	traversal_recorder r(1);
+	traverse_(instance, visited, r, 0, max_level);
+	return r.get_list();
 }
 
 /// @note: for backwards compatibility
@@ -1553,6 +1599,11 @@ IfcEntityList::ptr IfcFile::traverse(IfcUtil::IfcBaseClass* instance, int max_le
 	return IfcParse::traverse(instance, max_level);
 }
 
+/// @note: for backwards compatibility
+IfcEntityList::ptr IfcFile::traverse_breadth_first(IfcUtil::IfcBaseClass* instance, int max_level) {
+	return IfcParse::traverse_breadth_first(instance, max_level);
+}
+
 void IfcFile::mark_entity_as_modified(int /*id*/)
 {
 	by_ref_cached_.clear();
@@ -1825,7 +1876,7 @@ void IfcFile::removeEntity(IfcUtil::IfcBaseClass* entity) {
 		throw IfcParse::IfcException("Instance not part of this file");
 	}
 
-	batch_deletion_ids_.insert(id);
+	batch_deletion_ids_.push_back(id);
 
 	if (!batch_mode_) {
 		process_deletion_();
@@ -1834,7 +1885,7 @@ void IfcFile::removeEntity(IfcUtil::IfcBaseClass* entity) {
 
 void IfcFile::process_deletion_() {
 
-	for (auto& id : batch_deletion_ids_) {
+	for (auto& id : batch_deletion_ids_.get<0>()) {
 		auto entity = instance_by_id(id);
 
 		IfcEntityList::ptr references = instances_by_reference(id);
@@ -1980,10 +2031,10 @@ void IfcFile::process_deletion_() {
 	
 	if (batch_mode_) {
 		for (auto it = byref.begin(); it != byref.end();) {
-			bool do_delete = batch_deletion_ids_.find(it->first) != batch_deletion_ids_.end();
+			bool do_delete = batch_deletion_ids_.get<1>().find(it->first) != batch_deletion_ids_.get<1>().end();
 			if (!do_delete) {
 				it->second.erase(std::remove_if(it->second.begin(), it->second.end(), [this](int x) {
-					return batch_deletion_ids_.find(x) != batch_deletion_ids_.end();
+					return batch_deletion_ids_.get<1>().find(x) != batch_deletion_ids_.get<1>().end();
 				}), it->second.end());
 				do_delete = it->second.empty();
 			}

src/ifcparse/IfcParse.h

@@ -269,6 +269,8 @@ namespace IfcParse {
 	IFC_PARSE_API IfcEntityInstanceData* read(unsigned int i, IfcFile* t, boost::optional<unsigned> offset = boost::none);
 
 	IFC_PARSE_API IfcEntityList::ptr traverse(IfcUtil::IfcBaseClass* instance, int max_level = -1);
+
+	IFC_PARSE_API IfcEntityList::ptr traverse_breadth_first(IfcUtil::IfcBaseClass* instance, int max_level = -1);
 }
 
 IFC_PARSE_API std::ostream& operator<< (std::ostream& os, const IfcParse::IfcFile& f);