On Tue, May 3, 2016 at 3:04 AM, Erik Bray <erik.m.b...@gmail.com> wrote:
> On Thu, Apr 28, 2016 at 9:29 AM, Robert Bradshaw > <rober...@math.washington.edu> wrote: > > On Wed, Apr 27, 2016 at 3:07 AM, Erik Bray <erik.m.b...@gmail.com> > wrote: > >> > >> On Tue, Apr 26, 2016 at 10:55 PM, Robert Bradshaw <rober...@gmail.com> > >> wrote: > >> > On Tue, Apr 26, 2016 at 8:36 AM, Erik Bray <erik.m.b...@gmail.com> > >> > wrote: > >> >> > >> >> On Tue, Apr 26, 2016 at 5:16 PM, Dima Pasechnik > >> >> <dimpase+git...@gmail.com> wrote: > >> >> > Hi, > >> >> > certainly we did something with Sage on cygwin to work around > >> >> > these... > >> >> > Just in case, > >> >> > >> >> From what I can tell there are several places where Sage has hacked > >> >> around this issue in different packages, but it's not doing anything > >> >> specifically with it for Cython. Sage uses the Cephes math lib to > >> >> support these functions on FreeBSD--and apparently used to use it on > >> >> Cygwin too, but disabled that for some reason. > >> >> > >> >> Regardless, Cython should ultimately do something sensible in these > >> >> cases. > >> >> > >> >> My general thinking is that in cases where Cython generates code > >> >> containing C math functions, it ought to support a fallback. This > >> >> will require some feature checks so that Cython can generate > wrappers, > >> >> when necessary, around the double versions of those functions (as > >> >> Numpy currently does). > >> > > >> > > >> > Let's make things concrete. You're complaining that something like > >> > > >> > cdef extern from "math.h": > >> > long double sqrtl(long double) > >> > > >> > def foo(long double x): > >> > return sqrtl(x) > >> > > >> > Doesn't work on Cygwin? > >> > > >> > The same is true for *any* C function that you use that's not totally > >> > portable (this is the bane of trying to use C). I don't think Cython > >> > should > >> > be detecting this and substituting a (less accurate) sqrt for sqrtl in > >> > this > >> > case. If you want do do this, write your own headers that > >> > (conditionally) > >> > define these things however you want. > >> > >> No, not at all. That would be silly. Let me be clearer... > >> > >> > Or, are you complaining that Cython's test suite doesn't pass on some > >> > Cygwin > >> > because there are tests of features not available on Cygwin? (Your > >> > original > >> > email isn't clear.) If so, the test framework can be set up to exclude > >> > these > >> > tests on that platform. > >> > >> There are really two concerns. This is one of them, yes. The first > >> question is what would be the best way to exclude certain tests on > >> certain platforms? There's no clear documentation on that (which is > >> fine, but it's why I'm asking :) > > > > > > Probably add a tag and then modify runtests.py to detect Cygwin and > > automatically add this to the exclusion list. > > Okay. > > >> The second concern, and more serious, is that there *are* cases where > >> Cython generates code that uses long double functions where, for > >> example, long double is passed as an argument. For example the > >> following code > >> > >> def truncate_long_double(long double x): > >> cdef float r = int(x) > >> return r > >> > >> compiles to something that includes: > >> > >> /* "truncl.pyx":2 > >> * def truncate_long_double(long double x): > >> * cdef float r = int(x) # <<<<<<<<<<<<<< > >> * return r > >> */ > >> __pyx_t_1 = truncl(__pyx_v_x); > >> __pyx_v_r = __pyx_t_1; > >> > >> /* "truncl.pyx":3 > >> * def truncate_long_double(long double x): > >> * cdef float r = int(x) > >> * return r # <<<<<<<<<<<<<< > >> */ > >> __Pyx_XDECREF(__pyx_r); > >> __pyx_t_2 = PyFloat_FromDouble(__pyx_v_r); if > >> (unlikely(!__pyx_t_2)) __PYX_ERR(0, 3, __pyx_L1_error) > >> > >> It's not not clear what the best way would be to *not* use this code > >> on platforms where truncl missing (and there are a handful of other > >> examples besides truncl). This code is generated by an optimization > >> that was added here: http://trac.cython.org/ticket/400 In this case > >> replacing truncl with trunc shouldn't hurt. > > > > > > Here, yes, but in general truncl(x) != trunc(x) for, say, 2**54 + 1. > > > >> It's not uncommon (I think) to ship pre-cythonized C sources in a > >> package's source distribution so that it can be installed (especially > >> by pip) without requiring the user to have Cython. This code will > >> fail to compile on platforms like Cygwin. > >> > >> But it's not clear how to handle this at Cythonization time either > >> since it doesn't know in advance what platform it will be targeting. > >> I'm suggesting that maybe rather than using these functions directly > >> Cython should generate some fallbacks for where they're not available, > >> or at least have the option to. > > > > > > The safest is to never use the long double type in this case--we assume > that > > if long double is present (used), so are the corresponding functions in > > math.h (which is wrong for this platform, but if we need to use powl I > don't > > know what else to do). Alternatively, ship your own (conditionally > defined) > > fallbacks. > > I agree that it would be safest not to use long double where long > double functions are not support, and an application that does should > probably check for that. > > The problem is that Cython is generating code that simply *cannot* be > compiled on such platforms. Does Cython ever generate such code when long doubles are not used in the source? If so, we should fix that. But I don't think Cython should be swapping in double versions for long double operations. > My suggestion would be to replace the > current calls to [trunc,fmod,pow]l to the appropriate > __Pyx_%(func)_%(type) wrapper which would provide two alternatives: > One that works properly, and one that falls back to using the double > version of the function (Numpy does this so there is some precedent). > I would go further and suggest throwing up a pre-compiler warning when > using the double versions. > > The only question is what condition (in my mind) is what to use as a > condition for selecting the fallbacks. For now I'm thinking something > quite specific like > > #if defined(__CYGWIN__) && defined(_LDBL_EQ_DBL) > > since I'm apparently the only person who cares about this at the > moment, and Cygwin is the platform I'm targeting. > > Again, my only real concern right now is that Cython generates code > that can be compiled on Cygwin. > <https://mail.python.org/mailman/listinfo/cython-devel> > If you want this behavior, wouldn't including a header #if defined(__CYGWIN__) && defined(_LDBL_EQ_DBL) #define truncl trunc #define fmodl fmod #define powl pow #endif as part of your package do the job?
_______________________________________________ cython-devel mailing list cython-devel@python.org https://mail.python.org/mailman/listinfo/cython-devel
