search

buildingSMART / IDS

Computer interpretable (XML) standard to define Information Delivery Specifications for BIM (mainly used for IFC)
https://www.buildingsmart.org/standards/bsi-standards/information-delivery-specification-ids/
Other
215 stars 66 forks source link

Classification Facet behavior is not full #223

Open I-Sokolov opened 11 months ago

I-Sokolov commented 11 months ago

1) value: in case of multi-level classification reference - is it refer to top identification or identification at any level?

2) URI - is it refer to IfcClassification.Specification(Location) or to IfcExternalReference.Location

andyward commented 11 months ago
  1. My understanding is we should check the value at any level in the classification hierarchy (any ancestor). See this test case
  2. URI - This is presumably up to the spec author? But I'd assume a link to the particular classification would be most relevant, rather than the system. It's meta data not a testable attribute I believe
I-Sokolov commented 11 months ago

My understanding for 1 is the same, but I'd like to have it clearly declared in the doc. For 2 it should not be up to spec author because checker developer does not know his intentions:)

Moult commented 11 months ago

The URI is not part of the requirement, it's just metadata. Like the URI in other facets.

I-Sokolov commented 11 months ago

@Moult, I would disagree, in case of classification facet URI declared as parameter, not metadata

image

andyward commented 11 months ago

Probably needs clarifying in the docs, but my reading on it is that URI is a parameter you'd suggest be could specified by the author/authoring tool, to aid the ultimate users delivering models against the spec.

But I don't think there's any expectation it forms part of the verification parameters. At best it might be something take the user to for more information in the event of a verification failure?

I can also see a structured URI being highly useful to populate a requirement - and indeed some of the object libraries / PDTs services are outputting IDS, where it would make total sense to leave a URI link back to the master data. But there's no saying the URI will be structured. I've seen web pages, PDFs and XLS already in the URI.

Moult commented 11 months ago

Yeah I was the one who wrote that table and at the time I believe the intention was to encourage users to use the bSDD where possible to guide users on how to input data: https://github.com/buildingSMART/IDS/commit/4381e77164f120afbfc93bda9c49f83d17e4bb5c - so as you can see it covers classifications, materials, and properties. However the intention was never to actually audit the classification reference location field.

I'll be happy to update the docs to prevent this future confusion, sorry.

atomczak commented 2 days ago

The URI is not part of the requirement, it's just metadata. Like the URI in other facets.

I confirm, the URI is not part of the IDS check, just optional metadata to guide IDS users about the class definition. Documentation needs improvement.

My understanding is we should check the value at any level in the classification hierarchy (any ancestor). See this test case

Really?! So according to the test case if an IDS classification value requires '12' an IfcClassificationReference with a code '1234' will pass right now? In my opinion, it shouldn't. If someone wants to do such a check, they can use regex pattern '12.*'.

andyward commented 1 day ago

My understanding is we should check the value at any level in the classification hierarchy (any ancestor). See this test case

Really?! So test case if IDS classification value requires '12' an IfcClassificationReference with a code '1234' will pass right now? In my opinion, it shouldn't. If someone wants to do such a check, they can use regex pattern '12.*'.

Agreed it seems overly complex. Those classification tests are a bit confusing, but I believe what it's testing is if the IDS classification value matches a parent/ancestor of IfcClassificationReferences, it should treat it as a pass. Because all classification systems typically employ the parent name as the prefix of the child's name (i.e. Uniclass' _Pr_60_60_0833 : Gas-fired boilers shares its prefix with its parent _Pr_60_6008 : Boilers) it does feel pretty redundant. I can't think of a public classification system that doesn't do this.

The only other reason I can see for it would be that it does make it slightly simpler for the author - they don't need to apply wildcard:

E.g. "All IfcBoilers must be classified with value Pr_60_60_08 from Uniclass" - will pick up Gas-fired Boilers and Electric Boilers etc. without needing it to be a pattern. (If the ClassificationReferences are exported hierarchically of course)

atomczak commented 1 day ago

So this prevents the ability to check that IfcBoilers are exactly Pr_60_60_08 (Boilers) not Gas-fired Boilers and Electric Boilers? With pattern approach we allow both.

andyward commented 1 day ago

Yep. And it'd be simpler to implement and explain. Can't say I see many models with full IfcClassificationReference hierarchies anyway so it was always a bit hypothetical.