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

IfcCompositeCurve missing Derive: Dim attribute #74

Closed
pjanck opened this issue Mar 19, 2021 · 21 comments
Closed

IfcCompositeCurve missing Derive: Dim attribute #74

pjanck opened this issue Mar 19, 2021 · 21 comments
Assignees
Labels
BUG A bug in code or checking actions

Comments

@pjanck
Copy link
Member

pjanck commented Mar 19, 2021

Looking at the code below, though: there is no DERIVE: Dim parameter specified that would return Segments[0].Dim or something similar. So it makes sense, that the checker returns false.

https://github.com/bSI-InfraRoom/IFC-Specification/blob/64f1fc0e8cdf8aba82d2b99a423ddefae2f5a2ba/IFC4x3_RC3.exp#L5105-L5119

Originally posted by @pjanck in #73 (comment)

@SergejMuhic
Copy link

SergejMuhic commented Mar 19, 2021

Can you post the checker message? IfcCurve has the derived Dim.

The IfcCurve.DIm calls IfcCurveDim, so I am not sure what the checker is complaining about.

@larswik
Copy link
Collaborator

larswik commented Mar 19, 2021

Looks like it is connected to the "dim" warning.

Errors:

    <MVDMessage>
      <MessageType>ERROR</MessageType>
      <MVDType>SCHEMADEFINITION</MVDType>
      <MessageCode>IFCSHAPEREPRESENTATION.CORRECTITEMSFORTYPE</MessageCode>
      <Instance>#56=IFCSHAPEREPRESENTATION(#55,'Axis','Curve2D',(#50))</Instance>
      <InstanceLink></InstanceLink>
      <ObjectInstance>#24=IFCALIGNMENTHORIZONTAL('3$D$u$Lvj4IRsKVLePDIuA',$,$,$,$,#21,#57,2103.72056)</ObjectInstance>
      <ObjectInstanceLink></ObjectInstanceLink>
      <Messages>
        <MessageString> where rule 'IFCSHAPEREPRESENTATION.CORRECTITEMSFORTYPE' evaluates to FALSE</MessageString>
      </Messages>
    </MVDMessage>
    <MVDMessage>
      <MessageType>ERROR</MessageType>
      <MVDType>SCHEMADEFINITION</MVDType>
      <MessageCode>IFCSHAPEREPRESENTATION.CORRECTITEMSFORTYPE</MessageCode>
      <Instance>#107=IFCSHAPEREPRESENTATION(#55,'Axis','Curve3D',(#106))</Instance>
      <InstanceLink></InstanceLink>
      <ObjectInstance>#65=IFCALIGNMENTVERTICAL('1bR$_QwLnBJv2m8fn2FwZ0',$,'PR_Twin_Branch_section',$,$,#21,#108)</ObjectInstance>
      <ObjectInstanceLink></ObjectInstanceLink>
      <Messages>
        <MessageString> where rule 'IFCSHAPEREPRESENTATION.CORRECTITEMSFORTYPE' evaluates to FALSE</MessageString>
      </Messages>
    </MVDMessage>

Warnings (maybe one per curve segment?):

    <MVDMessage>
      <MessageType>WARNING</MessageType>
      <MVDType>SCHEMADEFINITION</MVDType>
      <MessageCode>SYNTAX_VALIDATION_22</MessageCode>
      <Instance>#42=IFCCURVESEGMENT(CONTINUOUS,#36,0.,741.371391339357,#41)</Instance>
      <InstanceLink></InstanceLink>
      <ObjectInstance>#24=IFCALIGNMENTHORIZONTAL('3$D$u$Lvj4IRsKVLePDIuA',$,$,$,$,#21,#57,2103.72056)</ObjectInstance>
      <ObjectInstanceLink></ObjectInstanceLink>
      <Messages>
        <MessageString> access to non-existing attribute 'dim'</MessageString>
      </Messages>
    </MVDMessage>
    <MVDMessage>
      <MessageType>WARNING</MessageType>
      <MVDType>SCHEMADEFINITION</MVDType>
      <MessageCode>SYNTAX_VALIDATION_22</MessageCode>
      <Instance>#78=IFCCURVESEGMENT(CONTINUOUS,#76,-63.4469995157475,346.277532651367,#77)</Instance>
      <InstanceLink></InstanceLink>
      <ObjectInstance>#65=IFCALIGNMENTVERTICAL('1bR$_QwLnBJv2m8fn2FwZ0',$,'PR_Twin_Branch_section',$,$,#21,#108)</ObjectInstance>
      <ObjectInstanceLink></ObjectInstanceLink>
      <Messages>
        <MessageString> access to non-existing attribute 'dim'</MessageString>
      </Messages>
    </MVDMessage>

