-
Notifications
You must be signed in to change notification settings - Fork 18
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-80/GRD000 - Grid Information #257
base: development
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,26 @@ | ||||||
@implementer-agreement | ||||||
@GRD | ||||||
@POS | ||||||
@version1 | ||||||
@E00020 | ||||||
|
||||||
Feature: GRD000 - Grid Information | ||||||
The rule verifies the presence of IFC entities used to define a design grid to be used as reference for object placement. | ||||||
https://ifc43-docs.standards.buildingsmart.org/IFC/RELEASE/IFC4x3/HTML/concepts/Product_Shape/Product_Placement/Product_Grid_Placement/content.html | ||||||
|
||||||
|
||||||
Scenario: Check for activation | ||||||
|
||||||
Given an IfcGridPlacement | ||||||
Given its attribute PlacesObject | ||||||
Given its entity type is 'IfcProduct' including subtypes | ||||||
Comment on lines
+15
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is governed by the schema.
But what is not governed is that there is at least one Product being placed by the Placement, which I think is a good requirement to add here as an activation criterion. |
||||||
Given return to IfcGridPlacement | ||||||
Given its attribute PlacementRelTo | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PlacementRelTo does not exist on schemas pre IFC4.3. I think this must be reflected in the scenario. |
||||||
Given its attribute PlacesObject | ||||||
Given its entity type is 'IfcGrid' | ||||||
Comment on lines
+18
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These three lines should actually not be a given statement, but a then statement.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This makes it a normative rule, and not a rule to check for activation. |
||||||
Given return to IfcGridPlacement | ||||||
Given its attribute PlacementLocation | ||||||
Given IntersectingAxes = not empty | ||||||
Comment on lines
+22
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is given by the schema:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. I was wondering how close we should stay to the relating concept template. |
||||||
|
||||||
Then The IFC model contains information on quantities of elements | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Copy pasta There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Speaking of which, maybe this is too error prone and we can rather easily replace this with:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Undercooked pasta, in this case .. It's indeed very easy to miss, and maybe not such an important part. We have the description in the feature file to provide context after all. |
||||||
|
||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So all in all, you could consider rewriting this scenario as: Given an IfcProduct
And its attribute ObjectPlacement
And its entity type is IfcGridPlacement
Then The IFC model contains information on ... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But would this be close enough to the relating concept template? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, it goes back to the age old question that we have to second guess the intention behind the template, and this case I'd say it's twofold:
And then a third component that is important, but is not in the template:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's phrase it differently, as soon as the model contains:
The file contains a "grid placement" according to the functional part. But if it's not according to the 2 further specifications I just highlighted, it's not that the model doesn't contain it, it's that the usage is invalid. So yes, these are additional normative parts and we need to create this separation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @evandroAlfieri @Ghesselink Annother rule for the backlog is this informal proposition from the docs:
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
from givens import attributes, entities, relationships, values | ||
from thens import alignment, attributes, existance, geometry, nesting, reference, relations, values | ||
from steps import attributes | ||
from steps import attributes, return_to |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
from utils import misc, ifc | ||
|
||
import ifcopenshell | ||
from validation_handling import gherkin_ifc, global_rule | ||
|
||
from behave import register_type | ||
from parse_type import TypeBuilder | ||
from enum import Enum, auto | ||
|
||
from utils.subtype_handling import SubTypeHandling, check_entity_type | ||
|
||
register_type(include_or_exclude_subtypes=TypeBuilder.make_enum({"including subtypes": SubTypeHandling.INCLUDE, "excluding subtypes": SubTypeHandling.EXCLUDE })) | ||
|
||
@global_rule | ||
@gherkin_ifc.step("Return to {entity}") | ||
@gherkin_ifc.step("Return to {entity} {include_or_exclude_subtypes:include_or_exclude_subtypes}") | ||
def step_impl(context, entity, include_or_exclude_subtypes=SubTypeHandling.EXCLUDE): | ||
""" | ||
Disclaimer: Currently untested for normative rules | ||
|
||
| Feature Step | Context Stack | | ||
|----------------------------|-------------------------------------| | ||
| Given an IfcEntity | [entity1, entity2, entity3] | | ||
| Given its attribute X | [attr, None, attr] | | ||
| Given Y is attr | [True, False, True] | | ||
| Given return to IfcEntity | [entity1, entity2, entity3] | | ||
For rules used for activation, this is not a problem (one is sufficient to activate the rule). | ||
However, for normative rules, we do not want to consider entity2. | ||
Simply using indexes would not work as the stack is often nested. | ||
""" | ||
context.include_or_exclude_subtypes = include_or_exclude_subtypes | ||
def filter_stack_tree(layer): | ||
def check_inclusion_criteria(input): | ||
""" | ||
Verifies if layer includes a boolean variable or instance of {entity} | ||
""" | ||
is_bool = isinstance(input, bool) | ||
correct_entity = False | ||
if isinstance(input, ifcopenshell.entity_instance): | ||
correct_entity = check_entity_type(input, entity, context.include_or_exclude_subtypes) | ||
context.include_layer = is_bool or correct_entity | ||
layer = layer.get('instances') | ||
misc.map_state(layer, check_inclusion_criteria) | ||
return layer if context.include_layer else None | ||
|
||
#ensure the stack does not get pupulated when nothing was yielded in the last step | ||
if (lambda f: f(f))(lambda f: lambda data: bool(data) and (not isinstance(data, (list, tuple)) or any(f(f)(item) for item in data)))(context.instances): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again this is rocket science to me. Can we clear this up first in the other PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's bring the rocket science back to building information science then 🚀
In case we don't check whether context.instances is meaningful, we'd return the context.instances as was the case after the initial Given step. This is not the preferred behavior, since the preoccuring Given statements determined that the rule would not be applicable. The |
||
stack_tree_filtered = list( | ||
filter(None, list(map(filter_stack_tree, context._stack)))) | ||
context.instances = stack_tree_filtered |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
import ifcopenshell | ||
|
||
from enum import Enum, auto | ||
|
||
class SubTypeHandling (Enum): | ||
INCLUDE = auto() | ||
EXCLUDE = auto() | ||
|
||
def check_entity_type(inst: ifcopenshell.entity_instance, entity_type: str, handling: SubTypeHandling) -> bool: | ||
""" | ||
Check if the instance is of a specific entity type or its subtype. | ||
INCLUDE will evaluate to True if inst is a subtype of entity_type while the second function for EXCLUDE will evaluate to True only for an exact type match | ||
|
||
Parameters: | ||
inst (ifcopenshell.entity_instance): The instance to check. | ||
entity_type (str): The entity type to check against. | ||
handling (SubTypeHandling): Determines whether to include subtypes or not. | ||
|
||
Returns: | ||
bool: True if the instance matches the entity type criteria, False otherwise. | ||
""" | ||
handling_functions = { | ||
SubTypeHandling.INCLUDE: lambda inst, entity_type: inst.is_a(entity_type), | ||
SubTypeHandling.EXCLUDE: lambda inst, entity_type: inst.is_a() == entity_type, | ||
} | ||
return handling_functions[handling](inst, entity_type) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
import ifcopenshell | ||
import ifcopenshell.template | ||
import os | ||
|
||
rule_code = "grd000" | ||
|
||
abs_path = os.path.join(os.getcwd(), "test", "files", rule_code) | ||
save_ifc_file = lambda file, filename: file.write(os.path.join(abs_path, filename)) | ||
|
||
f = ifcopenshell.template.create(schema_identifier="IFC4X3_ADD2") | ||
|
||
placement = f.createIfcLocalPlacement() | ||
O = 0., 0., 0. | ||
X = 1., 0., 0. | ||
Y = 0., 1., 0. | ||
Z = 0., 0., 1. | ||
grid = f.createIfcGrid( | ||
GlobalId = ifcopenshell.guid.new(), | ||
ObjectPlacement = placement, | ||
UAxes = [f.createIfcGridAxis( | ||
AxisTag = "1", | ||
AxisCurve = f.createIfcPolyLine([f.createIfcCartesianPoint(O), f.createIfcCartesianPoint(X)]), | ||
SameSense = True | ||
)], | ||
VAxes = [f.createIfcGridAxis( | ||
AxisTag = "2", | ||
AxisCurve = f.createIfcPolyLine([f.createIfcCartesianPoint(O), f.createIfcCartesianPoint(Y)]), | ||
SameSense = True | ||
)], | ||
WAxes = [f.createIfcGridAxis( | ||
AxisTag = "3", | ||
AxisCurve = f.createIfcPolyLine([f.createIfcCartesianPoint(O), f.createIfcCartesianPoint(Z)]), | ||
SameSense = True | ||
)] | ||
) | ||
|
||
save_ifc_file(f, f'pass-{rule_code}-not_activated_no_placement.ifc') | ||
|
||
|
||
grid_axis_1 = f.createIfcGridAxis( | ||
AxisTag = "Axis 1", | ||
AxisCurve = f.createIfcPolyLine([f.createIfcCartesianPoint(O), f.createIfcCartesianPoint(X)]), | ||
SameSense = True | ||
) | ||
|
||
grid_axis_2 = f.createIfcGridAxis( | ||
AxisTag = "Axis 2", | ||
AxisCurve = f.createIfcPolyLine([f.createIfcCartesianPoint(O), f.createIfcCartesianPoint(Y)]), | ||
SameSense = True | ||
) | ||
|
||
grid_placement = f.createIfcGridPlacement( | ||
PlacementRelTo = placement, | ||
) | ||
|
||
column = f.createIfcColumn( | ||
GlobalId = ifcopenshell.guid.new(), | ||
ObjectPlacement = grid_placement, | ||
) | ||
|
||
grid.ObjectPlacement = placement | ||
|
||
grid_placement.PlacementLocation = f.createIfcVirtualGridIntersection( | ||
[grid_axis_1, grid_axis_2], | ||
(0.0, 0.0, 0.0) | ||
) | ||
Comment on lines
+63
to
+66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This violates the informal propositions here: https://ifc43-docs.standards.buildingsmart.org/IFC/RELEASE/IFC4x3/HTML/lexical/IfcVirtualGridIntersection.htm It's also a really creative grid. I would define:
And then you intersect e.g UAxes[1] with VAxes[1], but they need to be instances from the grid itself. Also you don't actually reference this variable, but redefined sth on L 777 Also the polyline grid axes are best 2d instead of 3d. |
||
|
||
save_ifc_file(f, f'pass-{rule_code}-activated_valid_grid_placement.ifc') | ||
|
||
f = ifcopenshell.template.create(schema_identifier="IFC4X3_ADD2") | ||
|
||
column = f.createIfcColumn( | ||
GlobalId = ifcopenshell.guid.new(), | ||
ObjectPlacement = f.createIfcGridPlacement( | ||
PlacementLocation = f.createIfcVirtualGridIntersection( | ||
[ | ||
f.createIfcGridAxis( | ||
AxisTag = "Axis 1", | ||
AxisCurve = f.createIfcPolyLine([f.createIfcCartesianPoint(O), f.createIfcCartesianPoint(X)]), | ||
SameSense = True | ||
), | ||
f.createIfcGridAxis( | ||
AxisTag = "Axis 2", | ||
AxisCurve = f.createIfcPolyLine([f.createIfcCartesianPoint(O), f.createIfcCartesianPoint(Y)]), | ||
SameSense = True | ||
) | ||
] | ||
), | ||
), | ||
) | ||
|
||
save_ifc_file(f, f'pass-{rule_code}-not_activated_no_grid.ifc') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@evandroAlfieri Why did we pick "Information" instead of "Placement"? Grid Information to me hints at information on grids in the model, not other elements placed according to grid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I thought moving the pr back to draft was enough to save you from reviewing it. It's about this rule that we (Geert and I) wanted to talk with you on Monday