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

Multiple matched Attributes #279

Open
arishinsemen opened this issue Apr 22, 2024 · 6 comments
Open

Multiple matched Attributes #279

arishinsemen opened this issue Apr 22, 2024 · 6 comments

Comments

@arishinsemen
Copy link

From facet-configurations:
NAME: The Attribute should be populated (i.e. not null) (this is different from the Property facets, which accepts a null)
NAME/VALUE: all matching attributes must match the value constraints (excludes null)

Look at test pass-name_restrictions_will_match_any_result_2_3.
It passed. So, looks like for multiple attributes text above should be 'NAME: The ANY OF AttributeS should be populated (i.e. not null)'

Is it wrong test? If not, why NAME/VALUE case use 'ALL' word and different from NAME case.

@andyward
Copy link
Contributor

I think the test is fine. pass-name_restrictions_will_match_any_result_2_3.ids is essentially saying "All IfcWalls require either the Name or Description attribute to have a value".

The IDS Restrictions docs suggest enumeration restrictions should be read as an 'OR' / OneOf / AnyOf restriction. Not an 'AND' / AllOf.

But you raise an interesting question. If the Attribute Name has a restriction that satisfies multiple Attributes, and the Value is also specified/constrained on the requirement, do all the attributes have to satisfy that value requirement? The text seems to indicate All should. But there's no test I can find to assert this.

e.g. given this IFC

#1=IFCWALL('1hqIFTRjfV6AWq_bMtnZwI',$,$,'Foo','Bar',$,$,$,$);

Should this IDS pass or fail?

<specification name="For all Walls, Name OR Description value must be Bar" ifcVersion="IFC2X3 IFC4">
  <applicability maxOccurs="unbounded">
    <entity>
      <name>
        <simpleValue>IFCWALL</simpleValue>
      </name>
    </entity>
  </applicability>
  <requirements>
    <attribute>
      <name>
        <xs:restriction base="xs:string">
          <xs:enumeration value="Name" />
          <xs:enumeration value="Description" />
        </xs:restriction>
      </name>
      <value>
        <simpleValue>Bar</simpleValue>
      </value>
    </attribute>
  </requirements>
</specification>

In our implementation this would pass, but I can see some could interpret differently. But if you wanted to test 'For all Walls, Name AND Description value must be Bar' you could specify two Attribute facets

@arishinsemen
Copy link
Author

What happened when change Name restriction from enumeration to Regex "Name|Description"?
I suppose nothing - formalize the same in other words. Just change restriction kind to extract target attribute(s) from object.

@arishinsemen
Copy link
Author

arishinsemen commented Apr 22, 2024

Simple value or restriction - for me its just a way to go from IFC file to target property (attribute, classification and etc.)
IFC -> extract entity -> extract attribute -> check attribute. OR logic should be inside 'extract attribute' but in test it moved outside.

@andyward
Copy link
Contributor

Don't Restrictions just express different types of 'predicate' in a filter/where clause? i.e. in pseudocode:

simpleValue : Where X Equals(value)
pattern restriction : Where X Matches(regexPattern)
enum restriction : Where X in (enumValues)
bounds restriction : Where X between (minValue and maxValue)
length restriction : Where Length(X) between (minValue and maxValue)

They just codify part of a requirement in more sophisticated ways than a simple equality test.

But I think we're getting away from the original issue. Probably worth restating what you think the problem is? It sounds like facet-configurations.md may need some clarifications/edits?

@arishinsemen
Copy link
Author

From facet-configuration:
Attribute
NAME: The Attribute should be populated (i.e. not null)
NAME/VALUE: all matching attributes must match the value constraints (excludes null)

  1. Test fine - i think that NAME text should be changed to "The any of matched attributes ...."
  2. I try your requirement from comment in blenderbim - it failed. And i agree with that: "All matching attributes" -> we match 2 attributes -> one failed -> fail

My question:
Why logic is different? NAME - Any of matched. NAME/VALUE - All matching
No obvious logic operators - users need to remember hidden logic for some facets (classification - at least one, material - at least one, pset - all (also no info in parameters, just in tests). But different logic inside one facet what depends on parameters... Looks monstrous and hard to remember.

@andyward
Copy link
Contributor

I'd agree it's confusing to have these inconsistencies. Especially when not backed up by some reference test cases. As we're seeing it just creates ambiguity for authors and implementors. I've not been party to the design meetings but I think the changes came out of PR #240 quite recently.

In particular Facet-configuration is new and looks like work in progress and not necessarily consistent with the test cases. I'm coincidentally in a meeting with @CBenghi this afternoon so will get his view

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants