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

1.0 / abs(-0.0) returns -inf instead of inf #16494

Closed
timotheecour opened this issue Dec 28, 2020 · 1 comment
Closed

1.0 / abs(-0.0) returns -inf instead of inf #16494

timotheecour opened this issue Dec 28, 2020 · 1 comment

Comments

@timotheecour
Copy link
Member

timotheecour commented Dec 28, 2020

  • abs(-0.0) returns -0.0 instead of 0.0, which causes further issues, eg:
    1.0 / abs(-0.0) returns -inf instead of inf
  • likewise, it should behave like fabs even for NaN (ie setting the sign bit)

Example1: -0.0

doAssert 1.0 / abs(-0.0) == Inf

Current Output

fails

Expected Output

works

Example 2: same issue with nans

when true:
  proc fabs(a: cdouble): cdouble {.importc.}
  proc signbit(a: cdouble): cint {.importc, header: "<math.h>".}
  proc copySign(a, b: cdouble): cdouble {.importc: "copysign", header: "<math.h>".}
  let nan2 = copySign(NaN, -1.0)
  doAssert nan2.fabs.signbit == 0 # ok
  doAssert nan2.abs.signbit == 0 # fails

Possible Solution

for implementation of abs(SomeFloat), use importc fabs and fabsf in RT + VM (via vmops), with a fallback for js as usual; the fallback for js can use the same logic as for copySign(see #16406)

Additional Information

devel 1.5.1 6d442a4

@ringabout
Copy link
Member

proc abs*(x: float64): float64 {.noSideEffect, inline.} =
  if x < 0.0: -x
  elif x == 0.0: 0.0
  else: x
proc abs*(x: float32): float32 {.noSideEffect, inline.} =
  if x < 0.0: -x
  elif x == 0.0: 0.0
  else: x

@ringabout ringabout self-assigned this Dec 30, 2020
ringabout added a commit to ringabout/Nim that referenced this issue Dec 30, 2020
@ringabout ringabout mentioned this issue Dec 30, 2020
1 task
mildred pushed a commit to mildred/Nim that referenced this issue Jan 11, 2021
* fix nim-lang#16494

* fix

* fix

* fix

* fix

* fix

* fix performance

* add comments

* improve performance

* Update lib/system.nim

Co-authored-by: Timothee Cour <[email protected]>

* Update lib/system.nim

Co-authored-by: Timothee Cour <[email protected]>

* Update tests/stdlib/tmath_misc.nim

Co-authored-by: Timothee Cour <[email protected]>

* Update tests/stdlib/tmath_misc.nim

Co-authored-by: Timothee Cour <[email protected]>

Co-authored-by: Timothee Cour <[email protected]>
ardek66 pushed a commit to ardek66/Nim that referenced this issue Mar 26, 2021
* fix nim-lang#16494

* fix

* fix

* fix

* fix

* fix

* fix performance

* add comments

* improve performance

* Update lib/system.nim

Co-authored-by: Timothee Cour <[email protected]>

* Update lib/system.nim

Co-authored-by: Timothee Cour <[email protected]>

* Update tests/stdlib/tmath_misc.nim

Co-authored-by: Timothee Cour <[email protected]>

* Update tests/stdlib/tmath_misc.nim

Co-authored-by: Timothee Cour <[email protected]>

Co-authored-by: Timothee Cour <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants