buildingSMART / buildingSMART/IDS

review of a number of testcases

Open
#121 1 comment 0 reactions 1 assignee Claimed by @MarcelStepien View on GitHub
tests
Dominant language
C#
Stars
318
Forks
90
PR merge metrics
No merged PRs in 30d

Description

Hello all,

I have taken the time to test a majority of the testcases provided in the documentation folder against a running implementation of an IDS engine.

Specifically the following facets has been tested:
- Attribute
- Entity
- Property

While most tests worked fine, some of the listed cases require adjustments. Here, I want to showcase some findings and provide feedback to improve the provided cases.


1. (Entity) fail - a predefined type must always specify a meaningful type not userdefined itself

In this case, the check of `predefinedValue -> USERDEFINED` is expected to result in a failed test. However, this should result in a successful validation. A valid use case would be _"generically find all IFC entities that have a custom defined type associated with them"_. Other test cases also consider some sort of priority to be checked for predefinedTypes:

- First, the `predefinedType` is always checked.
- Secondly, the `objectType`, `processType `or `elementType` is checked.
- And finally, if none of the previous apply, the relation of `IfcRelDefinedByType` is checked.

At this point, a case can be made for simply renaming "predefinedType" to "type" in the IDS specification.


2. (Entity) fail - entities must be specified as uppercase strings

This case validates the IFC entity class name by checking the equivalence between class name and defined requirement. In this case, I would recommend excluding entity class name validation from case sensitivity checks. The reason for this is that case sensitivity is a developer-specific dependency. Camel case or upper case notation can both be used in implementations and should not be enforced for the entity class name by the schema or test cases. So, this case should result in a successful validation.

But keep in mind, I would only recomend removing case sensitivity for class dependent requirements (such as `IfcWall`, `IfcBoolean `or `IfcRelDefinedByType`).

This relates also to:
- (Property) fail - booleans must be specified as uppercase strings 3 3


3. (Attribute) fail - attributes with empty strings always fail

The check evaluates an empty string as an failure. This should not be the case. There are several use cases in a BIM data exchange scenario where empty srings are a requirement for `IfcValue` entries. To be precise, if the check is to validate that something other than an empty string is provided, a pattern constraint should be used here.

This relates also to:
- (Property) fail - an empty string is considered falsey and will not pass


4. (Attribute) fail - booleans must be specified as uppercase strings 2 3

In this case, the validation considers a difference between `False` and `.F.`. However, considering that an IDS engine operates with an loaded and object-oriented IFC model, the boolean values are wrapped as IfcBoolean-types. There are several ways to represent a `false `boolean value, such as `False`, `false`, `FALSE`, `F`, `f`, or `0`. These are logically different representations of the same value and therefore should result in successful validation.

Considering the possible definition of `measure` in IDS, the type IfcValue must also be checked. If no `measure` is specified, the `simpleValue` check is usually for a `String`. Here, this `String` should be forcibly converted to the same type as the `IfcValue` it is check against in the model, to ensure that no type discrepancy occurs.

This relates also to:
- (Property) pass - integer values checked using type casting


5. (Property) pass - floating point numbers are compared with a 1e6 tolerance

All checks of this type do not work logically. As defined, they should fail rather then pass, because `42 != 42.00042` and `41.999958 != 42.00042`. I would raise the question, does it make sence to try and restrict the floating point accurency to `1e6` tolerance? The limit should be variable and rather dependent on the attached unit or the IfcValue type.


6. (Property) pass - non ascii characters are treated without encoding

There is a spelling issue in the IFC model file:
`♫Don'tÄrgerhôtelЊет (IDS) != ♫Don''tÄrgerhôtelЊет (IFC)`

There is a quotation mark to much included.

This relates also to:
- (Attribute) pass - non ascii characters are treated without encoding

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.