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

IVS-79/LAY000 - Presentation Layer Assignment #256

Closed
wants to merge 3 commits into from

Conversation

Ghesselink
Copy link
Contributor

A couple of remarks;

  • The schema does not allow for any other entity type than IfcPresentationLayerAssignment for the attribute LayerAssignments (of IfcShapeRepresentation). It seems it's not possible to assign any other entity than mentioned above due to the assigning taking place by the AssignedItems (of IfcPresLayerAssign) attribute. Therefore, I think it would be redundant to add this explicitly in a gherkin steps; solely Its attribute LayerAssignments suffices.
    image
    image

  • The not-activated file has the following list in context.instances that is getting handled by the then step.
    image

  • The activated file has the following instances in the stack for the last then step. Index 5 and 7 (both IfcPresentationLayerAssignment) activate the rule.
    image

Copy link
Collaborator

@evandroAlfieri evandroAlfieri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work

@Ghesselink Ghesselink added the rule tracker for development of a new rule/warning label Aug 7, 2024
Copy link
Collaborator

@aothms aothms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must say I find this functional part not something I would advise end users to make decisions on.

Given its attribute Representations
Given its attribute LayerAssignments

Then The IFC model contains information on geometry layers
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

geometry layers != layers information

Shouldn't this be consistent?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed it to
The IFC model contains information on the selected functional part
See #257 (comment)

@version1
@E00020

Feature: LAY000 - Layers Information
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better english would be layer, but I would also really add the word Presentation, so that it's not to be confused with the physical material layers for compound structures such as walls and slab (IfcMaterialLayerSet). Lastly since again it doesn't really contain information about these layers, I would call it assignment. And then accidentally I have arrived at the verbatim entity name.

Suggested change
Feature: LAY000 - Layers Information
Feature: LAY000 - Presentation Layer Assignment

Copy link
Contributor Author

@Ghesselink Ghesselink Aug 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A single name for the entity and the functional part, that sounds consistent
Modified it
We'll also have to modify the feature filename in that case.


Given an IfcProduct
Given its attribute Representation
Given its attribute Representations
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is one additional consideration, that's a bit in detail, but actually you can have layer assignment on the representation but also on the items. This is missing in the concept template, but is rendered in the documentation.

ifcpresentationlayerassignment-fig1

I would actually add this, because semantically it makes more sense for me to assign the representation item than to assign the representation itself.

    Scenario: Layer assignment to representation

    Given an IfcProduct
    Given its attribute Representation
    Given its attribute Representations
    Given its attribute LayerAssignments

    Scenario: Layer assignment to representation items

    Given an IfcProduct
    Given its attribute Representation
    Given its attribute Representations
    Given its attribute Items
    Given its attribute LayerAssignments

Given an IfcProduct
Given its attribute Representation
Given its attribute Representations
Given its attribute LayerAssignments
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the check that this is not empty in the The IFC model contains information on implementation? That's a bit implicit.

Should there not be something like:

Given its attribute LayerAssignments is not empty

or separated into two steps?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two statements are similar and, for activation, they function the same. The difference lies in which entity is selected for the next step. For these 000 rules, we don't evaluate the instances but simply check if they exist in the model. If the LayerAssignments attribute in an IFC file isn't empty, the rule will activate regardless of which statement we use.

Currently, the last instances in the stack (if any) are of type IfcPresentationLayerAssignment. With the 'is not empty' statement, they will be of type IfcShapeRepresentation (filtered out by the current statement from the previous Given statement).

@Ghesselink Ghesselink changed the title IVS-79/LAY000 - Layers Information IVS-79/LAY000 - Presentation Layer Assignment Aug 10, 2024
@Ghesselink Ghesselink mentioned this pull request Sep 15, 2024
@Ghesselink
Copy link
Contributor Author

Outdated #281

@Ghesselink Ghesselink closed this Sep 15, 2024
@Ghesselink Ghesselink deleted the IVS-79-LAY000-Layers_information branch September 15, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule tracker for development of a new rule/warning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants