buildingSMART / IFC4.3.x-development

Repository to collect updates to the IFC4.3 Specification
Other
158 stars 81 forks source link

Settle georeferencing #533

Open aothms opened 1 year ago

aothms commented 1 year ago

There has been a meeting (myself excluded) about some adjustments georeferencing. The changes (with some light modifications) discussed are:

afbeelding

--- IFC.exp 2022-09-13 14:05:16.062397000 +0200
+++ IFC.exp 2022-09-30 21:11:54.409712800 +0200
@@ -435,20 +435,23 @@

 TYPE IfcVolumetricFlowRateMeasure = REAL;
 END_TYPE;

 TYPE IfcWarpingConstantMeasure = REAL;
 END_TYPE;

 TYPE IfcWarpingMomentMeasure = REAL;
 END_TYPE;

+TYPE IfcWellKnownTextLiteral = STRING;
+END_TYPE;
+
 TYPE IfcActionRequestTypeEnum = ENUMERATION OF
    (EMAIL
    ,FAX
    ,PHONE
    ,POST
    ,VERBAL
    ,USERDEFINED
    ,NOTDEFINED);
 END_TYPE;

@@ -5398,34 +5401,37 @@
 ENTITY IfcCoolingTowerType
  SUBTYPE OF (IfcEnergyConversionDeviceType);
    PredefinedType : IfcCoolingTowerTypeEnum;
  WHERE
    CorrectPredefinedType : (PredefinedType <> IfcCoolingTowerTypeEnum.USERDEFINED) OR
  ((PredefinedType = IfcCoolingTowerTypeEnum.USERDEFINED) AND EXISTS (SELF\IfcElementType.ElementType));
 END_ENTITY;

 ENTITY IfcCoordinateOperation
  ABSTRACT SUPERTYPE OF (ONEOF
-   (IfcMapConversion));
+   (IfcMapConversion
+   ,IfcRigidOperation));
    SourceCRS : IfcCoordinateReferenceSystemSelect;
    TargetCRS : IfcCoordinateReferenceSystem;
 END_ENTITY;

 ENTITY IfcCoordinateReferenceSystem
  ABSTRACT SUPERTYPE OF (ONEOF
-   (IfcProjectedCRS));
-   Name : IfcLabel;
+   (IfcGeographicCRS
+   ,IfcProjectedCRS));
+   Name : OPTIONAL IfcLabel;
    Description : OPTIONAL IfcText;
-   GeodeticDatum : OPTIONAL IfcIdentifier;
-   VerticalDatum : OPTIONAL IfcIdentifier;
  INVERSE
    HasCoordinateOperation : SET [0:1] OF IfcCoordinateOperation FOR SourceCRS;
+   WellKnownText : SET [0:1] OF IfcWellKnownText FOR CoordinateReferenceSystem;
+ WHERE
+   NameOrWKT : (HIINDEX(WellKnownText) = 1) OR EXISTS(Name);
 END_ENTITY;

 ENTITY IfcCosineSpiral
  SUBTYPE OF (IfcSpiral);
    CosineTerm : IfcLengthMeasure;
    ConstantTerm : OPTIONAL IfcLengthMeasure;
 END_ENTITY;

 ENTITY IfcCostItem
  SUBTYPE OF (IfcControl);
@@ -7072,20 +7078,29 @@
 ENTITY IfcFurnitureType
  SUBTYPE OF (IfcFurnishingElementType);
    AssemblyPlace : IfcAssemblyPlaceEnum;
    PredefinedType : OPTIONAL IfcFurnitureTypeEnum;
  WHERE
    CorrectPredefinedType : NOT(EXISTS(PredefinedType)) OR
  (PredefinedType <> IfcFurnitureTypeEnum.USERDEFINED) OR
  ((PredefinedType = IfcFurnitureTypeEnum.USERDEFINED) AND EXISTS (SELF\IfcElementType.ElementType));
 END_ENTITY;

+ENTITY IfcGeographicCRS
+ SUBTYPE OF (IfcCoordinateReferenceSystem);
+   GeodeticDatum : OPTIONAL IfcIdentifier;
+   PrimeMeridian : OPTIONAL IfcIdentifier;
+   Unit : OPTIONAL IfcNamedUnit;
+ WHERE
+   IsPlaneAngleUnit : NOT(EXISTS(Unit)) OR (Unit.UnitType=IfcUnitEnum.PLANEANGLEUNIT);
+END_ENTITY;
+
 ENTITY IfcGeographicElement
  SUBTYPE OF (IfcElement);
    PredefinedType : OPTIONAL IfcGeographicElementTypeEnum;
  WHERE
    CorrectPredefinedType : NOT(EXISTS(PredefinedType)) OR
  (PredefinedType <> IfcGeographicElementTypeEnum.USERDEFINED) OR
  ((PredefinedType = IfcGeographicElementTypeEnum.USERDEFINED) AND EXISTS (SELF\IfcObject.ObjectType));
    CorrectTypeAssigned : (SIZEOF(IsTypedBy) = 0) OR
   ('IFC4X3_DEV.IFCGEOGRAPHICELEMENTTYPE' IN TYPEOF(SELF\IfcObject.IsTypedBy[1].RelatingType));
 END_ENTITY;
@@ -7707,29 +7722,36 @@

 ENTITY IfcManifoldSolidBrep
  ABSTRACT SUPERTYPE OF (ONEOF
    (IfcAdvancedBrep
    ,IfcFacetedBrep))
  SUBTYPE OF (IfcSolidModel);
    Outer : IfcClosedShell;
 END_ENTITY;

 ENTITY IfcMapConversion
