BUG: Fix casting from longdouble to long - #9971

Why not npy_uint64 with chunk_size 64?

digits in the original code was unsigned

Hadn't noticed that it was unsigned. Yes, could probably change to sizeof(npy_long) * CHAR_BIT and npy_long

Changed to use PyLong_FromUnsignedLongLong, npy_ulonglong, and NPY_BITSOF_ULONGLONG

OK, maybe the reason for 32 bit chunk is because npy_long can be 32bit, and chunk needs to be npy_long.

There is a funny character here. Windows newline? OK, it says "No newline at end of file", so no problem.

Newline added

Alternate name proposal npy_longdouble_to_PyLong.

Either seems fine. Maybe the npy_ alternative is safer in case Python3 decides to define this too.

This seemed to be just for the long/int choice below, which is far easier (although a little slower) to handle by just producing a long, and letting python decide whether to downcast to an int.

I think we can now remove the @realtype@ repeat var. @unsigntyp@ too

(I don't exactly understand why these lines were needed before, since C-casting truncates the double fractional part anyway.. but the new code looks correct so I won't worry too much).

Oh I think it was probably so the x < LONG_MAX and x < LONG_MIN checks aren't affected by the fractional value, in the case of exact equality of the integer part of x to those values.

Indeed, that was the purpose - but I don't think the decrease in clarity is worth it for a marginal performance improvement on only python 2.

I'll remove the repeat vars - good catch

This would probably be generally handy throughout the codebase.

A job for another PR: would there be a better place to put it, and do we want to cal lit Py_SETREF and #ifndef Py_SETREF it?

If we decide to use it I agree that doing and #ifndef seems reasonable.

For anyone else reading, it looks like this python macro was created in python3.4, and there are a lot of discussions of it on the python mailing list and on the python issue tracker. It is still going through some changes as of april 2016, I think they decided to split it into two macros Py_SETREF and Py_XSETREF.

One motivation for it is to avoid a certain kind of bug, according to the comment in CPython. On the other hand it's not part of the "stable api", so there are some risks in using it.

Hmm, should be C style comment.

Careless, good spot. This is the reason for the Mac failure?

No. At the moment I'm suspecting the power function.

The problem is this line: huge_ld = 1234 * np.longdouble(2) ** exp. We need someone running on a Mac to check this out. Maybe a fallback double function is being used somewhere.

Here it is, powl is blacklisted on the Mac

/* powl gives zero division warning on OS X, see gh-8307 */
#if defined(HAVE_POWL) && defined(NPY_OS_DARWIN)
#undef HAVE_POWL
#endif

Although I don't see why that should be a problem, the double value of the fallback function should be OK with result cast to long double, so the multiplication should work. Maybe inlining is failing somewhere ...

Fired up the old Mac. The Mac double pow function errors with exp == 1024, Linux works with that. I'm simply going to use exp = 1023 as a fix in this case.

exp=1023 doesn't work, because then the result fits in double - the goal here is to produce a longdouble that exceeds the limits for a double.

I suppose using exp=1023 and multiplying by 2 would work

Yes, that is what I did. See #10000 . Oh, looky there, issue 10000, it's like watching the odometer turn over ...

Guess we will eventually find out if this all works for IBM double double.

This test fails on ppc64.

I think I'm going to add @dec.skipif(platform.machine().startswith("ppc64")) here.