https://issues.dlang.org/show_bug.cgi?id=14786

Iain Buclaw <ibuclaw@gdcproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ibuclaw@gdcproject.org
          Component|druntime                    |phobos

--- Comment #1 from Iain Buclaw <ibuclaw@gdcproject.org> ---
Moving from druntime to phobos (std.math.pow is the component, unless the compiler was able to fold the operation).

--

https://issues.dlang.org/show_bug.cgi?id=14786

--- Comment #2 from Iain Buclaw <ibuclaw@gdcproject.org> ---
This seems to be the correct extract from std.math to look at:

---
5675│             // Result is real only if y is an integer
5676│             // Check for a non-zero fractional part
5677├>            if (y > -1.0 / real.epsilon && y < 1.0 / real.epsilon)
5678│             {
5679│                 long w = cast(long)y;
5680│                 if (w != y)
5681│                     return sqrt(x); // Complex result -- create a NaN
5682│                 if (w & 1) sign = -1.0;
5683│             }
5684│             x = -x;
---


If 'e = long.max', it is representable, and the (w & 1) check sets sign to
'-1.0'

As instead 'e = long.max + 2', it is not representable, and so the signed-ness is not adjusted.


Checked result against libm routines, and yes it looks like a bug on our side.

--

https://issues.dlang.org/show_bug.cgi?id=14786

--- Comment #3 from Iain Buclaw <ibuclaw@gdcproject.org> ---
OK, it looks like std.math.pow is only implemented to work with numbers only within the following boundaries.

-1.0/real.epsilon < X < 1/real.epsilon

I *could* relax this, but we'll need to test raising some random numbers to the power of an integer between long.max and ulong.max.

--

https://issues.dlang.org/show_bug.cgi?id=14786

--- Comment #4 from thomas.bockman@gmail.com ---
I haven't tested it extensively yet, but here is a fixed version of that section:

// Result is real only if y is an integer
// Check for a non-zero fractional part
enum real maxPrecise = ulong.max;
enum real minPrecise = -maxPrecise;
if (y >= minPrecise && y <= maxPrecise)
{
    real absY = abs(y);
    ulong w = cast(ulong)absY;
    if (w != absY)
        return sqrt(x); // Complex result -- create a NaN
    if (w & 1) sign = -1.0;
}
x = -x;

However, looking at the whole function, I think it would benefit from a major refactoring as well. So much duplicated logic...

Is that something I should do, or would it mostly likely be rejected on the basis of, "If it ain't broke, don't fix it"?

--

https://issues.dlang.org/show_bug.cgi?id=14786

--- Comment #5 from Iain Buclaw <ibuclaw@gdcproject.org> ---
(In reply to thomas.bockman from comment #4)
> I haven't tested it extensively yet, but here is a fixed version of that section:
> 
> // Result is real only if y is an integer
> // Check for a non-zero fractional part
> enum real maxPrecise = ulong.max;
> enum real minPrecise = -maxPrecise;
> if (y >= minPrecise && y <= maxPrecise)
> {
>     real absY = abs(y);
>     ulong w = cast(ulong)absY;
>     if (w != absY)
>         return sqrt(x); // Complex result -- create a NaN
>     if (w & 1) sign = -1.0;
> }
> x = -x;
> 
> However, looking at the whole function, I think it would benefit from a major refactoring as well. So much duplicated logic...
> 

There is no duplicated logic. ;)

--

https://issues.dlang.org/show_bug.cgi?id=14786

--- Comment #6 from thomas.bockman@gmail.com ---
I have filed a pull request on GitHub:
[phobos PR #3598](https://github.com/D-Programming-Language/phobos/pull/3598)

--

https://issues.dlang.org/show_bug.cgi?id=14786

--- Comment #7 from github-bugzilla@puremagic.com ---
Commits pushed to master at https://github.com/D-Programming-Language/phobos

https://github.com/D-Programming-Language/phobos/commit/49d4720e5cc730bad9e265d45050e73d8954999a Fix Issue 14786 -  std.math.pow sometimes gets the sign of the result wrong.

https://github.com/D-Programming-Language/phobos/commit/0816ad6113a950224c50707a3cd77fea29e9b03a Merge pull request #3598 from tsbockman/issue_14786

Fix Issue 14786 -  std.math.pow sometimes gets the sign of the result wrong.

--

https://issues.dlang.org/show_bug.cgi?id=14786

github-bugzilla@puremagic.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--

https://issues.dlang.org/show_bug.cgi?id=14786

--- Comment #8 from github-bugzilla@puremagic.com ---
Commits pushed to stable at https://github.com/D-Programming-Language/phobos

https://github.com/D-Programming-Language/phobos/commit/49d4720e5cc730bad9e265d45050e73d8954999a Fix Issue 14786 -  std.math.pow sometimes gets the sign of the result wrong.

https://github.com/D-Programming-Language/phobos/commit/0816ad6113a950224c50707a3cd77fea29e9b03a Merge pull request #3598 from tsbockman/issue_14786

--