Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PartOf recursive #164

Closed
rubendel opened this issue Jun 2, 2023 · 9 comments
Closed

PartOf recursive #164

rubendel opened this issue Jun 2, 2023 · 9 comments
Labels
Discussion documentation Improvements or additions to documentation
Milestone

Comments

@rubendel
Copy link
Contributor

rubendel commented Jun 2, 2023

The documentation currently does not mention whether PartOf should be checked recursively. I think it should, but would be good to discuss and then document.

Example: Checking whether objects are part of a building storey, naively only checking the direct parent through the IfcRelContainedInSpatialStructure will work in a lot of cases, but not when objects are part of an assembly or other grouping objects.

https://github.com/buildingSMART/IDS/blob/master/Documentation/partof-facet.md

@rubendel rubendel added documentation Improvements or additions to documentation Discussion labels Jun 2, 2023
@rubendel rubendel added this to the 1.0 milestone Jun 2, 2023
@rubendel
Copy link
Contributor Author

rubendel commented Jun 2, 2023

I guess it could also be valuable to allow recursiveness to be set on the PartOf facet itself in IDS

@rubendel
Copy link
Contributor Author

rubendel commented Jun 2, 2023

Actually it gets a little more complicated, consider the following situation:

IfcDoor -> IfcRelAggregates -> IfcCurtainWall -> IfcRelContainedInSpatialStructure -> IfcBuildingStorey

If you were to build an IDS to check whether the IfcDoor are all part of an IfcBuildingStorey, how would you do that?

My initial thought would be to always (for all 4 types of aggregations defined in IDS) incorporate traversal via IfcRelAggregates, and then from any object encountered do the actual PartOf check. This would also be valuable for example for systems. But it kind of depends a bit on the typical use cases...

@janbrouwer
Copy link
Contributor

Interesting case... Maybe just walk DOWN the spatial tree until the first match of the specified relationship? When checking IfcDoor for Relation IfcRelContainedInSpatialStructure, just ignore all relationships inbetween and stop at IfcBuildingStorey.
But for IfcRelAssignsToGroup it would be the other way around. You would have to walk UP the tree to find the IfcFlowTerminal that is part of a IfcSystem through a grouped IfcElementAssembly

@rubendel
Copy link
Contributor Author

rubendel commented Jun 2, 2023

Thanks for chiming in Jan. I don't really understand what you mean with walking down the tree though, I think in all cases we are interested in walking up. The problem is that there is no IfcRelContainedInSpatialStructure from IfcDoor to IfcCurtainWall in this particular case, but I would say that logically the IfcDoor is definitely in an IfcBuildingStorey.

Thinking about it, in a lot of use cases, I would say the additional "relation" field in IDS makes things a lot more complicated. Why would we not just always check all 4 routes, but maybe I am thinking too simplisticly.

@pjanck
Copy link
Contributor

pjanck commented Jun 6, 2023

  1. Prohibited facet

Does this mean, that a prohibited partOf facet would make any object invalid, that is in any way related to the type of parent in question?

Example: prohibited partOf with IfcRelContainedInSpatialStructure applied to IfcDoor with check being IfcSite.
In other words: no IfcDoor shall be contained in IfcSite. This could/would invalidate most of 2x3 files:

Example: IfcDoor -> IfcRelContainedInSpatialStructure -> IfcBuildingStorey -> IfcRelAggregates -> IfcBuilding -> IfcRelAggregates -> IfcSite would fail in this case.

  1. I agree with previous posts: I wonder which relationships should count for 'recursion'. What is the reasoning to choose one but not the other?

Why would we not just always check all 4 routes

There aren't only 4 IfcRelationships in the IFC data model. I fear there would be unwanted side-effects of such policy. Also, I fail to recognize what common denominator the 4 chosen relationships have.

incorporate traversal via IfcRelAggregates, and then from any object encountered do the actual PartOf check

This proposes to only apply "recursion" with IfcRelAggregates. It feels hard-coded, but somewhat clearer for its semantic meaning.

My opinion: I would not support recursion of any sorts. Out of the two options above, I could live with the latter. Also, in this case, I would greet such addition:

allow recursiveness to be set on the PartOf facet itself in IDS

@aothms
Copy link

aothms commented Jun 6, 2023

This could/would invalidate most of 2x3 files:

In itself that's not an argument against enabling recursion. prohibit entity IfcProject would also make every ifc2x3 file invalid. In your case it just means that the distinction between an IfcDoor contained in the Site directly cannot be distinguished from contained in the Site indirectly, which might be a valid limitation for v1 if we accept it.

Also, I fail to recognize what common denominator the 4 chosen relationships have.

I think they are just the four type of relationships that came up during the use case analysis as relationships that are meaningful to enforce/specify for end-users.

If I would picture me as an end user I think I would like partOf to be transitive. If A is contained in B, B is contained in C. Then yes, A is contained in C.

I find the combination of the various relationships indeed very challenging. A <- Aggregates -> B <- Contains -> C, is really hard to distinguish from A <- Contains -> B <- Aggregates -> C or from just A <- Contains -> C. From that point of view I'm quite sympathetic to @rubendel's argument to just arbitrary and recursively keep traversing these 4 relationships until you arrive at the specified entity.

It supports the lowest handing fruit kind of use cases I imagined for this feature.

  • Something must be part of a IfcSystem
  • IfcFurnitureElement must be part of an IfcSpace
  • A IfcBuildingElementPart that is part of an IfcWall must exist

But indeed not use cases related to directly being contained in something. So instead of saying:

  • An IfcDoor cannot be directly contained in the IfcSite

you have to say

  • An IfcDoor needs to be contained in an IfcBuilding

Given how project specific IDS's are I don't think that'd pose an immediate problem given the other kinds of facilities in IFC, but it is a limitation.

@berlotti
Copy link
Member

Proposal: arbitrary and recursively keep traversing these 4 relationships until you arrive at the specified entity.

This means we need to remove the enumeration of ifcrelaggregates, ifcrelassignstogroup, ifcrelcontainedinspatialstructure and ifcrelnests.

@berlotti
Copy link
Member

berlotti commented Jun 16, 2023

After thinking about it a bit longer, Proposal 2: turn 'relation' attribute into optional. When 'partof' is used it can be any recursive path of any relationship to the entity; when explicitly defined it should be a direct relation (or recursive but only with that same relationtype).

@berlotti
Copy link
Member

berlotti commented Jul 5, 2023

@berlotti berlotti closed this as completed Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

5 participants