ifcopenshell.util.element: dedupe SET-typed attributes in replace_attribute

replace_attribute() rewrites references inside aggregate attributes via
element.walk(), but never checked whether the replacement value was
already present elsewhere in the same aggregate. For an EXPRESS SET
(e.g. IfcProject.RepresentationContexts, IfcRelAggregates.RelatedObjects)
this can leave the same reference listed twice, which is invalid IFC.
LIST and BAG aggregates legitimately allow duplicates, so a blanket dedup
would be wrong; only SET-typed attributes are deduplicated, determined at
runtime from the schema declaration (IfcOpenShell#8706 review comment).

The SET/LIST/BAG check is cached per (schema, class, attribute index), and
the dedup pass itself only runs when a cheap linear pre-check finds the
replacement value already present in the aggregate, so the common case
(no duplicate produced) pays only that pre-check, not a hash-set rebuild.
Benchmarked against a 23MB (431k entities) and a 104MB (2.4M entities) IFC
model against a large SET attribute: worst case adds well under 1ms per
call; the realistic case (merging duplicate contexts, matching the PR
#8706 scenario) shows no measurable regression.

Fixes the root cause flagged in IfcOpenShell#8706 (Moult), obviating the
need for MergeDuplicateContexts' own manual aggregate-dedup pass for that
scenario.

Generated with the assistance of an AI coding tool.
This commit is contained in:
Petru Conduraru
2026-07-20 15:54:59 +03:00
committed by Dion Moult
parent 8fb8966094
commit 8c434b7167
2 changed files with 45 additions and 1 deletions
@@ -16,12 +16,14 @@
# You should have received a copy of the GNU Lesser General Public License
# along with IfcOpenShell. If not, see <http://www.gnu.org/licenses/>.
import functools
from collections import namedtuple
from collections.abc import Callable, Generator, Sequence
from typing import Any, Literal, Optional, Union, overload
import ifcopenshell
import ifcopenshell.guid
import ifcopenshell.ifcopenshell_wrapper
import ifcopenshell.util.element
import ifcopenshell.util.representation
@@ -1590,10 +1592,36 @@ def replace_element(element: ifcopenshell.entity_instance, replacement: ifcopens
replace_attribute(inverse, element, replacement)
@functools.cache
def _is_set_attribute(schema_identifier: str, ifc_class: str, index: int) -> bool:
"""Whether attribute `index` of `ifc_class` is declared as an EXPRESS SET.
SET aggregates may not contain duplicate members, whereas LIST and BAG
aggregates may, so only SET-typed attributes are safe to deduplicate.
"""
declaration = ifcopenshell.ifcopenshell_wrapper.schema_by_name(schema_identifier).declaration_by_name(ifc_class)
attribute = declaration.attribute_by_index(index)
aggregation = attribute.type_of_attribute().as_aggregation_type()
return aggregation is not None and aggregation.type_of_aggregation_string() == "set"
def replace_attribute(element: ifcopenshell.entity_instance, old: Any, new: Any) -> None:
for i, attribute_value in enumerate(element):
if has_element_reference(attribute_value, old):
element[i] = element.walk(lambda v: v == old, lambda v: new, attribute_value)
new_value = element.walk(lambda v: v == old, lambda v: new, attribute_value)
if (
isinstance(attribute_value, tuple)
and has_element_reference(attribute_value, new)
and _is_set_attribute(element.file.schema_identifier, element.is_a(), i)
):
seen: set[Any] = set()
deduplicated = []
for v in new_value:
if v not in seen:
seen.add(v)
deduplicated.append(v)
new_value = tuple(deduplicated)
element[i] = new_value
def has_element_reference(value: Any, element: ifcopenshell.entity_instance) -> bool:
@@ -1244,6 +1244,22 @@ class TestReplaceAttributeIFC4(test.bootstrap.IFC4):
subject.replace_attribute(rel, old, new)
assert rel.RelatedObjects == (new,)
def test_replacing_into_a_set_deduplicates_the_survivor(self):
old = self.file.createIfcWall()
new = self.file.createIfcWall()
rel = self.file.createIfcRelAggregates()
rel.RelatedObjects = [old, new]
subject.replace_attribute(rel, old, new)
assert rel.RelatedObjects == (new,)
def test_replacing_into_a_list_keeps_legitimate_duplicates(self):
p1 = self.file.createIfcCartesianPoint((0.0, 0.0, 0.0))
p2 = self.file.createIfcCartesianPoint((1.0, 0.0, 0.0))
p3 = self.file.createIfcCartesianPoint((2.0, 0.0, 0.0))
polyline = self.file.createIfcPolyline([p1, p2, p3, p1])
subject.replace_attribute(polyline, p2, p3)
assert polyline.Points == (p1, p3, p3, p1)
class TestHasElementReferenceIFC4(test.bootstrap.IFC4):
def test_if_a_element_attribute_references_another_element(self):