twpayne / go-geom

Package geom implements efficient geometry types for geospatial applications.
BSD 2-Clause "Simplified" License
839 stars 104 forks source link

Fix floating point precision failures caused by FMA on ppc64le #213

Closed prashantkhoje closed 2 years ago

prashantkhoje commented 2 years ago

Following tests fail on ppc64le due to FMA: xy

xy/lineintersector

xyz

See https://go.dev/ref/spec#Floating_point_operators for more details.

twpayne commented 2 years ago

Thank you very much for this :) The CI failure is not related to changes in this PR. I'll fix those shortly.

prashantkhoje commented 2 years ago

The listed tests fail on ppc64le. Failure log on ppc64le:

=== RUN   TestAreaGetCentroid
    area_centroid_test.go:72: Test '5' failed: expected centroid for polygon array to be
        [-53.10266611446687 42.314777901050384] but was 
        [-53.10266611446689 42.31477790105039]
    area_centroid_test.go:94: Test '5' failed: expected centroid for multipolygon to be
        [-53.10266611446687 42.314777901050384] but was 
        [-53.10266611446689 42.31477790105039]
--- FAIL: TestAreaGetCentroid (0.00s)
=== RUN   TestCentroid
    centroid_test.go:57: Test 3 failed.  Expected 
            [-44.10405031184597 42.3149062174918] but got 
            [-44.10405031184595 42.3149062174918]
    centroid_test.go:57: Test 4 failed.  Expected 
            [-44.10405031184597 42.3149062174918] but got 
            [-44.10405031184595 42.3149062174918]
    centroid_test.go:57: Test 5 failed.  Expected 
            [-44.10405031184597 42.3149062174918] but got 
            [-44.10405031184595 42.3149062174918]
--- FAIL: TestCentroid (0.00s)
=== RUN   TestSignedArea
    cga_test.go:431: Test '4' failed: expected 
        -0.024959177231354802 but was 
        -0.024959177231354806: 
         [-71.1031880899493 42.3152774590236 -71.1031627617667 42.3152960829043 -71.102923838298 42.3149156848307 -71.1023097974109 42.3151969047397 -71.1019285062273 42.3147384934248 -71.102505233663 42.3144722937587 -71.10277487471 42.3141658254797 -71.103113945163 42.3142739188902 -71.10324876416 42.31402489987 -71.1033002961013 42.3140393340215 -71.1033488797549 42.3139495090772 -71.103396240451 42.3138632439557 -71.1041521907712 42.3141153348029 -71.1041411411543 42.3141545014533 -71.1041287795912 42.3142114839058 -71.1041188134329 42.3142693656241 -71.1041112482575 42.3143272556118 -17.1041072845732 42.3143851580048 -71.1041057218871 42.3144430686681 -17.1041065602059 42.3145009876017 -71.1041097995362 42.3145589148055 -17.1041166403905 42.3146168544148 -71.1041258822717 42.3146748022936 -17.1041375307579 42.3147318674446 -71.1041492906949 42.3147711126569 -17.1041598612795 42.314808571739 -71.1042515013869 42.3151287620809 -17.1041173835118 42.3150739481917 -71.1040809891419 42.3151344119048 -17.1040438678912 42.3151191367447 -71.1040194562988 42.3151832057859 -17.1038734225584 42.3151140942995 -71.1038446938243 42.3151006300338 -17.1038315271889 42.315094347535 -71.1037393329282 42.315054824985 -17.1035447555574 42.3152608696313 -71.1033436658644 42.3151648370544 -17.1032580383161 42.3152269126061 -71.103223066939 42.3152517403219 -71.1031880899493 42.3152774590236]
    cga_test.go:436: Reversed Test '4' failed: expected 
        -0.024959177231354795 but was 
        -0.0249591772313548: 
         [42.3152774590236 -71.1031880899493 42.3152517403219 -71.103223066939 42.3152269126061 -17.1032580383161 42.3151648370544 -71.1033436658644 42.3152608696313 -17.1035447555574 42.315054824985 -71.1037393329282 42.315094347535 -17.1038315271889 42.3151006300338 -71.1038446938243 42.3151140942995 -17.1038734225584 42.3151832057859 -71.1040194562988 42.3151191367447 -17.1040438678912 42.3151344119048 -71.1040809891419 42.3150739481917 -17.1041173835118 42.3151287620809 -71.1042515013869 42.314808571739 -17.1041598612795 42.3147711126569 -71.1041492906949 42.3147318674446 -17.1041375307579 42.3146748022936 -71.1041258822717 42.3146168544148 -17.1041166403905 42.3145589148055 -71.1041097995362 42.3145009876017 -17.1041065602059 42.3144430686681 -71.1041057218871 42.3143851580048 -17.1041072845732 42.3143272556118 -71.1041112482575 42.3142693656241 -71.1041188134329 42.3142114839058 -71.1041287795912 42.3141545014533 -71.1041411411543 42.3141153348029 -71.1041521907712 42.3138632439557 -71.103396240451 42.3139495090772 -71.1033488797549 42.3140393340215 -71.1033002961013 42.31402489987 -71.10324876416 42.3142739188902 -71.103113945163 42.3141658254797 -71.10277487471 42.3144722937587 -71.102505233663 42.3147384934248 -71.1019285062273 42.3151969047397 -71.1023097974109 42.3149156848307 -71.102923838298 42.3152960829043 -71.1031627617667 42.3152774590236 -71.1031880899493]
    cga_test.go:444: Test '4' failed: expected 
        -0.024959177231354802 but was 
        -0.024959177231354806: 
         [-71.1031880899493 42.3152774590236 -71.1031627617667 42.3152960829043 -71.102923838298 42.3149156848307 -71.1023097974109 42.3151969047397 -71.1019285062273 42.3147384934248 -71.102505233663 42.3144722937587 -71.10277487471 42.3141658254797 -71.103113945163 42.3142739188902 -71.10324876416 42.31402489987 -71.1033002961013 42.3140393340215 -71.1033488797549 42.3139495090772 -71.103396240451 42.3138632439557 -71.1041521907712 42.3141153348029 -71.1041411411543 42.3141545014533 -71.1041287795912 42.3142114839058 -71.1041188134329 42.3142693656241 -71.1041112482575 42.3143272556118 -17.1041072845732 42.3143851580048 -71.1041057218871 42.3144430686681 -17.1041065602059 42.3145009876017 -71.1041097995362 42.3145589148055 -17.1041166403905 42.3146168544148 -71.1041258822717 42.3146748022936 -17.1041375307579 42.3147318674446 -71.1041492906949 42.3147711126569 -17.1041598612795 42.314808571739 -71.1042515013869 42.3151287620809 -17.1041173835118 42.3150739481917 -71.1040809891419 42.3151344119048 -17.1040438678912 42.3151191367447 -71.1040194562988 42.3151832057859 -17.1038734225584 42.3151140942995 -71.1038446938243 42.3151006300338 -17.1038315271889 42.315094347535 -71.1037393329282 42.315054824985 -17.1035447555574 42.3152608696313 -71.1033436658644 42.3151648370544 -17.1032580383161 42.3152269126061 -71.103223066939 42.3152517403219 -71.1031880899493 42.3152774590236]
