Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC][Hybrid Script] Syntax sugars for Hybrid Script #2262

Closed
were opened this issue Dec 10, 2018 · 13 comments
Closed

[RFC][Hybrid Script] Syntax sugars for Hybrid Script #2262

were opened this issue Dec 10, 2018 · 13 comments

Comments

@were
Copy link
Contributor

were commented Dec 10, 2018

Hybrid Script is still under a very preliminary stage of development.
Comparing with its counterpart in FaceBook, TorchScript, a lot of syntax sugars are not supported yet.

  1. Global function call Done
@script
def foo(a):
    # balah
    return b

@script
def goo(a):
    b = foo(a)
    return b

foo can be found in __global__ dict, so it is not hard to do this I believe.

  1. Tensor level operations
# RFC
# actually, all these tensor operation rules will be hardcoded in the compiler
# i do not want to make it super complicate...
  1. Triple inequity Done
if 0 <= a < n:
    # balah
  1. In-place Operator
#RFC: Is this one good enough? Users are required to explicitly return the tensor with side effect.
@script
def goo(a):
    for i in range(a.shape[0]):
        a[i] += 1
    return a

@kevinthesun Any more suggestions?

@kevinthesun
Copy link
Contributor

continue

is not supported.

@kevinthesun
Copy link
Contributor

Logical operator 'and', 'or' and 'not' are not supported.

@were
Copy link
Contributor Author

were commented Dec 10, 2018

@kevinthesun
I am not very sure how continue, break and in-loop return can be supported, because so far HalideIR does not support it yet.

@tqchen
Do you mind if I add break/continue and while loop in Halide IR?

Also I am not sure, what impact will be on the control flow graph, after adding these backend supports.

@tqchen
Copy link
Member

tqchen commented Dec 10, 2018

I don't think it is a good idea to support break/continue, since they will break the analysis completely.

@were
Copy link
Contributor Author

were commented Dec 10, 2018

@tqchen Exactly, I know the current analysis is specific to loops without break/continue. That's why I am also reluctant to add it either.

@tqchen tqchen changed the title [Hybrid Script] Syntax sugars for Hybrid Script [RFC][Hybrid Script] Syntax sugars for Hybrid Script Dec 10, 2018
@kevinthesun
Copy link
Contributor

kevinthesun commented Dec 13, 2018

Chain of logical expr is not supported.

if a < b and c < d and e < f: 

@were
Copy link
Contributor Author

were commented Dec 14, 2018

@kevinthesun Can you be more specific that I can replicate it?

@kevinthesun
Copy link
Contributor

kevinthesun commented Dec 14, 2018

@tvm.hybrid.script
def test(a, bool_val):
    b = output_tensor((a.shape[0],), a.dtype)
    if a[0] > 4 and a[1] < 4.5 and bool_val:
        b[0] = max(a[0], 5.0)
    return b

if __name__ == '__main__':
    a_plc = tvm.placeholder((5,), dtype="float32")
    bool_v = tvm.const(True, dtype="bool")
    out = test(a_plc, bool_v)

@were
Copy link
Contributor Author

were commented Dec 14, 2018

#2287 Can you refer this one's last test case? At least that works fine for me.

@were
Copy link
Contributor Author

were commented Dec 14, 2018

@kevinthesun Can you show me something more concrete instead of a rough code snippet?

@were
Copy link
Contributor Author

were commented Dec 29, 2018

@kevinthesun
Follow up your request on tvm.container.Array:
Unfortunately, tvm.container.Array cannot be directly accessed by HalideIR.

i = tvm.var('i')
a = tvm.convert([1, 2, 3])
expr = a[i] # This is illegal

@kevinthesun
Copy link
Contributor

@were Would you think supporting array argument requires much more work than expected? If this is case, I can roll back multibox operators to use ir_build for now in #2353.

@tqchen
Copy link
Member

tqchen commented Feb 3, 2019

Close this RFC for now as most discussed items has been implemented, please open new threads for new feature discussion

@tqchen tqchen closed this as completed Feb 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants