Skip to content

Docs: PartOf accepts predefinedType, which the page never mentions - #450

Draft
BIMvoice wants to merge 2 commits into
buildingSMART:developmentfrom
BIMvoice:docs/partof-predefinedtype
Draft

Docs: PartOf accepts predefinedType, which the page never mentions#450
BIMvoice wants to merge 2 commits into
buildingSMART:developmentfrom
BIMvoice:docs/partof-predefinedtype

Conversation

@BIMvoice

@BIMvoice BIMvoice commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

partof-facet.md's Parameters table lists two parameters, Entity and Relation. The word predefinedType does not appear anywhere on the page.

But PartOf's nested entity is a full entityType. From Schema/ids.xsd:

<xs:complexType name="partOfType">
    <xs:sequence>
        <xs:element name="entity" type="ids:entityType" minOccurs="1"/>

That is the same complex type the Entity facet uses, so predefinedType is legal inside PartOf and behaves the same way. Meanwhile entity-facet.md documents it extensively, including the five-step resolution procedure and worked examples showing that predefinedType="USERDEFINED" matches the raw IFC attribute literally rather than the substituted ObjectType text. Issue #306 records that this literal match is deliberate and agreed.

So the rule is stated once, for one facet, and silently applies to another that never mentions the parameter exists.

Why we noticed

We hit it as a real defect. IfcTester's PartOf never handled the literal USERDEFINED case, at six separate sites, while its Entity facet did. The consequence was a false pass: an IfcSpace containing an IfcWall, checked with PartOf(predefinedType="USERDEFINED", cardinality="prohibited"), reported PASS when the correct verdict is FAIL. A prohibition silently satisfied, so a non-compliant model passes with nothing to prompt anyone to look.

Fixed on our side in IfcOpenShell#9203.

A gap in the test suite, which may matter more than the docs

We enumerated all 24 official PartOf test cases. None tests a literal USERDEFINED. Issue #307 added exactly that case for the Entity facet after #306 agreed the behaviour, but the equivalent was never added for PartOf.

So an implementer can pass the whole conformance suite with this bug present, which is what happened to us. Happy to contribute that test case if it would be useful; tell us the naming and placement you would want.

This change

One paragraph under the Parameters table, pointing at the Entity facet's existing rules rather than duplicating them, so there is one source of truth.

Reword freely to fit the manual's voice.

@BIMvoice

BIMvoice commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Following up on the test-suite gap, because it turns out to cost nothing.

I said we would be happy to contribute the missing PartOf USERDEFINED test cases and asked what naming you would want. That question is moot: the cases generate themselves in your own convention as a side effect of the IfcTester fix.

ifctester/test/ids_doc_generator.py imports test_facet and writes testcases/<facet>/<result>-<description>.ifc and .ids from its assertions:

basename = f"{result}-" + re.sub("[^0-9a-zA-Z]", "_", name.lower())
ifc.write(os.path.join(outdir, "testcases", self.facet, f"{basename}.ifc"))

So the existing suite is generated from IfcTester's own tests, which is exactly why the gap existed: PartOf had no USERDEFINED assertion, so no case was ever emitted, so no implementation was ever checked against it, including ours.