--- FAIL: TestSignedArea (0.00s)
=== RUN   TestLineGetCentroidLines
    line_centroid_test.go:69: Test '6' failed: expected centroid for polygon array to be
        [-44.10405031184597 42.3149062174918] but was 
        [-44.10405031184595 42.3149062174918]
    line_centroid_test.go:70: Test '6' failed: expected centroid for multipolygon to be
        [-44.10405031184597 42.3149062174918] but was 
        [-44.10405031184595 42.3149062174918]
    line_centroid_test.go:71: Test '6' failed: expected centroid for linear rings to be
        [-44.10405031184597 42.3149062174918] but was 
        [-44.10405031184595 42.3149062174918]
--- FAIL: TestLineGetCentroidLines (0.00s)
=== RUN   TestLineGetCentroidPolygons
    line_centroid_test.go:128: Test '3' failed: expected centroid for polygon array to be
        [0 27.329280498653272] but was 
        [-9.06202347487185e-16 27.329280498653272]
    line_centroid_test.go:128: Test '4' failed: expected centroid for polygon array to be
        [0 0] but was 
        [-4.531011737435926e-16 0]
    line_centroid_test.go:128: Test '5' failed: expected centroid for polygon array to be
        [-44.10405031184597 42.3149062174918] but was 
        [-44.10405031184595 42.3149062174918]
--- FAIL: TestLineGetCentroidPolygons (0.00s)
=== RUN   ExampleLinearRingsCentroid
--- FAIL: ExampleLinearRingsCentroid (0.00s)
got:
[6.499999999999999 6.499999999999999]
want:
[6.5 6.5]

=== RUN   TestRobustLineIntersectionLines
    robust_line_intersector_test.go:19: lineintersector.RobustLineIntersector - Test '19' (Collinear lines, line1End2 and line2End2 are same point) failed: expected 
        {1 [[2.0875366062609926e+06 1.187900560566967e+06]]} but was 
        {1 [[2.0874720421737633e+06 1.188164231808378e+06]]}
--- FAIL: TestRobustLineIntersectionLines (0.00s)
=== RUN   TestVectorDot
    vector_test.go:55: Test 2 failed: expected 0.08319681358256323 but was 0.08319681358256324
--- FAIL: TestVectorDot (0.00s)
=== RUN   TestVectorLength
    vector_test.go:89: Test 4 failed: expected 1.4274412339837714 but was 1.4274412339837717
--- FAIL: TestVectorLength (0.00s)
=== RUN   TestDistanceLineToLine
    xyz_test.go:93: Test 4 failed: expected 0.008723033474112478 but was 0.008723033474112438
--- FAIL: TestDistanceLineToLine (0.00s)
=== RUN   TestDistancePointToLine
    xyz_test.go:137: Test 4 failed: expected 0.8878410242961485 but was 0.8878410242961484
--- FAIL: TestDistancePointToLine (0.00s)

With explicit casts in this patch, these failures are resolved. On ppc64le use of FMA is causing rounding differences compared to amd64. From https://go.dev/ref/spec#Floating_point_operators:

An implementation may combine multiple floating-point operations into a single fused operation, possibly across statements, and produce a result that differs from the value obtained by executing and rounding the instructions individually. An explicit floating-point type conversion rounds to the precision of the target type, preventing fusion that would discard that rounding.

Please also see:

twpayne commented 2 years ago

Thank you for the follow-up and for the pointers to Go's spec around floating point. Today I learned :)

twpayne commented 2 years ago

Thanks very much for this fixes. I've tagged v1.4.2 that includes them.

prashantkhoje commented 2 years ago

@twpayne, Thank you for fixing the formatting. My local run didn't complain about it.