@SergejMuhic
Copy link

This is not IfcCompositeCurve though. It is IfcCurveSegment. This was addressed though in c5a9282615009b56fb6b2e8eb6faf25f7241d2db. I guess Andreas did not use the latest exp file to insert the new WRs. I have not inserted the new WRs into ifcdoc as we were testing. I have to do that first and then regenerate the exp for him. Probably some time next week, unless it is super urgent.

@pjanck
Copy link
Member Author

pjanck commented Mar 19, 2021

Right, I see. So just an update on the checker is necessary?

@larswik
Copy link
Collaborator

larswik commented Mar 22, 2021

Please let me know when the checker has been updated.

@larswik
Copy link
Collaborator

larswik commented Mar 30, 2021

I tested with new alignments and new checker and got this message. It happens for the IfcGradientCurve and I suspect that it has to do with IfcPolynomialCurve...

  <MVDMessage>
      <MessageType>ERROR</MessageType>
      <MVDType>SCHEMADEFINITION</MVDType>
      <MessageCode>IFCCOMPOSITECURVE.SAMEDIM</MessageCode>
      <Instance>#106=IFCGRADIENTCURVE((#78,#82,#87,#91,#96,#100,#105),UNKNOWN,#50,$)</Instance>
      <InstanceLink></InstanceLink>
      <ObjectInstance>#65=IFCALIGNMENTVERTICAL('1mggcWhRT1buFoX8TmC_xS',$,'PR_Twin_Branch_section',$,$,#21,#108)</ObjectInstance>
      <ObjectInstanceLink></ObjectInstanceLink>
      <Messages>
        <MessageString> where rule 'IFCCOMPOSITECURVE.SAMEDIM' evaluates to FALSE</MessageString>
      </Messages>
    </MVDMessage>
  </MVDMessageSet>

@SergejMuhic
Copy link

Do your segments have a dim of 2? I think I have to update the CurveDim for IfcPolynomialCurve then. It currently returns 3. Probably should return 2 if one of the coefficient lists is '$'.

@larswik
Copy link
Collaborator

larswik commented Mar 30, 2021

CoefficientsZ = '$'
image

@SergejMuhic
Copy link

Thanks @larswik. Compiling this function is going to be interesting. :)

@SergejMuhic
Copy link

SergejMuhic commented Mar 30, 2021

Here my proposal:

  1. CurveDim function update for IfcPolynomialCurve:
    IF ('IFCGEOMETRYRESOURCE.IFCPOLYNOMIALCURVE' IN TYPEOF(Curve))
    IF (NOT EXISTS(Curve\IfcPolynomialCurve.CoefficientsX) OR NOT EXISTS(Curve\IfcPolynomialCurve.CoefficientsY) OR NOT EXISTS(Curve\IfcPolynomialCurve.CoefficientsZ))
    THEN RETURN(2);
    ELSE
    THEN RETURN(3);
    END_IF
    END_IF;
  2. Where rule for IfcPolynomialCurve:
    ValidCoefficient : (EXISTS(CoefficientsX) AND EXISTS(CoefficientsY)) OR (EXISTS(CoefficientsX) AND EXISTS(CoefficientsZ)) OR (EXISTS(CoefficientsY) AND EXISTS(CoefficientsZ))

Looking forward to feedback.

@larswik
Copy link
Collaborator

larswik commented Mar 30, 2021

Needs at least X/Y, X/Z or Y/Z and if one of X, Y or Z is missing then dim=2, else 3. Looks ok to me!

@SergejMuhic
Copy link