+ SUPERTYPE OF (ONEOF
+   (IfcMapConversionScaled))
  SUBTYPE OF (IfcCoordinateOperation);
    Eastings : IfcLengthMeasure;
    Northings : IfcLengthMeasure;
    OrthogonalHeight : IfcLengthMeasure;
    XAxisAbscissa : OPTIONAL IfcReal;
    XAxisOrdinate : OPTIONAL IfcReal;
    Scale : OPTIONAL IfcReal;
-   ScaleY : OPTIONAL IfcReal;
-   ScaleZ : OPTIONAL IfcReal;
+END_ENTITY;
+
+ENTITY IfcMapConversionScaled
+ SUBTYPE OF (IfcMapConversion);
+   ScaleX : IfcReal;
+   ScaleY : IfcReal;
+   ScaleZ : IfcReal;
 END_ENTITY;

 ENTITY IfcMappedItem
  SUBTYPE OF (IfcRepresentationItem);
    MappingSource : IfcRepresentationMap;
    MappingTarget : IfcCartesianTransformationOperator;
 END_ENTITY;

 ENTITY IfcMarineFacility
  SUBTYPE OF (IfcFacility);
@@ -8881,20 +8903,22 @@

 ENTITY IfcProjectOrder
  SUBTYPE OF (IfcControl);
    PredefinedType : OPTIONAL IfcProjectOrderTypeEnum;
    Status : OPTIONAL IfcLabel;
    LongDescription : OPTIONAL IfcText;
 END_ENTITY;

 ENTITY IfcProjectedCRS
  SUBTYPE OF (IfcCoordinateReferenceSystem);
+   GeodeticDatum : OPTIONAL IfcIdentifier;
+   VerticalDatum : OPTIONAL IfcIdentifier;
    MapProjection : OPTIONAL IfcIdentifier;
    MapZone : OPTIONAL IfcIdentifier;
    MapUnit : OPTIONAL IfcNamedUnit;
  WHERE
    IsLengthUnit : NOT(EXISTS(MapUnit)) OR (MapUnit.UnitType = IfcUnitEnum.LENGTHUNIT);
 END_ENTITY;

 ENTITY IfcProjectionElement
  SUBTYPE OF (IfcFeatureElementAddition);
    PredefinedType : OPTIONAL IfcProjectionElementTypeEnum;
@@ -10085,20 +10109,29 @@
    Height : IfcPositiveLengthMeasure;
    BottomRadius : IfcPositiveLengthMeasure;
 END_ENTITY;

 ENTITY IfcRightCircularCylinder
  SUBTYPE OF (IfcCsgPrimitive3D);
    Height : IfcPositiveLengthMeasure;
    Radius : IfcPositiveLengthMeasure;
 END_ENTITY;

+ENTITY IfcRigidOperation
+ SUBTYPE OF (IfcCoordinateOperation);
+   FirstCoordinate : IfcMeasureValue;
+   SecondCoordinate : IfcMeasureValue;
+   Height : IfcLengthMeasure;
+ WHERE
+   SameCoordinateType : (('IFC4X3_DEV.IFCLENGTHMEASURE' IN TYPEOF(FirstCoordinate)) AND ('IFC4X3_DEV.IFCLENGTHMEASURE' IN TYPEOF(SecondCoordinate))) OR (('IFC4X3_DEV.IFCPLANEANGLEMEASURE' IN TYPEOF(FirstCoordinate)) AND ('IFC4X3_DEV.IFCPLANEANGLEMEASURE' IN TYPEOF(SecondCoordinate)));
+END_ENTITY;
+
 ENTITY IfcRoad
  SUBTYPE OF (IfcFacility);
    PredefinedType : OPTIONAL IfcRoadTypeEnum;
  WHERE
    CorrectPredefinedType : NOT(EXISTS(PredefinedType)) OR
  (PredefinedType <> IfcRoadTypeEnum.USERDEFINED) OR
  ((PredefinedType = IfcRoadTypeEnum.USERDEFINED) AND EXISTS (SELF\IfcObject.ObjectType));
 END_ENTITY;

 ENTITY IfcRoadPart
@@ -12116,20 +12149,25 @@
 END_ENTITY;

 ENTITY IfcWasteTerminalType
  SUBTYPE OF (IfcFlowTerminalType);
    PredefinedType : IfcWasteTerminalTypeEnum;
  WHERE
    CorrectPredefinedType : (PredefinedType <> IfcWasteTerminalTypeEnum.USERDEFINED) OR
  ((PredefinedType = IfcWasteTerminalTypeEnum.USERDEFINED) AND EXISTS (SELF\IfcElementType.ElementType));
 END_ENTITY;

+ENTITY IfcWellKnownText;
+   WellKnownText : IfcWellKnownTextLiteral;
+   CoordinateReferenceSystem : IfcCoordinateReferenceSystem;
+END_ENTITY;
+
 ENTITY IfcWindow
  SUBTYPE OF (IfcBuiltElement);
    OverallHeight : OPTIONAL IfcPositiveLengthMeasure;
    OverallWidth : OPTIONAL IfcPositiveLengthMeasure;
    PredefinedType : OPTIONAL IfcWindowTypeEnum;
    PartitioningType : OPTIONAL IfcWindowTypePartitioningEnum;
    UserDefinedPartitioningType : OPTIONAL IfcLabel;
  WHERE
    CorrectPredefinedType : NOT(EXISTS(PredefinedType)) OR
  (PredefinedType <> IfcWindowTypeEnum.USERDEFINED) OR

@evandroAlfieri @SergejMuhic @pjanck please review carefully whether this is according to what has been discussed

