On Tue, May 3, 2016 at 7:15 PM, Robert Bradshaw <rober...@gmail.com> wrote:
> 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.
No, but the problem is that Cython is a pretty heavy layer of
abstraction. In the example I gave no long double function was ever
used explicitly--it was added by a compiler optimization. I probably
wouldn't write that code thinking that I will need a workaround on
platforms that don't have truncl.
In the slightly more explicit cases of fmod and pow I might agree.
Though there still isn't a great way to anticipate the need to write
separate handling for long double functions into a module, for a
platform that doesn't support them. I can't write something in a
Cython module like:
if sys.platform != 'cygwin':
def my_long_double_func(long double x):
....
Maybe would be nice to have a simple compiler directive that outright
disables compiling a function depending on a platform check. (I think
this would also be useful for the test framework--the current system
of module-level tags is not fine-grained as I would like, I'm finding.
>> 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.
>
>
> 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?
That's a reasonable workaround. But I would argue that should be baked
into Cythonized code, at least optionally, and should generate a
warning.
Best,
Erik
_______________________________________________
cython-devel mailing list
cython-devel@python.org
https://mail.python.org/mailman/listinfo/cython-devel