Probably should fix the where clause to:
ValidCoefficient : (EXISTS(CoefficientsX) AND EXISTS(CoefficientsY)) OR (EXISTS(CoefficientsX) AND EXISTS(CoefficientsZ)) OR (EXISTS(CoefficientsY) AND EXISTS(CoefficientsZ) OR (EXISTS(CoefficientsX) AND EXISTS(CoefficientsY) AND EXISTS(CoefficientsZ))

@jmirtsch
Copy link
Collaborator

As the polynomial has a placement, seems to me it should also validate if the placement is 2d or 3d.
It would be quite good if we could start establishing a unit test based approach to cross check the where rule (different scenarios of 2d and 3d configurations as well as valid and invalid).

@SergejMuhic
Copy link

SergejMuhic commented Mar 31, 2021

Yep, that was my first thought also. But IfcAxis2PlacementLinear gets its Dim from its basis curve. Then we could have a problem. Probably a polynomial would not be placed on itself, but you never know. :) Seemed to be more explicit to do it based on coefficients because there is no other restriction in setting the Coefficients.

Possible scenario: Could be that I have a dataset where IfcPolynomialCurve.Placement is Dim 2 but all three Coefficients are set. It would not be forbidden, but how to interpret it?

@SergejMuhic
Copy link

Any objections?

@pjanck
Copy link
Member Author

pjanck commented Apr 6, 2021

Discussion during the tech call - proposal:

  1. IfcPolynomialCurve: Introduce a where rule that enforces SameDim for the number of coefficients and placement:
CorrectPlacementDim : (NOT EXISTS(CoefficientsX) OR NOT EXISTS(CoefficientsY) OR NOT EXISTS(CoefficientsZ)) OR (Position.Dim = 3);

Reasoning:

  • a curve that is missing one of the coefficients can still be placed in 3D (but may as well be placed in 2D),
  • a curve that has all coefficients, HAS to be placed 3D.

This works well with the ValidCoefficients where rule.

  1. The dimension is then handled within IfcCurveDim:
IF ('IFC4X3_RC3.IFCPOLYNOMIALCURVE' IN TYPEOF(Curve))
    IF ( NOT EXISTS(Curve\IfcPolynomialCurve.CoefficientsZ) AND Curve\IfcPolynomialCurve.Position.Dim = 2 )
        THEN RETURN(2);
    END_IF;
    RETURN(3);
END_IF;

This rule assumes that as soon as we have a z-dimension, the curve is 3D. This argumentation is based on the way conics handle their dimension.

@AlexBrad1eyCT
Copy link
Collaborator

Is this Resolved, by This Commit, if so can we close the issue??

@SergejMuhic
Copy link

By itself, this rule does not enforce the same Dim of placement and curve. It states that if one of the coefficients is missing Placement.Dim does not have to be 3.

To actually enforce the dims to be the same, we would have to look at it from a different direction. We have two valid Position.Dim values. If we are enforcing those, we need to cover that one case where Position.Dim is 2. In my view, a valid definition looks like this: either Position.Dim equals 2 and CoefficientsZ is empty or Position.Dim is 3. So, my proposal would be:

((Position.DIm=2) AND (NOT EXISTS(CoefficientsZ))) OR (Position.Dim=3)

Or fix text of where rule.

@pjanck
Copy link
Member Author

pjanck commented Apr 13, 2021

Correction for my post above:

Introduce a where rule that enforces SameDim for the number of coefficients and placement:

Should read:

Introduce a where rule that enforces CorrectPlacementDim for the number of coefficients and placement:

@pjanck
Copy link
Member Author

pjanck commented Apr 13, 2021

Proposed documentation for the where rule:

A curve that is missing one of the coefficients can be positioned in 2D or 3D; otherwise, its position has to be in 3D.

Possible namings:

  • CorrectPositionDim
  • Enforce3dPositionFor3dCurve
  • ValidPositionDim

@SergejMuhic
Copy link

SergejMuhic commented Apr 22, 2021

Closing. See commit bSI-InfraRoom/IFC-Specification@c6c3aa8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUG A bug in code or checking actions
Projects
None yet
Development

No branches or pull requests

5 participants