buildingSMART / IFC4.3.x-development

Repository to collect updates to the IFC4.3 Specification
Other
170 stars 86 forks source link

Why is IfcRelContainedInSpatialStructure referring to IfcProduct and not IfcElement? #582

Open Moult opened 1 year ago

Moult commented 1 year ago

According to https://ifc43-docs.standards.buildingsmart.org/IFC/RELEASE/IFC4x3/HTML/lexical/IfcRelContainedInSpatialStructure.htm RelatedElements is SET [1:?] OF IfcProduct with the explanation:

IFC2x-CHANGE The data type has been changed from IfcElement to IfcProduct with upward compatibility

I don't understand the logic behind this. Why isn't it IfcElement? IfcElement has the inverse attribute ContainedInStructure whereas other IfcProduct subtypes do not.

Brought up by @Andrej730

aothms commented 1 year ago

This is for things like IfcGrid and now IfcAlignment, which are IfcProduct descendants but not IfcElement.

Asymmetric inverses are indeed the devil, but what you see here is that e.g that IfcPositioningElement also has that same inverse ContainedInStructure. So it's not really assymetric, but "partial".

It would maybe be good to make it symmetric by making the IfcProduct subtypes that don't have the inverse attribute not allowed to take part in the relationship (now it just forbids spatial-structure-elem, but what about port etc.) by means of a where rule (or not to use the IfcProduct supertype, but rather a select of all relevant subtypes).

So I think this answers the question, but is there a solution needed?

Andrej730 commented 1 year ago

@aothms the problem where I actually met this inconsistency was me trying to get list of inverse attributes for some ifc entity (the method is below) and I came across the ContainedInStructure being actually IfcProduct inverse attribute when here it belongs to IfcElement.

So the original question is - ContainedInStructure should inverse attribute of IfcElement or IfcProduct?

The method I've used:

def get_inverse_attributes(el):
    inverse_attrs = []
    for a in el.all_inverse_attributes():
        attribute_type = a.attribute_reference().type_of_attribute()
        if isinstance(attribute_type, ifcopenshell.ifcopenshell_wrapper.aggregation_type):
            attribute_type = attribute_type.type_of_element()
        attribute_type = attribute_type.declared_type()

        if not isinstance(attribute_type, ifcopenshell.ifcopenshell_wrapper.entity):
            attr_types = [t.name() for t in attribute_type.select_list()]
        else:
            attr_types = [attribute_type.name()]

        if el.name() in attr_types:
            inverse_attrs.append(a)
    return inverse_attrs
aothms commented 1 year ago

Ok, I don't fully understand the aims here. But yeah, you'd have to take into account this occasional asymmetry.

Regarding the code, there are some schema complexities that are not considered:

# aggregate of aggregate is also possible so this can be a while loop to keep unpacking the aggregate to element type
if isinstance(attribute_type, ifcopenshell.ifcopenshell_wrapper.aggregation_type):

# better make this explicitly if isinstance(attribute_type, ifcopenshell.ifcopenshell_wrapper.select_type)
# but selects can also select other selects so this probably also needs to be recursive
if not isinstance(attribute_type, ifcopenshell.ifcopenshell_wrapper.entity):