buildingSMART / IDS

Computer interpretable (XML) standard to define Information Delivery Specifications for BIM (mainly used for IFC)
Other
167 stars 52 forks source link

Facets review #240

Closed CBenghi closed 2 months ago

CBenghi commented 3 months ago

Working progress revision of the schema and documentation.

CBenghi commented 3 months ago

I haven't looked through the documentation changes, since the text seems very WIP today.

Indeed that is more of a set of working notes that will need to be distributed in the rest of the documentation. PRs are welcome for that.

NickNisbet commented 3 months ago

I need to require that an IfcElement MUST have an IfcElementType.

Regards

andyward commented 3 months ago

I need to require that an IfcElement MUST have an IfcElementType.

  • Is requiring an attribute ‘PredefinedType’ with pattern restriction '.*' the way to do this? (preferred)
  • Is requiring an entity ‘subType’ with pattern restriction ‘.*’ the way to do this?

Regards, Nick.

Nick, I'm not sure either would work reliably - any validator would presumably only flag up issues where there was a PredefinedType attribute value on the ElementType but not on the Element. So as a requirement it might work for my IfcFlowTerminal=>IsTypedBy=>IfcSanitaryTerminalType example above (any value must come from the Type). And it would work for say IfcBeam and IfcBeamType in IFC2x3 but not in later schemas since IfcBeam.PredefinedType got introduced in Ifc4. And wouldn't work in any schema for IfcPile since PredefinedType existed on both Element and Type since IFC2x3.

Probably needs a separate issue if there's not one already...

Andy

MatthiasWeise commented 3 months ago

I need to require that an IfcElement MUST have an IfcElementType.

  • Is requiring an attribute ‘PredefinedType’ with pattern restriction '.*' the way to do this? (preferred)
  • Is requiring an entity ‘subType’ with pattern restriction ‘.*’ the way to do this?

Regards, Nick.

Nick, I'm not sure either would work reliably - any validator would presumably only flag up issues where there was a PredefinedType attribute value on the ElementType but not on the Element. So as a requirement it might work for my IfcFlowTerminal=>IsTypedBy=>IfcSanitaryTerminalType example above (any value must come from the Type). And it would work for say IfcBeam and IfcBeamType in IFC2x3 but not in later schemas since IfcBeam.PredefinedType got introduced in Ifc4. And wouldn't work in any schema for IfcPile since PredefinedType existed on both Element and Type since IFC2x3.

Probably needs a separate issue if there's not one already...

Andy

May check this issue (related to IFC2x3 types): https://github.com/buildingSMART/IDS/issues/116

gverduci commented 3 months ago

Hi Claudio, the extension of the attributeType contains the attribute group "xs:occurs" and the new attribute cardinality (line 156). Maybe it is a mistake

gverduci commented 2 months ago

The entity name in the restriction test case "fail-an_enumeration_matches_case_sensitively_1_3.ids" is in pascal case format

<applicability minOccurs="0" maxOccurs="unbounded">
  <entity>
    <name>
      <simpleValue>IfcWall</simpleValue>
    </name>
 </entity>
</applicability>
gverduci commented 2 months ago

Classification test cases:

The related ifc files contain two IfcWalls but only one classified: the ids files fail due to the presence of an unclassified wall

CBenghi commented 2 months ago

Hi Claudio, the extension of the attributeType contains the attribute group "xs:occurs" and the new attribute cardinality (line 156). Maybe it is a mistake

Hi @gverduci, thanks. I think I spotted that too while revising the schema. It should be solved now.

gverduci commented 2 months ago

Hi Claudio, the file "classification/fail-an_optional_classification_value_fails_if_no_match.ids" contains a title and a specification name with a different meaning than the file name.

This file contains an optional requirement with system and value, and the related ifc file contains an IFCWALL without classifications.

For the optional case with system and value, the facet-configuration.md file says: "OPTIONAL SYSTEM/VALUE: entity -> if there is a classification system for the entity to be tested, its value must match"

Is it correct that this test fails?

gverduci commented 2 months ago

the file "material/fail-an_optional_material_fails_if_no_value_matches.ids" contains a title and a specification name with a different meaning than the file name

CBenghi commented 2 months ago

Hi @gverduci,

with regards to your comments:

classification/fail-an_optional_classification_value_fails_if_no_match.ids Is it correct that this test fails?

I think the origina IFC had a problem, it attached the classification to the project rather than the wall, it should now be ok. It should fail, because the classification is an empty string and should not match the expected value.

the file "material/fail-an_optional_material_fails_if_no_value_matches.ids" contains a title and a specification name with a different meaning than the file name

I've changed the code so that we are notified of the title mismatches on testcase regeneration, see 9d37660.

Thanks for spotting these.

andyward commented 2 months ago

Hi @CBenghi, when regenerating the testcases (which I assume closes out #192 ?) were you able to fix the invalid IFC for pass-occurrences_override_the_type_classification_per_system_1_3.ids

There's a whole bunch of open issues for this one bad test as every implementor seems to hit it: e.g. #108, #194, #225

There's a dead simple fix: change the type of one of the entities so it doesn't get affected by prior test data.

https://github.com/buildingSMART/IDS/pull/226#issuecomment-1850018702

CBenghi commented 2 months ago

Hi @andyward, I think I fixed the issue with that IFC file, but I was not following the relevant issues, so I hope I did it right. I have not implemented the updated verification logic for a while, so your help is much welcome if you have a list of open testcase issues. Also, now that IDS generation is automated I would like to do the same for IFC, your advice much welcome there too, if you have any. I'm thinking to produce a number of IFC files that can match the requirements (e.g. combinatorial variation of relevant schemas and element solutions). Think only of the many options that we have when it comes to materials!

andyward commented 2 months ago

Hi @andyward, I think I fixed the issue with that IFC file, but I was not following the relevant issues, so I hope I did it right. I have not implemented the updated verification logic for a while, so your help is much welcome if you have a list of open testcase issues. Also, now that IDS generation is automated I would like to do the same for IFC, your advice much welcome there too, if you have any. I'm thinking to produce a number of IFC files that can match the requirements (e.g. combinatorial variation of relevant schemas and element solutions). Think only of the many options that we have when it comes to materials!

Sure, I'll see if we can integrate the tests from this PR. Last I checked we pass all the current testcases except 1-2 or esoteric edge cases (ie6 tolerance and Predefined Properties are the only 2 I'm aware of) - so suggest we get it all (regression) tested on 0.9.7, and then can look at additional tests. Is there an XIDS update with the latest IDS schema?

Should be able to help with the IFC generation. That said I think Dion's testcases cover most of the material cases already. Multi-schema would be good to test. I also think Applicability needs some test cases (Currently the tests focus on Requirement). There's a whole bunch of scenarios with restrictions on applicability where it would be good to have a standard set of test cases for.