google / starlark-go

Starlark in Go: the Starlark configuration language, implemented in Go
BSD 3-Clause "New" or "Revised" License
2.26k stars 204 forks source link

s390x precision seems to be a little bit off in some operations #488

Closed alexsaezm closed 11 months ago

alexsaezm commented 11 months ago

On s390x the precision of some operation makes TestExecFile fail.

# go test -run "^TestExecFile$" go.starlark.net/starlark
--- FAIL: TestExecFile (0.35s)
    starlarktest.go:118: Traceback (most recent call last):
          testdata/math.star:186:10: in <toplevel>
          assert.star:14:14: in _eq
        Error: 0.7853981633974484 != 0.7853981633974483
    starlarktest.go:118: Traceback (most recent call last):
          testdata/math.star:187:10: in <toplevel>
          assert.star:14:14: in _eq
        Error: -0.7853981633974484 != -0.7853981633974483
    starlarktest.go:118: Traceback (most recent call last):
          testdata/math.star:188:10: in <toplevel>
          assert.star:14:14: in _eq
        Error: 0.7853981633974484 != 0.7853981633974483
    starlarktest.go:118: Traceback (most recent call last):
          testdata/math.star:189:10: in <toplevel>
          assert.star:14:14: in _eq
        Error: -0.7853981633974484 != -0.7853981633974483
    starlarktest.go:118: Traceback (most recent call last):
          testdata/math.star:195:10: in <toplevel>
          assert.star:14:14: in _eq
        Error: 0.7853981633974484 != 0.7853981633974483
    starlarktest.go:118: Traceback (most recent call last):
          testdata/math.star:196:10: in <toplevel>
          assert.star:14:14: in _eq
        Error: -0.7853981633974484 != -0.7853981633974483
    starlarktest.go:118: Traceback (most recent call last):
          testdata/math.star:202:10: in <toplevel>
          assert.star:14:14: in _eq
        Error: 0.7853981633974484 != 0.7853981633974483
    starlarktest.go:118: Traceback (most recent call last):
          testdata/math.star:203:10: in <toplevel>
          assert.star:14:14: in _eq
        Error: -0.7853981633974484 != -0.7853981633974483
    starlarktest.go:118: Traceback (most recent call last):
          testdata/math.star:310:10: in <toplevel>
          assert.star:14:14: in _eq
        Error: 0.5493061443340549 != 0.5493061443340548
    starlarktest.go:118: Traceback (most recent call last):
          testdata/math.star:311:10: in <toplevel>
          assert.star:14:14: in _eq
        Error: -0.5493061443340549 != -0.5493061443340548
    starlarktest.go:118: Traceback (most recent call last):
          testdata/math.star:348:10: in <toplevel>
          assert.star:14:14: in _eq
        Error: -0.761594155955765 != -0.7615941559557649
    starlarktest.go:118: Traceback (most recent call last):
          testdata/math.star:349:10: in <toplevel>
          assert.star:14:14: in _eq
        Error: -0.761594155955765 != -0.7615941559557649
FAIL
FAIL    go.starlark.net/starlark        0.376s
FAIL

I tried with several versions of Go (from 1.16 up to 1.20.4) and the results are always the same.

adonovan commented 11 months ago

Thanks for reporting this. It looks like s390's atan (and other operations) return a result that is higher by 1 ULP (unit in the last place). The assert.eq(x, y) function needs to be modified so that if x and y are both finite floats, it instead performs the check y-ulp(x) <= x <= y+ulp(x) where ulp(x) is the value of the last mantissa bit of x. This will need another Go function (ulp) exposed to Starlark.

adonovan commented 11 months ago

Hi Álex, would you try out #490 and see if it fixes the problem? I don't have an s390 next to me. :) Thanks.

alexsaezm commented 11 months ago

Hi Álex, would you try out #490 and see if it fixes the problem? I don't have an s390 next to me. :) Thanks.

For the record, yes, it worked fine. Thank you very much!