diff --git a/src/ifcpatch/ifcpatch/recipes/Optimise.py b/src/ifcpatch/ifcpatch/recipes/Optimise.py index b0043a7299..dd7e3bc84c 100644 --- a/src/ifcpatch/ifcpatch/recipes/Optimise.py +++ b/src/ifcpatch/ifcpatch/recipes/Optimise.py @@ -36,8 +36,10 @@ class Patcher: can usually be solved through other means. Consult the bonsai Add-on documentation on dealing with large models for more details. - Warning: this optimise recipe is very, very slow. Please consider using - RecycleNonRootedElements instead. + Warning: this optimise recipe is slower than RecycleNonRootedElements, + as it performs a full, transitive fold instead of a single pass. + Consider RecycleNonRootedElements first if a quicker, partial + optimisation is acceptable. Example: @@ -58,27 +60,30 @@ class Patcher: the set of all of its references contained in its attributes. """ for inst in self.file: - yield inst.id(), set(i.id() for i in self.file.traverse(inst)[1:] if i.id()) + yield inst.id(), set(i.id() for i in self.file.traverse(inst, max_levels=1)[1:] if i.id()) instance_mapping = {} - def map_value(v): + def map_value(v, as_key=False): """ - Recursive function which replicates an entity instance, with - its attributes, mapping references to already registered - instances. Indeed, because of the toposort we know that - forward attribute value instances are mapped before the instances - that reference them. + Recursive function which either replicates an entity instance + with its attributes mapped to already registered instances + (as_key=False), or builds a hashable canonical key for it + (as_key=True), reusing already-folded references instead of + re-expanding their attribute subtrees. """ if isinstance(v, (list, tuple)): - # lists are recursively traversed - return type(v)(map(map_value, v)) + return type(v)(map_value(item, as_key=as_key) for item in v) elif isinstance(v, ifcopenshell.entity_instance): if v.id() == 0: # express simple types are not part of the toposort and just copied + if as_key: + return ("__type__", v.is_a(), v[0]) return self.optimized_file.create_entity(v.is_a(), v[0]) - - return instance_mapping[v] + mapped = instance_mapping[v] + if as_key: + return ("__id__", mapped.id()) + return mapped else: # a plain python value can just be returned return v @@ -87,7 +92,7 @@ class Patcher: for id in toposort(dict(generate_instances_and_references())): inst = self.file[id] - info = inst.get_info(include_identifier=False, recursive=True, return_type=frozenset) + info = map_value(inst.get_info(include_identifier=False, recursive=False, return_type=tuple), as_key=True) if info in info_to_id: mapped = instance_mapping[inst] = instance_mapping[self.file[info_to_id[info]]] diff --git a/src/ifcpatch/test/test_Optimise.py b/src/ifcpatch/test/test_Optimise.py new file mode 100644 index 0000000000..e4beaf122d --- /dev/null +++ b/src/ifcpatch/test/test_Optimise.py @@ -0,0 +1,73 @@ +# IfcOpenShell - IFC toolkit and geometry engine +# Copyright (C) 2026 Dion Moult +# +# This file is part of IfcOpenShell. +# +# IfcOpenShell is free software: you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# IfcOpenShell is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with IfcOpenShell. If not, see . + +# This file was generated with the assistance of an AI coding tool. + +import ifcopenshell +import ifcopenshell.guid + +import ifcpatch +import test.bootstrap + + +def add_context(f: ifcopenshell.file) -> ifcopenshell.entity_instance: + origin = f.createIfcAxis2Placement3D(f.createIfcCartesianPoint((0.0, 0.0, 0.0))) + return f.createIfcGeometricRepresentationContext(None, "Model", 3, 1.0e-05, origin, None) + + +def add_wall_with_curve(f: ifcopenshell.file, context, coords) -> ifcopenshell.entity_instance: + wall = f.create_entity("IfcWall", ifcopenshell.guid.new()) + points = [f.createIfcCartesianPoint(c) for c in coords] + polyline = f.createIfcPolyline(points) + rep = f.createIfcShapeRepresentation(context, "Body", "Curve2D", [polyline]) + wall.Representation = f.createIfcProductDefinitionShape(None, None, [rep]) + return wall + + +def curve_of(wall: ifcopenshell.entity_instance) -> ifcopenshell.entity_instance: + return wall.Representation.Representations[0].Items[0] + + +class TestOptimise(test.bootstrap.IFC4): + def test_folding_value_identical_non_rooted_entities(self): + # Two polylines built from separate but value-identical points. + context = add_context(self.file) + coords = [(0.0, 0.0), (1.0, 0.0), (1.0, 1.0), (0.0, 0.0)] + wall1 = add_wall_with_curve(self.file, context, coords) + wall2 = add_wall_with_curve(self.file, context, coords) + assert curve_of(wall1) != curve_of(wall2) + + output = ifcpatch.execute({"file": self.file, "recipe": "Optimise", "arguments": []}) + + assert len(output.by_type("IfcPolyline")) == 1 + + walls_after = output.by_type("IfcWall") + assert len(walls_after) == 2 + assert curve_of(walls_after[0]) == curve_of(walls_after[1]) + + def test_distinct_values_are_not_folded(self): + context = add_context(self.file) + wall1 = add_wall_with_curve(self.file, context, [(0.0, 0.0), (1.0, 0.0)]) + wall2 = add_wall_with_curve(self.file, context, [(0.0, 0.0), (2.0, 0.0)]) + assert curve_of(wall1) != curve_of(wall2) + + output = ifcpatch.execute({"file": self.file, "recipe": "Optimise", "arguments": []}) + + assert len(output.by_type("IfcPolyline")) == 2 + walls_after = output.by_type("IfcWall") + assert curve_of(walls_after[0]) != curve_of(walls_after[1])