@Moult @TLiebich please review

Moult commented 1 year ago

There seem to be 4 things happening here:

  1. +1 WKT is being supported. I think this is a good idea. A big side effect is that the Name is now optional, until someone reads the WR and understands why. No issues from me here. I trust IfcWellKnownTextLiteral is a sane option as opposed to just plain old IfcText?
  2. (?) IfcMapConversionScaled is a new subtype which offers ScaleY and ScaleZ. I personally never saw a scenario for ScaleY and especially for ScaleZ, which is why I asked @LeeGregory12d about it last year: https://github.com/buildingSMART/IFC4.3.x-development/issues/43 - he's the domain expert and says "I believe Andre had cases where ScaleX != ScaleY", but I'd like to hear it from the horses mouth that this is an actual thing and not just academic. The existing "Scale" attribute is already something I need to have a meeting with the surveyor and architect to explain what's happening. Also, then that someone needs to tell me the equation for it, otherwise it's useless for implementers. I've asked this before here as well https://github.com/buildingSMART/IFC4.3.x-development/issues/44 and unfortunately on this one @LeeGregory12d who is a mathematician didn't know the answer.
  3. -1 GeodeticDatum : OPTIONAL IfcIdentifier; and VerticalDatum : OPTIONAL IfcIdentifier; are added. This makes no sense to me, as it already exists on IfcCoordinateReferenceSystem. Is it a mistake? Shouldn't I also see in the diff that GeodeticDatum/VerticalDatum has been removed from IfcCoordinateReferenceSystem?
  4. (?) IfcRigidOperation is added. I don't understand this at all. Can a georef expert explain why this is needed, how it differs to mapconversion, etc?
LeeGregory12d commented 1 year ago

Dion,

  1. Yes WKT is required because EPSG does not cover all cases. I agree that without some sort of name it would be hard to refer to it in any text document. Also, I don't think a WKT actually has anywhere that states what the Epoch is, except in the text name.
    And without the Epoch, the WKT is still vague.

  2. New subtype with ScaleY and ScaleZ. I believe this was introduced because of the limitations in the existing IfcMapConversion which has a scale that was to be applied to X, Y and Z. As was pointed out in Model Setup IDM that this is not very useful for transforming from local engineering coordinates to map coordinates except in the case when the combined scale factor is 1 and hence Scale = 1. The documentation read that Scale was for converting units like millimeters to metres, which is a totally different thing than for going from local engineering coordinates to the coordinates of the underlying map (ie map coordinates) where you are going from ground metres to map metres.

But because in the past there may be files that used IfcMapConversion where Scale was applied to X, Y and Z, the argument was that you couldn't change IfcMapConversion itself and a new entity was needed that actually did what the documentation for IfcMapConversion actually said - converting from local engineering coordindates to map coordinates. It is not turning millimetres in metres, it is for converting from ground metres to map metres.

If ScaleX does not equal ScaleY it is a 2D Affine transformation and the equations are almost the same but with two scales. My concern with a 2D Affine transformation is that circles do not transform to circles. An example of when that is used is when you are digitising from paper where it has stretched in one direction only. However it can't be applied to Alignments which are made up of arcs and transitions and Alignments are fundamental to infrastructure projects. So yes, I would like to see and example of its use for transforming anything other than points and lines.

Definitely ask any Surveyor about how they go from local engineering coordinates to map coordinates (eg MGA2020) and they will tell you that they calculate a 2D Helmert from a number of points known in both systems and that gives you a translation, rotation and a scale (combined scale factor) which is applied to both X and Y. It is not applied to Z. There is an example in the Model Setup IDM, although after checking it recently, I think the scale was actually reported as 1/Scale. I can send you a discussion paper I recently wrote that works through that case and one from Queensland.

Note: the surveyor will only use that process if the area being considered is such that the combined scale factor can be taken as constant over the data being converted. If that is not the case then things get really nasty.

  1. The IcRigitOperation was introduced for the case when all the data is already in map metres (eg MGA2020) but it needs a translation to get into actual map coordinates. And example of this is when the data has been truncated by removing the first couple of digits.
    So it is not a case of converting from local engineering coordinates to map coordinates at all and hence shouldn't be done using IfcMapConversion.

Note: Truncating of map coordinates is regularly done when people give data to bridge designers who then work away on it as though it was local engineering coordinates. Fair enough. BUT there is a real danger in this practice because someone may think that to get the designed bridge back into map coordinates that you simply do the reverse translation and get map coordinates. This incorrect reversal can lead to huge mistakes when working with long bridges. Similarly with long roads, long railway lines and long tunnels

Moult commented 1 year ago
  1. Makes sense :)
  2. Agreed, the docs were wrong but I think all of us knew the true intention of surface<->grid conversions. The georef guide doc clarifies this and also clarifies how it is only applied to X and Y and not Z.

But because in the past there may be files that used IfcMapConversion where Scale was applied to X, Y and Z, the argument was that you couldn't change IfcMapConversion itself and a new entity was needed that actually did what the documentation for IfcMapConversion actually said - converting from local engineering coordindates to map coordinates. It is not turning millimetres in metres, it is for converting from ground metres to map metres.

This logic does not make sense. The scale was always used for X and Y only, and never Z. The new entity is also not changing Scale, it is adding ScaleY and ScaleZ. I agree the docs need to be fixed

Even without the complexities of circles, if no equation can be shown on how to use these ScaleY and ScaleZ, then these parameters don't have any meaning and should not be added.

  1. I don't think it is necessary. That's just a MapConversion where Scale=1, and is already supported with current workflows. Why add another entity?
