cornell-zhang / heterocl

HeteroCL: A Multi-Paradigm Programming Infrastructure for Software-Defined Heterogeneous Computing
https://cornell-zhang.github.io/heterocl/
Apache License 2.0
322 stars 92 forks source link

[API] Enable declarative code with hcl.def #166

Closed seanlatias closed 4 years ago

seanlatias commented 4 years ago

In this PR, we enable using declarative code along with hcl.def. Following is an example.

@hcl.def_([a.shape, b.shape, c.shape])                                                                                                                                                           
def add(a, b, c):                                                                                                                                                                                
    d = hcl.compute(a.shape, lambda *x: a[x] + b[x], "d")                                                                                                                                        
    hcl.update(c, lambda *x: d[x] + 1, "u")                                                                                                                                                      

add(a, b, c)

More examples can be found in the test.

zhangzhiru commented 4 years ago

Can we rename "hcl.def_" to "hcl.kernel"?

seanlatias commented 4 years ago

Do you think kernel is better or module is better? I remembered one reason you did not want to use kernel is to avoid the confusion between OpenCL kernel.

comaniac commented 4 years ago

IMHO, at least the underline should not be used in APIs. It usually indicates private or internal variables/methods.

Sent with GitHawk

zhangzhiru commented 4 years ago

Do you think kernel is better or module is better? I remembered one reason you did not want to use kernel is to avoid the confusion between OpenCL kernel.

I'm now leaning towards "kernel" since "module" sounds too hardware centric.

zhangzhiru commented 4 years ago

IMHO, at least the underline should not be used in APIs. It usually indicates private or internal variables/methods.

Sent with GitHawk

agreed

seanlatias commented 4 years ago

The reason why we were using underscore is to avoid name conflict with reserved names. Should we also update other imperative DSL? For example, from hcl.if_ to hcl.If?

Blaok commented 4 years ago

According to PEP8, it is encouraged to use a trailing underscore when colliding with a reserved keyword instead of trying to mangle/change the name. Besides, underscore indicates protected/private attributes when used as the prefix, not suffix. Since hcl.def_ etc. are not class names, hcl.Def violates PEP8, too. IMHO, I think hcl.def_ is good enough. kernel can be confusing to people who works with OS kernels and def leverages the existing concept in Python.

seanlatias commented 4 years ago

Aside from PEP8, the original reason of using hcl.def_ is to be consistent with other imperative DSL. Basically, by applying with this decorator, the original Python function becomes a HeteroCL function that can be synthesized to hardware.

zhangzhiru commented 4 years ago

IMHO, I think hcl.def_ is good enough. kernel can be confusing to people who works with OS kernels and def leverages the existing concept in Python.

"def_" is even more confusing since (1) we have already defined a function using "def" (2) the name itsef carries no meaning. The "kernel" concept is well understood in GPU computing. I don't remember seeing concerns from OS developers. They are not the intended users anyway.

zhangzhiru commented 4 years ago

The reason why we were using underscore is to avoid name conflict with reserved names. Should we also update other imperative DSL? For example, from hcl.if_ to hcl.If?

Yes, the shorter the better. We should also rename "Stage" to "Scope". There we are not using "stage_". So we already have inconsistency with our keyword naming.

zhangzhiru commented 4 years ago

Aside from PEP8, the original reason of using hcl.def_ is to be consistent with other imperative DSL. Basically, by applying with this decorator, the original Python function becomes a HeteroCL function that can be synthesized to hardware.

I don't see why PEP8 is relevant here. The suggestion is on the function argument. We are creating a new set of keywords for our own DSL. What is the other imperative DSL?

comaniac commented 4 years ago

I agree that prefix underscore should be avoided at all not only because of PEP8 but also Python 3 interpreter (class methods/variables with underscore prefix will not be exposed by default). However, even PEP8 doesn’t forbid suffix underscore, it is still not a good practice for APIs. Note that this API is supposed to be used by end users instead of HeteorCL developers. It should be trivial and easy. I agree the reason of using “kernel” suggested by @zhangzhiru. Other directions might be “op”, “operator”, “accelerate”, “target(acc_name”.

Sent with GitHawk

zhangzhiru commented 4 years ago

Basically, by applying with this decorator, the original Python function becomes a HeteroCL function that can be synthesized to hardware.

The users will then ask what's the meaning of a HeteroCL function? A function without the def decorator can also be synthesized into hardware. Similarly, a function with def can be executed on the CPU. All we care for is to define a module boundary.

I'm open to other suggestions, but "def_" is simply too vague and confusing.

seanlatias commented 4 years ago

Just think about the difference between a Python if and the imperative DSL hcl.if_. The previous one will not be reflected in the IR and thus not reflected in the hardware while the latter does. You may ask in which case we want to use Python if in HeteroCL. I can imagine that in the future, after we really support heterogeneous programming, the user can write something like this in the algorithm.

if target == cpu:
  # use MKL library
else:
  # use HeteroCL DSL to implement same functionality

In the above example, we are using Python if instead of HeteroCL if because this if statement should not be reflected in the hardware. The decision is a compile-time decision. Similarly, we need to make difference between a Python function and a HeteroCL function. The Python function can be useful to make the program cleaner but we do not necessarily want that to be in the hardware. And since Python uses def to declare a function, I chose to use hcl.def_ for HeteroCL functions. I'm ok with both with and without underscore. As for hcl.Stage, we do not have a correspondence in Python. Thus, we can have whatever name we want.

Blaok commented 4 years ago

I think violating Python conventions confuses people who already are familiar with Python. We cannot really create any new keywords because we are embedded in Python. For end users, I think concise and easy-to-understand documentation are more important than names for the APIs.

zhangzhiru commented 4 years ago

Just think about the difference between a Python if and the imperative DSL hcl.if_. The previous one will not be reflected in the IR and thus not reflected in the hardware while the latter does. You may ask in which case we want to use Python if in HeteroCL.

Note that our programming model should be user centric. So all the discussions on whether something is reflected in the IR or not are moot. We need a separate developer document to further clarify our naming convention.

zhangzhiru commented 4 years ago

I think violating Python conventions confuses people who already are familiar with Python. We cannot really create any new keywords because we are embedded in Python. For end users, I think concise and easy-to-understand documentation are more important than names for the APIs.

Again, I don't see where we are violating the Python conventions. For those who have to use our imperative DSL should already understand this is not regular software Python code. A keyword is defined with respect to our own DSL.

zhangzhiru commented 4 years ago

As for hcl.Stage, we do not have a correspondence in Python. Thus, we can have whatever name we want.

So what is the meaning of stage, from the user's perspective?

zhangzhiru commented 4 years ago

For end users, I think concise and easy-to-understand documentation are more important than names for the APIs.

Forgot to address this point. Yes, it's very important to have good documentations. But if we don't have meaningful names for the important keywords in our DSL, there is very little hope that we will be able to provide an easy-to-understand doc since our thinking process from the get-go is lacking clarity.

comaniac commented 4 years ago

I am not sure I agree that documentation is more important then naming. At most of time the first program written by users is modified from a provided example. If the example is hard to understand then the first impression of the users would be bad. In addition, even documentation is more important then naming doesn’t mean naming could be a mess.

zhangzhiru commented 4 years ago

After discussing with Sean, we will use noinline instead of def_ or kernel

seanlatias commented 4 years ago

I'll file another PR for renaming.