inducer / loopy

A code generator for array-based code on CPUs and GPUs
http://mathema.tician.de/software/loopy
MIT License
587 stars 73 forks source link

What is Loopy's function namespace? (was: Is min a reduction or a 2-arg minimum like OpenCL) #73

Open skyreflectedinmirrors opened 7 years ago

skyreflectedinmirrors commented 7 years ago

Sorry about the many pull requests / issues today! Been playing around with reductions, and breaking things in all sorts of interesting ways :)

Here's a fun one:

import loopy as lp
import pyopencl as cl
lp.set_caching_enabled(False)

ctx = cl.create_some_context()
queue = cl.CommandQueue(ctx)

knl = lp.make_kernel('{[i]: 0 < i < 10}',
"""
        out[i] = min(i, 5)
""",
['...'])

print lp.generate_code(knl)[0]

Results in:

  File "test.py", line 14, in <module>
    print lp.generate_code(knl)[0]
  File "/home/nick/loopy/loopy/codegen/__init__.py", line 523, in generate_code
    codegen_result = generate_code_v2(kernel)
  File "/home/nick/loopy/loopy/codegen/__init__.py", line 383, in generate_code_v2
    kernel = preprocess_kernel(kernel)
  File "/home/nick/loopy/loopy/preprocess.py", line 912, in preprocess_kernel
    kernel = realize_reduction(kernel, unknown_types_ok=False)
  File "/home/nick/loopy/loopy/preprocess.py", line 649, in realize_reduction
    new_expressions = (cb_mapper(insn.expression),)
  File "/home/nick/.local/lib/python2.7/site-packages/pymbolic/mapper/__init__.py", line 134, in __call__
    return method(expr, *args, **kwargs)
  File "/home/nick/loopy/loopy/symbolic.py", line 1345, in map_reduction
    result = self.callback(expr, self.rec, **kwargs)
  File "/home/nick/loopy/loopy/preprocess.py", line 559, in map_reduction
    "supposed to reduce over: " + ", ".join(bad_inames))
loopy.diagnostic.LoopyError: reduction used within loop(s) that it was supposed to reduce over: i

However,

import loopy as lp
import pyopencl as cl
lp.set_caching_enabled(False)

ctx = cl.create_some_context()
queue = cl.CommandQueue(ctx)

knl = lp.make_kernel('{[i]: 0 < i < 10}',
"""
        out[i] = abs(-i)
""",
['...'])

print knl(queue)[1]

works as expected (both are defined in the opencl function manglers).

inducer commented 7 years ago

cc @mattwala

inducer commented 7 years ago

min(i, ...) is a reduction. So, technically, the iname it reduces over (i) does not exist at its output. So this is all as designed. No bugs, just insufficient docs.

mattwala commented 7 years ago

I think what @arghdos is saying is that min is also a builtin function in OpenCL which is defined by the function manglers in loopy, and that the "reduction" interpretation is masking the "function" interpretation.

mattwala commented 7 years ago

Somewhat tangential: This issue also brings up the point that the syntax for reductions is a bit confusing, because the first parameter looks like a variable. Maybe it's time to consider another syntax for reductions (maybe min<i>(...))?

inducer commented 7 years ago

min<i>(...): I'm open to it, although we'd need to accept both types of syntax for a transition period.

I think what @arghdos is saying is that min is also a builtin function in OpenCL which is defined by the function manglers in loopy, and that the "reduction" interpretation is masking the "function" interpretation.

I see. numpy's solution is to use min and minimum, so my thinking is that we would follow its lead.

skyreflectedinmirrors commented 7 years ago

@mattwala that's correct. Even using min(3, 4) (i.e. no inames at all) results in the same error.

numpy.minimum refers to an element-wise minimum of two arrays, which could be interpreted as the minimum of two arrays of size one (i.e. OpenCL's min). numpy.min is similar to the min reduction currently implemented.

I might be inclined towards the min<i>(...) syntax for clarity in all reductions

inducer commented 7 years ago

The uncomfortable decision beneath all this is what the function namespace in loopy should actually be.

I'm leaning towards the third with some measure of the first.

One somewhat explicit form of the first would be accepting min$tgt() or min$cl() as explicitly target-specific function calls as a start. I don't really want to change the (existing) meaning of min. I'm OK exposing OpenCL's min as minimum following numpy as a loopy-lang thing.

How do you guys feel about this proposal?

skyreflectedinmirrors commented 7 years ago

Well, FWIW I've switched to an adding a fmin \ fmax function mangler which works fine. I guess that would be an example of the first form - although, reasonably general as it should apply to all C-based targets. I agree that changing the current meaning min would be a bad idea (breaking a whole bunch of current code).

I don't love the explicit min$cl() as it breaks the ability to easily change between targets---unless min$tgt() would be general, and adapt to whatever target is specified? but in that case it's not really the first case anymore is it?

I think the most sensible choice is to pick a (very) small number of simple math functions (min, max, abs, pow, exp, etc.) to implement in our own namespace. Honestly, the only functions that absolutely need to be done this way are min and max, although we may want to include more for the sake of consistency? It's fairly simple to add other functions that exist in the target via the mangler (although, I think at some point I'll try to add an example to the docs).

I'm fairly indifferent to the syntax used there, although I think that min$tgt() might make more sense than maximum(), simply because it explicitly tells you that something's going on in loopy more clearly differentiating the min reduction from the min target call

inducer commented 7 years ago

Yeah, I agree that min$cl() doesn't make a lot of sense. I'm not loving minimum vs min either, but there's at least precedent for it (in numpy), and IMO numpy is at least a reasonable thing to align with name-wise.

I imagine with time we'll do a combination:

This was a helpful discussion at least as a cautionary matter before I let even more CL semantics "leak" into loopy. :)

zachjweiner commented 5 years ago

@inducer and I were discussing a related issue and think that dotted namespaces might be useful here. The context was that I wanted to use pymbolic's math functions (e.g., Lookups to math) for differentiation but found that loopy doesn't know what to do with a Lookup. Certainly smoothing this out would be nice regardless, but dotted namespaces are certainly unambiguous and pythonic - what do you all think?