LeeGregory12d commented 1 year ago
  1. The logic is sound enough. If "the scale was always used for X and Y only, and never Z" then the argument would be that IfcMapConversion was not being used as documented. And unfortunately the documentation has never been changed since IFC 4 was released.

Regarding the equations: Yes if the new entity is suppose to be a 2D Afffine in X and Y, and a translation in Z, then equations for a 2D Affine, or at least some text saying that, is required to be added somewhere.

I'm not certain where the equations go as I never saw them documented anywhere in the IFC 4 standard. That is is probably why misunderstandings arose about what it all meant.

  1. Yes it is necessary. Mathematically the result will be the same but the extra information being conveyed is critical. If only IfcMapConversion is used then you can't tell the different between the two cases: (a) the data is in map metres
    (b) the data is in local engineering coordinates

The difference is important.

In case (a) no alarm bells are needed because everything should already be in map metres. If the person provided the data didn't convert the data correctly into map metres then you definitely know who to sue when the project screws up.

Knowing that you are in case (b), means that you are warned and so checks can be made. For example, you can check how large the linear area that the local data in the file covers. Depending on the precision required, the area covered may not be acceptable and the conversion is invalid. You can then hopefully follow that up before a disaster occurs

For case (b), I think it is critical that if the transformation was constructed from a number points whose coordinates were known in both local coordinates and map coordinates, then there should be a standard method of including those points in the IFC file. That way the person receiving the file can check if the attributes in the IfcMapConversion are correct, or if a calculation error has been made.

Moult commented 1 year ago

And unfortunately the documentation has never been changed since IFC 4 was released.

Yeah, unfortunately, something I've been trying to get changed for a while now, since every developer who asked questions about it on the bSI forums was told its true intention, and then the Georef guide https://www.buildingsmart.org/wp-content/uploads/2020/02/User-Guide-for-Geo-referencing-in-IFC-v2.0.pdf was published (which you were a part of) which everyone followed. So I'd argue people have always ignored the misleading doc string and followed the georef guide instead. I really pushed to get the doc stuff fixed and aligned with the Georef guide for IFC4.3 but it never happened. Maybe now is still not too late.

  1. Thanks, I understand that the difference is semantic and you are right that it is critical, and so my next question is that why doesn't it use the same attributes as MapConversion minus the Scale? Why suddenly a different convention of First and SecondCoordinate? Doesn't look very consistent.

Here's my current understanding, feel free to correct if wrong:

  1. Scenario: Project working directly in standard EPSG code with map coords, no offset:
    • ProjectedCRS is used with an EPSG Name
    • RigidOperation is used with FirstCoordinate == SecondCoordinate and Height=0
  2. Scenario Project working directly in standard EPSG with map coords, with an offset (e.g. truncation):
    • ProjectedCRS is used with an EPSG Name
    • RigidOperation is used with FirstCoordinate != SecondCoordinate as relevant and Height=X as relevant to offset
  3. Scenario Project working directly in custom WKT with map coords, no offset:
    • ProjectedCRS is used with an NULL Name
    • IfcWellKnownText is used with a relevant WKT definition (epoch potentially still a problem?)
    • RigidOperation is used with FirstCoordinate == SecondCoordinate and Height=0

And some local engineering coordinate examples:

  1. Scenario Project working in local eng coords, mapped to a standard EPSG code, with a small site where combined scale factor can be approximated as a constant:
    • ProjectedCRS is used with an EPSG name
    • IfcMapConversion is used with:
      • Eastings is likely to be non-zero, as advised by surveyor
      • Northings is likely to be non-zero, as advised by surveyor
      • OrthogonalHeight may be be non-zero, as advised by surveyor (in Australia, for AHD we usually leave this as 0)
      • XAxisAbscissa and XAxisOrdinate will be whatever the grid north vs project north difference is, as always has been
      • Scale will be the surface distance to grid distance approximated combined scale factor, as documented in the bSI Georef Guide. It is applied to X and Y, but not Z.
  2. Scenario Project working in local eng coords, mapped to a standard EPSG code, with a small site where combined scale factor can be approximated as a constant, but in a part of the world where it is funky and a different scale is needed for Y compared to X, and a Z scale is also needed for some reason:
    • ProjectedCRS is used with an EPSG name
    • IfcMapConversionScaled is used with:
      • Eastings is likely to be non-zero, as advised by surveyor
      • Northings is likely to be non-zero, as advised by surveyor
      • OrthogonalHeight may be be non-zero, as advised by surveyor (in Australia, for AHD we usually leave this as 0)
      • XAxisAbscissa and XAxisOrdinate will be whatever the grid north vs project north difference is, as always has been
      • Scale will be the surface distance to grid distance approximated combined scale factor, as documented in the bSI Georef Guide. In this odd scenario, it is only applied to X. Note that there is no equation provided for this, so right now, it is impossible to implement. See #44
      • ScaleY will be the surface distance to grid distance approximated combined scale factor, as documented in the bSI Georef Guide. In this odd scenario, it is only applied to Y. Note that there is no equation provided for this, so right now, it is impossible to implement. See #44
      • ScaleZ will be the surface distance to grid distance approximated combined scale factor, as documented in the bSI Georef Guide. In this odd scenario, it is only applied to Z. Note that there is no equation provided for this, so right now, it is impossible to implement. See #44

Note that this is the current equation copy pasted from the georef guide:

E = (A * X) - (B * Y) + Eastings
N = (B * X) + (A * Y) + Northings
H = Z + OrthogonalHeight
A = Scale * cos(Rotation),
B = Scale * sin(Rotation), and
Rotation = atan2(XAxisAbscissa, XAxisOrdinate)
aothms commented 1 year ago

