-
Notifications
You must be signed in to change notification settings - Fork 21
IVS-236 - GRF005 - CRS unit type differences #411
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
base: development
Are you sure you want to change the base?
Changes from all commits
451fa53
f837ba6
c706d07
8a57e2a
e2b0550
2685e0f
e09ee39
7ba42ad
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 @@ | ||
| @industry-practice | ||
| @GRF | ||
| @version1 | ||
| @E00100 | ||
| Feature: GRF005 - CRS unit type differences | ||
| The rule verifies that the Scale attribute of IfcMapConversion is used when the units of the CRS are not identical to the units of the engineering coordinate system. | ||
| If omitted, the value of 1.0 is assumed. | ||
| If the units of the referenced source location engineering coordinate system are different from the units of the referenced target coordinate system, | ||
| then this attribute must be included and must have the value of the scale from the source to the target units | ||
|
|
||
|
|
||
| Scenario Outline: When the length unit of the Local CRS (from IfcProject) is not equal to the length unit of the Projected CRS, then the IfcMapCOnversion.Scale must be provided and cannot be 1.0 | ||
|
|
||
| Given A model with Schema 'IFC4.3' | ||
| Given An .IfcMapConversion. | ||
| Given There must be at least 1 instance(s) of .<IfcCoordinateReferenceSystem>. | ||
|
Collaborator
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. Not a bit fan of "Given There must be ..." can we rephrase this?
Collaborator
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. IfcMapConversion.TargetCRS = (IfcCoordinateReferenceSystem) is also a mandatory attribute so I don't see how this could be different (or at least.. it's already handled by the schema check). |
||
| Given The <unit> unit of the project ^is not^ equal to the <unit> unit(s) of the .<IfcCoordinateReferenceSystem>. | ||
|
|
||
| Then .Scale. ^is not^ empty | ||
| Then .Scale. ^is not^ 1.0 | ||
|
Comment on lines
+19
to
+20
Collaborator
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. Rather than requiring not empty and not 1.0, can we require the actual value that is needed? I.e the one conversion factor divide by the other. |
||
|
|
||
| Examples: | ||
| | unit | IfcCoordinateReferenceSystem | | ||
| | length | IfcProjectedCRS | | ||
| | angle | IfcGeographicCRS | | ||
|
Comment on lines
+24
to
+25
Collaborator
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. If I look at the mapconv docs I don't see any reference to scaling angular units and I don't think this actually is a thing.
https://ifc43-docs.standards.buildingsmart.org/IFC/RELEASE/IFC4x3/HTML/lexical/IfcMapConversion.htm So I think the rule only has to check length |
||
|
|
||
civilx64 marked this conversation as resolved.
Show resolved
Hide resolved
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| from pyproj.database import query_crs_info | ||
| from pyproj import CRS | ||
| from validation_handling import gherkin_ifc | ||
| from utils import misc | ||
| import operator | ||
|
|
||
| from . import ValidationOutcome, OutcomeSeverity | ||
|
|
||
|
|
@@ -22,4 +24,85 @@ def step_impl(context, inst): | |
| yield ValidationOutcome(instance_id=inst, severity=OutcomeSeverity.PASSED) | ||
| else: | ||
| yield ValidationOutcome(inst=inst, severity=OutcomeSeverity.ERROR) | ||
|
|
||
|
|
||
|
|
||
| @gherkin_ifc.step("The {unit_types} unit(s) of the project ^{comparison_operator:equal_or_not_equal}^ equal to the {crs_unit_types} unit(s) of the .{crs_entity_type}.") | ||
| def step_impl(context, inst, unit_types, comparison_operator, crs_unit_types, crs_entity_type): | ||
| assert unit_types == crs_unit_types, "it's only possible to compare equal unit types" | ||
| pred = misc.negate(operator.eq) if comparison_operator in {"is not", "!="} else operator.eq | ||
| unit_types = unit_types.split(' and ') | ||
|
|
||
| conversion_based = False | ||
|
|
||
| unit_type_attr_map= { | ||
| 'length' : 'LENGTHUNIT', | ||
| 'area': 'AREAUNIT', | ||
| 'volume': 'VOLUMEUNIT', | ||
| 'angle': 'PLANEANGLEUNIT', | ||
| } | ||
|
|
||
| def map_units(units): | ||
| map = {} | ||
| for unit in units: | ||
| if unit.is_a('IfcNamedUnit') | ||
| unit_type = unit.UnitType | ||
| if unit_type and unit_type not in map: | ||
| map[unit_type] = unit | ||
| return map | ||
|
|
||
| # determine the project unit values | ||
| project = context.model.by_type("IfcProject")[0] | ||
|
Collaborator
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.
|
||
| project_unit_map = map_units(getattr(project.UnitsInContext, 'Units', [])) | ||
| project_units = {utype : project_unit_map.get(unit_type_attr_map[utype]) for utype in unit_types} | ||
|
|
||
|
|
||
| if 'length' in unit_types and (length := project_units.get('length')): | ||
| prefix = getattr(length, 'Prefix', '') | ||
| name = getattr(length, 'Name', '').lower() | ||
| project_units['length'] = (f"{prefix}{name}" if prefix else name).lower() | ||
|
|
||
| if 'angle' in unit_types and (angle := project_units.get('angle')): | ||
| if length.is_a('IfcSIUnit'): | ||
| project_units['angle'] = { | ||
| 'name': (f"{prefix}{name}" if prefix else name).lower(), | ||
| 'conversion_factor' : None | ||
| } | ||
| conversion_based=True | ||
| else: | ||
| project_units['angle'] = { | ||
| 'name': getattr(angle, 'Name', '').lower(), | ||
| 'conversion_factor': misc.do_try(lambda: angle.ConversionFactor.ValueComponent.wrappedValue, '') | ||
| } | ||
|
|
||
| # determine the crs unit values | ||
| crs = context.model.by_type(crs_entity_type)[0] | ||
| epsg_crs = CRS.from_string(crs.Name) | ||
| unit_attrs = [attr for attr in dir(crs) if attr.endswith('Unit')] | ||
| crs_unit_map = map_units([getattr(crs, attr) for attr in unit_attrs if getattr(crs, attr, None) is not None]) | ||
| crs_units = {utype: crs_unit_map.get(unit_type_attr_map[utype]) for utype in unit_types} | ||
|
|
||
| if 'length' in unit_types: | ||
| if length := crs_units.get('length'): | ||
| prefix = getattr(length, 'Prefix', '') | ||
| name = getattr(length, 'Name', '').lower() | ||
| crs_units['length'] = (f"{prefix}{name}" if prefix else name).lower() | ||
|
Collaborator
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. Instead of focussing on names not being equal, i'd rather focus on the conversion factors Divide that with the unit scale reported by proj and that's the value of Scale you expect. |
||
| else: | ||
| crs_units['length'] = epsg_crs.coordinate_system.axis_list[0].unit_name | ||
|
|
||
| if 'angle' in unit_types: | ||
| if angle := crs_units.get('angle'): | ||
| crs_units['angle'] = { | ||
| 'name': getattr(angle, 'Name').lower(), | ||
| 'conversion_factor': misc.do_try(lambda : project_units['angle'].ConversionFactor.ValueComponent.wrappedValue, '') | ||
| } | ||
| else: | ||
| crs_units['angle'] = { | ||
| 'name' : epsg_crs.coordinate_system.axis_list[0].unit_name, | ||
| 'conversion_factor': misc.do_try(lambda : epsg_crs.coordinate_system.axis_list[0].unit_conversion_factor, None) if conversion_based else None | ||
| } | ||
|
|
||
|
|
||
| if pred(project_units, crs_units): | ||
| yield ValidationOutcome(instance_id=inst, severity=OutcomeSeverity.PASSED) | ||
| else: | ||
| yield ValidationOutcome(inst=inst, severity=OutcomeSeverity.ERROR) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| ISO-10303-21; | ||
| /* IFC units are in metres, ESPG:2277 uses survey foot and the map conversion scaled is not set */ | ||
| HEADER; | ||
| FILE_DESCRIPTION(('ViewDefinition [ReferenceView]'),'2;1'); | ||
| FILE_NAME('fail-grf005-project_meters_crs_foot_no_map_conversion_scaled.ifc','2023-01-25T18:40:40',(''),(''),'','IfcOpenShell contributors - IfcOpenShell - v0.7.0+6180d73f',''); | ||
| FILE_SCHEMA(('IFC4X3_ADD2')); | ||
| ENDSEC; | ||
| DATA; | ||
| #1=IFCPERSON($,$,'',$,$,$,$,$); | ||
| #2=IFCORGANIZATION($,'',$,$,$); | ||
| #3=IFCPERSONANDORGANIZATION(#1,#2,$); | ||
| #4=IFCAPPLICATION(#2,'v0.7.0-6180d73f','IfcOpenShell-v0.7.0-6180d73f',''); | ||
| #5=IFCOWNERHISTORY(#3,#4,$,.NOCHANGE.,$,#3,#4,1674672040); | ||
| #6=IFCDIRECTION((1.,0.,0.)); | ||
| #7=IFCDIRECTION((0.,0.,1.)); | ||
| #8=IFCCARTESIANPOINT((0.,0.,0.)); | ||
| #9=IFCAXIS2PLACEMENT3D(#8,#7,#6); | ||
| #10=IFCDIRECTION((0.,1.)); | ||
| #11=IFCGEOMETRICREPRESENTATIONCONTEXT($,'Model',3,1.E-05,#9,#10); | ||
| #12=IFCDIMENSIONALEXPONENTS(0,0,0,0,0,0,0); | ||
| #13=IFCSIUNIT(*,.LENGTHUNIT.,$,.METRE.); | ||
| #14=IFCSIUNIT(*,.AREAUNIT.,$,.SQUARE_FOOT.); | ||
| #15=IFCSIUNIT(*,.VOLUMEUNIT.,$,.CUBIC_FOOT.); | ||
| #16=IFCSIUNIT(*,.PLANEANGLEUNIT.,$,.RADIAN.); | ||
| #17=IFCMEASUREWITHUNIT(IFCPLANEANGLEMEASURE(0.017453292519943295),#16); | ||
| #18=IFCCONVERSIONBASEDUNIT(#12,.PLANEANGLEUNIT.,'DEGREE',#17); | ||
| #19=IFCUNITASSIGNMENT((#13,#14,#15,#18)); | ||
| #20=IFCPROJECT('2X9wjf5oPB4PQjenCYrhHZ',#5,'',$,$,$,$,(#11),#19) | ||
| #21=IFCPROJECTEDCRS('EPSG:2277',$,'NAD83',$,'','3',$); | ||
| #22=IFCMAPCONVERSION(#11,#21,316131.64,5690966.11,1.,1.,0.,$); | ||
| #23=IFCGEOMETRICREPRESENTATIONCONTEXT($,'Model',3,1.E-05,#9,#10); | ||
| #24=IFCMAPCONVERSION(#23,#21,316131.64,5690966.11,1.,1.,0.,$); | ||
| ENDSEC; | ||
| END-ISO-10303-21; |
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.
So, MapConversion is a bit strange in the sense the SourceSRC is a SELECT between (CoordinateRef, RepresentationContext).
I think this allows us to workaround the by_type(IfcProject)[0], because we can do MapConv --Source--> RepContext -> Project/Context -> Unit.
So I think we should also condition this check to only execute when MapConv.Source is the RepContext (I'm not aware of any scenario in which this is not the case, but still).