Our fix (IfcOpenShell#9203) adds assertions covering all six PartOf relation branches, both directions each:

An aggregate predefined type may specify USERDEFINED itself
A prohibited aggregate predefined type check fails a user-defined match
A group predefined type may specify USERDEFINED itself
A prohibited group predefined type check fails a user-defined match
The container predefined type may specify USERDEFINED itself
A prohibited container predefined type check fails a user-defined match
The nest predefined type may specify USERDEFINED itself
A prohibited nest predefined type check fails a user-defined match
A default (no-relation) predefined type may specify USERDEFINED itself
A prohibited default (no-relation) predefined type check fails a user-defined match

Regenerating after that merges produces the corresponding pass- and fail- case pairs automatically, named the same way as the existing 24.

Worth noting the prohibited half specifically. That is where the absence bit us: a prohibition on a USERDEFINED predefined type was silently satisfied, so a non-compliant model passed. A false pass is the failure mode a conformance suite exists to catch, and there was no case for it.

Nothing needed from you on this beyond the documentation change in this PR. Flagging it so you know the suite gap closes on its own rather than needing a separate contribution.

@BIMvoice

BIMvoice commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Correcting my own comment above, which was wrong.

I said the conformance test cases are generated from IfcTester's ids_doc_generator.py, and that the missing PartOf USERDEFINED cases would therefore appear here as a side effect of fixing IfcTester. That is not how this repository works.

@andyward pointed this out on #283: the .ids files are generated inside this repository from fenced code blocks in Documentation/ImplementersDocumentation/TestCases/scripts.md via the CreateTestCases build target, with hand maintained minimised .ifc files alongside. The IFC files were seeded from IfcOpenShell once, long ago, and have been maintained here since. Nothing in our repository regenerates your suite.

So the offer in that comment does not hold as written. If the PartOf USERDEFINED cases are still wanted, they need authoring here as fenced blocks plus IFC companions, the same way we have since done for #371, #435, #181 and #155 in PRs #451, #452, #453 and #457. Happy to do that for USERDEFINED too if it is useful.

Apologies for the noise. The documentation change in this PR is unaffected.

@CBenghi
CBenghi marked this pull request as draft August 9, 2026 15:51
@CBenghi CBenghi added the tests label Aug 9, 2026
@CBenghi

CBenghi commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

@BIMvoice,
From what I understand it is easy for you to generate the IFC files in IfcOpenShell.
If you can add them to this PR it would be trivial for me to add the fenced code blocks in the documentation to improve the test suite.

@BIMvoice

BIMvoice commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

@CBenghi Done, pushed as 77d754f. Six minimal IFC4 fixtures under TestCases/partof/, one pass and fail pair per scenario:

  • container via IfcRelContainedInSpatialStructure: IfcElementAssembly in an IfcSpace with PredefinedType=USERDEFINED, ObjectType='BURROW'
  • nest via IfcRelNests: IfcDiscreteAccessory under an IfcFurniture with PredefinedType=USERDEFINED, ObjectType='WATERBOTTLE'
  • default, no relation: same shape as the container pair

Each pass and fail file in a pair is deliberately identical content, matching how the existing prohibited pairs in that directory share a model. The custom ObjectType values are deliberately not the text "USERDEFINED", so a match can only succeed through the raw PredefinedType attribute rather than an accidental text match. Filenames follow the slug rule from the titles already in this thread.

All six verified functionally with the fixed IfcTester: required passes, prohibited fails. And to confirm the fail cases test what they claim, the three prohibited cases were also run against pre-fix IfcTester, where all three falsely pass. That is the silent-satisfaction bug these cases exist to catch.

To save you the boilerplate, here is the specification content we used for verification, ready to adapt into the fenced blocks. Container pair:

<!-- pass -->
<specification name="The container predefined type may specify USERDEFINED itself" ifcVersion="IFC4">
  <applicability maxOccurs="unbounded">
    <entity><name><simpleValue>IFCELEMENTASSEMBLY</simpleValue></name></entity>
  </applicability>
  <requirements>
    <partOf relation="IFCRELCONTAINEDINSPATIALSTRUCTURE">
      <entity>
        <name><simpleValue>IFCSPACE</simpleValue></name>
        <predefinedType><simpleValue>USERDEFINED</simpleValue></predefinedType>
      </entity>
    </partOf>
  </requirements>
</specification>

<!-- fail -->
<specification name="A prohibited container predefined type check fails a user-defined match" ifcVersion="IFC4">
  <applicability maxOccurs="unbounded">
    <entity><name><simpleValue>IFCELEMENTASSEMBLY</simpleValue></name></entity>
  </applicability>
  <requirements>
    <partOf relation="IFCRELCONTAINEDINSPATIALSTRUCTURE" cardinality="prohibited">
      <entity>
        <name><simpleValue>IFCSPACE</simpleValue></name>
        <predefinedType><simpleValue>USERDEFINED</simpleValue></predefinedType>
      </entity>
    </partOf>
  </requirements>
</specification>

For the nest variant, applicability targets IFCDISCRETEACCESSORY, the required entity is IFCFURNITURE, and relation="IFCRELNESTS". For the default variant, drop the relation attribute from both.

One thing worth knowing: there was no prior partOf fixture without a relation attribute anywhere in the suite, so the default pair is the first of its kind. Worth a second look from you on whether the no-relation semantics in the docs match what you want tested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

2 participants