Only quickly replying to

  1. -1 GeodeticDatum : OPTIONAL IfcIdentifier; and VerticalDatum : OPTIONAL IfcIdentifier; are added. This makes no sense to me, as it already exists on IfcCoordinateReferenceSystem. Is it a mistake? Shouldn't I also see in the diff that GeodeticDatum/VerticalDatum has been removed from IfcCoordinateReferenceSystem?

Sorry, I missed the bit that made this meaningful. The attributes move down to the subtype to make room for newly introduced IfcGeographicCRS (although I guess GeodeticDatum could have stayed because it's the first attr on both subtypes). I've edited the content in the first post.

Edit:

Shouldn't I also see in the diff that GeodeticDatum/VerticalDatum has been removed from IfcCoordinateReferenceSystem?

This was already the case though, or am i missing something?

evandroAlfieri commented 1 year ago

Copying from a previous conversation with @pjanck

Evandro: Any specific reason for not leaving GeodeticDatum at the level of IfcCordinateRefernceSystem, but replicate it for both ProjectedCRS and GeographicCRS? Is it the same concept? Stefan: Yes. IfcCoordinateReferenceSystem is abstract and a CRS is not necessarily defined using GeodeticDatum. See also ISO 19111. The fact, that both IfcProjectedCRS and IfcGeographicCRS both have this attribute, is coincidence. If in the future other derived classes are introduced, they may not have GeodeticDatum. As such, it appears duplicated.

LeeGregory12d commented 1 year ago

Dion, I've now been replacing "Rotation = atan2(XAxisAbscissa, XAxisOrdinate)' with "Rotation = atan( XAxisOrdinate/XAxisAbscissa)" That way it is not saying which atan function to use. Of course, XAxisAbscissa = 0 is a problem but that is why when implementing atan functions, they don't use the "/" but have two separate coordinates.

LeeGregory12d commented 1 year ago

Dion, It has been a long while since I looked at what an affine transformation is but I now realise that a 2D affine includes a translation in X and Y, a ScaleX , a ScaleY, and also rotations of the X axis AND of the Y axis. Hence unlike a 2D Helmert transformation, a rectangle can be skewed by an affine transformation.

So given the attributes in IfcMapConversion with 3 scales, I think it could only cover what I call a 2D Orthogonal Affine transformation, which is an affine transformation that is constrained so that the rotation of the X axis and the rotation of the Y axis, are the same.

PS I have no idea why XAxisAbscissa and XAxisOrdinate are used instead of one Rotation attribute.

Moult commented 1 year ago

Cheers, all that makes sense. To confirm we're all speaking the same language here, can you help confirm whether or not https://github.com/buildingSMART/IFC4.3.x-development/issues/533#issuecomment-1253158960 is correct?

TLiebich commented 1 year ago

do I understand correctly, that the conversation is about a schema change for IFC4.4 (Tunnel)?

But for IFC4.3.0.1 there is also a need to fix the documentation - the explanation of scale (=scale-x) is still unchanged and wrong. http://ifc43-docs.standards.buildingsmart.org/IFC/RELEASE/IFC4x3/HTML/lexical/IfcMapConversion.html

aothms commented 1 year ago

If I understand correctly this is 4.3.0.2 @berlotti

Sent from a mobile device. Excuse my brevity. Kind regards, Thomas

Op wo 21 sep. 2022 15:07 schreef Thomas Liebich @.***>:

do I understand correctly, that the conversation is about a schema change for IFC4.4 (Tunnel)?

But for IFC4.3.0.1 there is also a need to fix the documentation - the explanation of scale (=scale-x) is still unchanged and wrong. http://ifc43-docs.standards.buildingsmart.org/IFC/RELEASE/IFC4x3/HTML/lexical/IfcMapConversion.html

— Reply to this email directly, view it on GitHub https://github.com/buildingSMART/IFC4.3.x-development/issues/533#issuecomment-1253685689, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAILWV3L4BS5NYFSZAZREDTV7MCBPANCNFSM6AAAAAAQRLA6LI . You are receiving this because you authored the thread.Message ID: @.***>

berlotti commented 1 year ago

yes, since documentation needs a fix anyway we can take this into the 4.3.02 (TC2)

TLiebich commented 1 year ago

are we talk about the same thing?

I understand, that documentation only updates that come now post ISO-submission go into IFC4.3.0.2 (TC2) but the original proposal in this thread contain schema additions (which is far beyond what is acceptable for a TC, like new entities and modified entities (IfcRigidOperation, IfcGeographicCRS, etc.). Is this already IFC4.4.0.0?

berlotti commented 1 year ago

The changes came from the ISO review process, and are mainly reverting back to the original IFC 4. Hence they are added before the final ISO submission.

SergejMuhic commented 1 year ago

These are not yet all the changes that were agreed. @LeeGregory12d already mentioned a few. We have to update the proposal to feature all requirements. I will be working on it on the weekend.

Could we please wait with any reviews until then?

SergejMuhic commented 1 year ago

4. Thanks, I understand that the difference is semantic and you are right that it is critical, and so my next question is that why doesn't it use the same attributes as MapConversion minus the Scale? Why suddenly a different convention of First and SecondCoordinate? Doesn't look very consistent.

Unfortunately, I just followed this point through. I hope @LeeGregory12d offered good explanations for the rest. 😄

A rigid operation can be used in any reference system (see IfcGeographicCRS for example). Imagine you are in the context of the IfcCoordinateReferenceSystem also in the IFC file (no conversion, just translation and rotation). This is the typical situation in infrastructure projects. The requirement is that projected coordinates are used in the file. In order to move you project origin, you would use IfcRigidOperation. This can be done in projected, geographic, earth-centered earth-fixed or snake grid coordinate reference systems (or any other that are not yet in use or that I have missed). All of these CRS' are obviously not maps but might be in use for georeferencing.

aothms commented 1 year ago

These are not yet all the changes that were agreed. @LeeGregory12d already mentioned a few. We have to update the proposal to feature all requirements. I will be working on it on the weekend.

@SergejMuhic any updates on this. Or an indication what we're looking at exactly?

SergejMuhic commented 1 year ago

I have updated the definitions (have not yet added IfcWellKnownTextLiteral - I like it). Also added some documentation.

The rest should actually be quite the match.

I am only looking at adding a function for conversion which would make implementation unambiguous.

larswik commented 1 year ago

Also, I don't think a WKT actually has anywhere that states what the Epoch is, except in the text name. And without the Epoch, the WKT is still vague.

Maybe check here: http://docs.opengeospatial.org/is/18-010r7/18-010r7.html#128 (chapter 16) and see if it fulfills your requirements.

LeeGregory12d commented 1 year ago

Thanks Lars. It says that Coordinate Epoch it is mandatory for a coordinate set that is referenced to a dynamic CRS but it is not part of a CRS Definition.

And in "Coordinate metadata", if says "for a coordinate set referenced to a static CRS it is in the CRS definition".

However I'm not certain how it is specified in the CRS definition. Hopefully someone can point out to me where the Epoch is in a static CRS given in WKT.

For example: In the EPSG code documentation for EPSG:7855 https://epsg.io/7855), the OGC WKT is:

PROJCS["GDA2020_MGA_Zone_55", GEOGCS["GCS_GDA2020", DATUM["GDA2020", SPHEROID["GRS_1980",6378137.0,298.257222101]], PRIMEM["Greenwich",0.0], UNIT["Degree",0.0174532925199433]], PROJECTION["Transverse_Mercator"], PARAMETER["False_Easting",500000.0], PARAMETER["False_Northing",10000000.0], PARAMETER["Central_Meridian",147.0], PARAMETER["Scale_Factor",0.9996], PARAMETER["Latitude_Of_Origin",0.0], UNIT["Meter",1.0]]

I can't see where they nail down the Epoch except possibly as part of some text but I assumed that text could be be anything and has nothing to do with the actual definition.

aothms commented 1 year ago

This isn't my expertise, but looking at the docs, it seems there is the option to provide a coordinate metadata, which isn't a field in CRS, but rather, seems to wrap the CRS definition with additional fields including an epoch.

http://docs.opengeospatial.org/is/18-010r7/18-010r7.html#130

So in my interpretation if you use a coordinate metadata object it seems that there is a provision to store an epoch

COORDINATEMETADATA[
  GEOGCRS["WGS 84 (G1762)",
    DYNAMIC[FRAMEEPOCH[2005.0]],
    DATUM["World Geodetic System 1984 (G1762)",
      ELLIPSOID["WGS 84",6378137,298.257223563,LENGTHUNIT["metre",1.0]]
    ],
    CS[ellipsoidal,3],
      AXIS["(lat)",north,ANGLEUNIT["degree",0.0174532925199433]],
      AXIS["(lon)",east,ANGLEUNIT["degree",0.0174532925199433]],
      AXIS["ellipsoidal height (h)",up,LENGTHUNIT["metre",1.0]]
  ],
  EPOCH[2016.47]
]

Hopefully someone can point out to me where the Epoch is in a static CRS given in WKT.

And in "Coordinate metadata", if says:

"for a coordinate set referenced to a static CRS it is in the CRS definition".

If I understand this correctly, this is about meta-data in general not the epoch, which isn't relevant on a static crs as it is, well, static.

SergejMuhic commented 1 year ago

@TLiebich I would put IfcWellKnownTextLiteral into IfcGeometryResource if that is okay with you. @aothms your thoughts?

aothms commented 1 year ago

I don't have strong opinions on what sits in what domain. In our docs we have stated the wkt for geometrical definitions is out of scope (which reflects usage in the schema) so it wouldn't be my first preference to have it there. In the future I also don't envision more geometric usage of wkt because what we currently have in ifc is a much more expressive and semantic superset.

http://ifc43-docs.standards.buildingsmart.org/IFC/RELEASE/IFC4x3/HTML/lexical/IfcWellKnownTextLiteral.htm

LeeGregory12d commented 1 year ago

Thank Thomas, I probably read the WKT documentation incorrectly. For some reason I thought it said that epoch was somehow already there for a Static CRS. But I couldn't see where it was.

And yes, if you add the EPOCH[2016.47] into the CRS definition then it is fine.

It looks like the WKT definitions in the EPSG doco may need to be amended to include the EPOCH command. I'm not certain when EPOCH was added to WKT- it may be a recent addition.

SergejMuhic commented 1 year ago

I don't have strong opinions on what sits in what domain. In our docs we have stated the wkt for geometrical definitions is out of scope (which reflects usage in the schema) so it wouldn't be my first preference to have it there. In the future I also don't envision more geometric usage of wkt because what we currently have in ifc is a much more expressive and semantic superset.

http://ifc43-docs.standards.buildingsmart.org/IFC/RELEASE/IFC4x3/HTML/lexical/IfcWellKnownTextLiteral.htm

I do not think this is a matter of opinion so much as it is a matter of the meaning of the type. For the purpose of how the type is used, that is typically encoded in usages and not in the NOTE part of the documentation.

pjanck commented 1 year ago
  1. This is (near) impossible to review. For example, the commit has 1 changed file with 2,969 additions and 2,948 deletions, while the commit message implies a very minuscule change Non-optional ScaleX Y and Z. (Some other have over 40000 changes!) I would have preferred a view similar to this, where there are 12 files with up to max. 35 changes per file - can I achieve this somehow?

  2. Additionally, the commit referenced above is from 6 days ago, while the issue's description has been changed for the last time 15 days ago (this seems to be the summary). Therefore, I don't know what has been changed and in what state the current model is. Especially with the latest change being 6 hours ago.

  3. Is this issue a mere report of the duplication of work from this PR or a separate development? There seem to be discrepancies.

Moult commented 1 year ago

What's the status of this? Is there a new diff to review? I also haven't heard back whether my understanding in https://github.com/buildingSMART/IFC4.3.x-development/issues/533#issuecomment-1253158960 is correct :(

aothms commented 1 year ago

the commit referenced above is from 6 days ago, while the issue's description has been changed for the last time 15 days ago

@pjanck correct. I've updated the schema to be more in line with https://github.com/bSI-InfraRoom/IFC-Specification/pull/330 description above updated. IfcMapConversionScaled in particular.

there's a little bit of extra noise on the line because we're cherry picking stuff between the 4.3.x dev and iso submission branches.

I also haven't heard back ...

There's indeed a bit of unconcluded topics in this thread. If there's anybody interested in meeting on monday morning to conclude the final bits I can set sth up.

pjanck commented 1 year ago

I've updated the schema

@aothms Thank you for an update. I notice that the compared express serializations of the schema are from 2022/09/13 and 2022/09/30:

grafik

while there has been a change that should be reflected in the schema just yesterday: Space after OR. Care to double-check?

to be more in line

This implies there are differences between this issue and the referenced PR - what are these?

aothms commented 1 year ago

Space after OR. Care to double-check?

That's the cherry picking I was referring to. That's the commit on the master branch whereas the diff is computed on 23ccc36f4cc7c4d8376487e14a6c1549c624b383. Still apparently I computed the diff a couple of minutes before committing to this repo apparently looking at the exact timestamps. But you can easily check that commit diff and see that that where rule syntax change is included in the above description.

This implies there are differences between this issue and the referenced PR - what are these?

There will definitely be minor differences in the Md, but what I was referring to is mostly some uncertainty in me not fully understanding how the XML changes in the infra repo diff will affect express. So if you or sergej render these changes we can do a diff of diffs to triple check.

pjanck commented 1 year ago

That's the cherry picking I was referring to.

I see. Apologies, I only checked the dates.

me not fully understanding how the XML changes in the infra repo diff will affect express

True, it is difficult to see that. I believe there is an action in place to generate the schema when the PR gets merged. Let's hope that will be soon!

What stands out for me, is missing Supertype for IfcWellKnownText, i.e. IfcGeometricRepresentationItem, as well as different "optionality" of the attributes. Perhaps this EXPRESS-G diagram can be of help for comparison?

aothms commented 1 year ago

Supertype for IfcWellKnownText, i.e. IfcGeometricRepresentationItem

Oh, thanks, I indeed overlooked this, but this is huge. This relates to the discussion above with @SergejMuhic I really don't think we should go this far. We have perfectly adequate representation items ourselves, do we really envision well known text now as a valid item of a representation?

larswik commented 1 year ago

Supertype for IfcWellKnownText, i.e. IfcGeometricRepresentationItem

Oh, thanks, I indeed overlooked this, but this is huge. This relates to the discussion above with @SergejMuhic I really don't think we should go this far. We have perfectly adequate representation items ourselves, do we really envision well known text now as a valid item of a representation?

Hmmm, would it be wise to just make IfcWellKnownText at the same level as e.g. IfcAddress, IfcPerson etc? Furthermore, it might be wise to name it IfcWellKnownTextCRS if we are not also including WKT for simple geometry (as defined by ISO 19125-1 or OGC SFA).

aothms commented 1 year ago

I brought up similar points in an email reply, but I see know you weren't included in that email, Lars. But I would indeed opt for the most minimal change and inheriting from a widely used type has a lot of consequences.

Sent from a mobile device. Excuse my brevity. Kind regards, Thomas

Op vr 7 okt. 2022 16:02 schreef larswik @.***>:

Supertype for IfcWellKnownText, i.e. IfcGeometricRepresentationItem

Oh, thanks, I indeed overlooked this, but this is huge. This relates to the discussion above with @SergejMuhic https://github.com/SergejMuhic I really don't think we should go this far. We have perfectly adequate representation items ourselves, do we really envision well known text now as a valid item of a representation?

Hmmm, would it be wise to just make IfcWellKnownText at the same level as e.g. IfcAddress, IfcPerson etc? Furthermore, it might be wise to name it IfcWellKnownTextCRS if we are not also including WKT for simple geometry (as defined by ISO 19125-1 or OGC SFA https://www.ogc.org/standards/sfa ).

— Reply to this email directly, view it on GitHub https://github.com/buildingSMART/IFC4.3.x-development/issues/533#issuecomment-1271637926, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAILWV2V6KHNBP3YVTCHCKTWCAUO7ANCNFSM6AAAAAAQRLA6LI . You are receiving this because you were mentioned.Message ID: @.***>

SergejMuhic commented 1 year ago

Well, the "hugeness" of this can be debated. In my opinion, this is not the case. But we do have surveyor stakeholders who are complaining about exchanging IFC datasets that are not supporting simpler geometry exchanges such as points and polylines @LeeGregory12d.

Surveyor software vendors that already support WKT could easily implement surveying exchanges.

Again, the same argument of usages and MVD scope is the mechanism to restrict it. bSI is advertising complete lifecycle for IFC and authorities are going from this and demanding deliveries. We have a discrepancy between marketing and actual capabilities of the standard.

IfcWellKnownText deriving from IfcGeometricRepresentationItem simply positions it correctly in the schema. Nobody requests it for a geometry exchange (yet). And if at some point someone does, I would expect it to be limited to certain exchanges between surveyors.

LeeGregory12d commented 1 year ago

Regarding the comment: 'If I understand this correctly, this is about meta-data in general not the epoch, which isn't relevant on a static crs as it is, well, static."

Just in case there is a misunderstanding of what a Static CRS actually is. "Static" means that the longitude and latitude of a point on the ellipsoid, is the longitude and latitude of where that point was at a fixed time. That is, at the EPOCH of the Static CRS.

In practice, that means for a Static CRS, when you obtain the longitude and latitude of a point, you need to adjust the longitude and latitude of the point to where it was at the given EPOCH for the Static CRS.

So, the EPOCH is critical in the "static" case.

As an example: Australia has two official Static CRS's that define longitude and latitude, GDA94 and GDA2020. The two CRS's are based on EXACTLY the same Ellipsoid (GRS 1980).

So, what happens when a surveyor is asked to provide the longitude and latitude of a point? If asked for the longitude and latitude of the point in the static CRS GDA94, it is where that point was on1st January 1994. If asked for the longitude and latitude of the same point in the static CRS GDA2020, it is where that point was on 1st January 2020.

The difference between the two sets of longitude and latitude is approximately 1.7 metres.

That distance might not seem important, but my house is 1 metre from the side boundary. If someone mixed up the EPOCHs for measurements of my house and the side boundary, then my house could appear to be encroaching on my neighbour's property. And that would be a disaster for me. I imagine it would be even more of a disaster for trains and tunnels, or autonomous vehicles.

Note if you look at the WKT given with EPSG codes, there is no differentiation in the WKT parameters between the two static CRS's. The only clue to the EPOCH is in the Text name.

PROJCS["GDA2020 / MGA zone 55", GEOGCS["GDA2020", DATUM["Geocentric_Datum_of_Australia_2020", SPHEROID["GRS 1980",6378137,298.257222101], TOWGS84[0,0,0,0,0,0,0]], PRIMEM["Greenwich",0, AUTHORITY["EPSG","8901"]], UNIT["degree",0.0174532925199433, AUTHORITY["EPSG","9122"]], AUTHORITY["EPSG","7844"]], PROJECTION["Transverse_Mercator"], PARAMETER["latitude_of_origin",0], PARAMETER["central_meridian",147], PARAMETER["scale_factor",0.9996], PARAMETER["false_easting",500000], PARAMETER["false_northing",10000000], UNIT["metre",1, AUTHORITY["EPSG","9001"]], AXIS["Easting",EAST], AXIS["Northing",NORTH], AUTHORITY["EPSG","7855"]]

PROJCS["GDA94 / MGA zone 55", GEOGCS["GDA94", DATUM["Geocentric_Datum_of_Australia_1994", SPHEROID["GRS 1980",6378137,298.257222101], TOWGS84[0,0,0,0,0,0,0]], PRIMEM["Greenwich",0, AUTHORITY["EPSG","8901"]], UNIT["degree",0.0174532925199433, AUTHORITY["EPSG","9122"]], AUTHORITY["EPSG","4283"]], PROJECTION["Transverse_Mercator"], PARAMETER["latitude_of_origin",0], PARAMETER["central_meridian",147], PARAMETER["scale_factor",0.9996], PARAMETER["false_easting",500000], PARAMETER["false_northing",10000000], UNIT["metre",1, AUTHORITY["EPSG","9001"]], AXIS["Easting",EAST], AXIS["Northing",NORTH], AUTHORITY["EPSG","28355"]]

TLiebich commented 1 year ago

just as an explanation to @aothms sentence

there's a little bit of extra noise on the line because we're cherry picking stuff between the 4.3.x dev and iso submission branches.

bSI submitted IFC4.3.0.1 (TC1 of the bSI official release of IFC4.3.0.0) to ISO for acceptance for the DIS ballot for ISO 16739-1. And a joint editorial group of ISO and bSI experts reviewed it, come up with some compatibility issues and resolved those. On 29.09 the ISO JWG 12 made the decision to propose the IFC4.3.0.1 dated 07.09 for DIS ballot.

On 30.09 a new IFC release was submitted by bSI - now being IFC4.3.1.0 (an addendum with schema changes affecting the P21 file structure). That brought up two main issues (and in consequence "a little bit of noice")

  1. if that new addendum IFC4.3.1.0 should be the basis for ISO 16739-1 - then a new round of decision making is needed in ISO causing a delay to the overall ISO publication time frame (and many stakeholders are already pushing towards acceleration, and not slowing down)
  2. if not (ISO version remains at IFC4.3.0.1) - we would have incompatible schema versions of IFC4.3 out (in bSI and in ISO), also not desireable

and there is also the question coming up on the general bSI policy of IFC schema management - what are the procedures to publish new schema versions (in the past, a technical corrigendum was always possible on short notice, an addendum = schema change required some governance procedures).

none of it is technical - but given the ongoing discussions also here in git - do we need to force the improved definition already into IFC4.3, or would it be better to roll it out together with IFC Tunnel and IFC4.4 ?