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

[Lang] Support chained assignments #3062

Merged
merged 10 commits into from
Oct 8, 2021
Merged

Conversation

gaocegege
Copy link
Collaborator

@gaocegege gaocegege commented Sep 30, 2021

Fix #2659

This PR is to support chained assignments

Test case:

@ti.kernel
def func():
    (a, b) = (c, d) = (7, 8)
    aa = bb = -1
    print(a, b, c, d)
    print(aa, bb)

func()

Python generated code

Initial AST:
def func():
    a, b = c, d = 7, 8
    aa = bb = -1
    print(a, b, c, d)
    print(aa, bb)

Preprocessed:
def func():
    import taichi as ti
    if 1:
        if 1:
            __tmp_tuple = ti.expr_init_list((7, 8), 2)
            a = ti.expr_init(__tmp_tuple[0])
            b = ti.expr_init(__tmp_tuple[1])
            del __tmp_tuple
        if 1:
            __tmp_tuple = ti.expr_init_list((7, 8), 2)
            c = ti.expr_init(__tmp_tuple[0])
            d = ti.expr_init(__tmp_tuple[1])
            del __tmp_tuple
    if 1:
        aa = bb = ti.expr_init(-1)
        aa = bb = ti.expr_init(-1)
    ti.ti_print(a, b, c, d)
    ti.ti_print(aa, bb)
    del bb
    del aa
    del d
    del c
    del b
    del a

Result:

7 8 7 8
-1 -1

@netlify
Copy link

netlify bot commented Sep 30, 2021

✔️ Deploy Preview for jovial-fermat-aa59dc ready!

🔨 Explore the source changes: 8b42b67

🔍 Inspect the deploy log: https://app.netlify.com/sites/jovial-fermat-aa59dc/deploys/615fe7734e0a8b00089a97bd

😎 Browse the preview: https://deploy-preview-3062--jovial-fermat-aa59dc.netlify.app

@gaocegege
Copy link
Collaborator Author

gaocegege commented Sep 30, 2021

/cc @k-ye

Do we have the unit tests or integration tests for the Python code transformation logic?

@k-ye
Copy link
Member

k-ye commented Sep 30, 2021

Yep, they're inside https://github.com/taichi-dev/taichi/tree/master/tests/python. See also https://docs.taichi.graphics/docs/lang/articles/contribution/write_test :-)

@gaocegege
Copy link
Collaborator Author

Yep, they're inside https://github.com/taichi-dev/taichi/tree/master/tests/python. See also https://docs.taichi.graphics/docs/lang/articles/contribution/write_test :-)

Thanks. I will add more test cases for the feature.

@gaocegege gaocegege changed the title [Lang] Support chained assignments WIP [Lang] Support chained assignments Sep 30, 2021
@gaocegege

This comment has been minimized.

@gaocegege gaocegege marked this pull request as draft October 1, 2021 00:22
@gaocegege gaocegege changed the title WIP [Lang] Support chained assignments [Lang] Support chained assignments Oct 1, 2021
@gaocegege gaocegege marked this pull request as ready for review October 1, 2021 09:12
@gaocegege gaocegege closed this Oct 1, 2021
@gaocegege gaocegege reopened this Oct 1, 2021
@gaocegege
Copy link
Collaborator Author

gaocegege commented Oct 1, 2021

I added some test cases (unpack, chained, chained + unpack, assign) for the feature, and the PR is ready to review, I think.

Please take a look, thanks for your time 😄

tests/python/test_assign.py Outdated Show resolved Hide resolved
gaocegege and others added 6 commits October 6, 2021 17:57
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
@gaocegege gaocegege changed the base branch from master to add-mciso-mpm3d October 6, 2021 14:23
@gaocegege gaocegege changed the base branch from add-mciso-mpm3d to master October 6, 2021 14:23
@gaocegege
Copy link
Collaborator Author

Please take a look, thanks for your time 😄

@lin-hitonami
Copy link
Contributor

is_local = isinstance(node_target, ast.Name)
if is_local and ctx.is_creation(node_target.id):
var_name = node_target.id
# Create, no AST resolution needed
init = ast.Attribute(value=ast.Name(id='ti',
ctx=ast.Load()),
attr='expr_init',
ctx=ast.Load())
rhs = ast.Call(
func=init,
args=[node.value],
keywords=[],
)
ctx.create_variable(var_name)
assign_stmts.append(
ast.copy_location(
ast.Assign(targets=node.targets,
value=rhs,
type_comment=None), node))
else:
# Assign
node_target.ctx = ast.Load()
func = ast.Attribute(value=node_target,
attr='assign',
ctx=ast.Load())
call = ast.Call(func=func, args=[node.value], keywords=[])
assign_stmts.append(
ast.copy_location(ast.Expr(value=call), node))

Can we make this part a static method and use it in both tuple and non-tuple cases?

@gaocegege
Copy link
Collaborator Author

is_local = isinstance(node_target, ast.Name)
if is_local and ctx.is_creation(node_target.id):
var_name = node_target.id
# Create, no AST resolution needed
init = ast.Attribute(value=ast.Name(id='ti',
ctx=ast.Load()),
attr='expr_init',
ctx=ast.Load())
rhs = ast.Call(
func=init,
args=[node.value],
keywords=[],
)
ctx.create_variable(var_name)
assign_stmts.append(
ast.copy_location(
ast.Assign(targets=node.targets,
value=rhs,
type_comment=None), node))
else:
# Assign
node_target.ctx = ast.Load()
func = ast.Attribute(value=node_target,
attr='assign',
ctx=ast.Load())
call = ast.Call(func=func, args=[node.value], keywords=[])
assign_stmts.append(
ast.copy_location(ast.Expr(value=call), node))

Can we make this part a static method and use it in both tuple and non-tuple cases?

Do you mean that we do not make tuple case a func call, we just wrap the detailed logic (assign and var creation) into one func, then use it in tuple and non-tuple?

@lin-hitonami
Copy link
Contributor

is_local = isinstance(node_target, ast.Name)
if is_local and ctx.is_creation(node_target.id):
var_name = node_target.id
# Create, no AST resolution needed
init = ast.Attribute(value=ast.Name(id='ti',
ctx=ast.Load()),
attr='expr_init',
ctx=ast.Load())
rhs = ast.Call(
func=init,
args=[node.value],
keywords=[],
)
ctx.create_variable(var_name)
assign_stmts.append(
ast.copy_location(
ast.Assign(targets=node.targets,
value=rhs,
type_comment=None), node))
else:
# Assign
node_target.ctx = ast.Load()
func = ast.Attribute(value=node_target,
attr='assign',
ctx=ast.Load())
call = ast.Call(func=func, args=[node.value], keywords=[])
assign_stmts.append(
ast.copy_location(ast.Expr(value=call), node))

Can we make this part a static method and use it in both tuple and non-tuple cases?

Do you mean that we do not make tuple case a func call, we just wrap the detailed logic (assign and var creation) into one func, then use it in tuple and non-tuple?

I just think the logic of assign and var creation is a bit duplicated. If you think the function is too long, you can also split it into small functions. (just my opinion)

@gaocegege
Copy link
Collaborator Author

Do you mean that we do not make tuple case a func call, we just wrap the detailed logic (assign and var creation) into one func, then use it in tuple and non-tuple?

SGTM. It makes sense.

@gaocegege gaocegege marked this pull request as draft October 8, 2021 06:10
@gaocegege gaocegege marked this pull request as ready for review October 8, 2021 06:48
Copy link
Contributor

@lin-hitonami lin-hitonami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@lin-hitonami lin-hitonami merged commit 43f8746 into taichi-dev:master Oct 8, 2021
@gaocegege gaocegege deleted the stmt branch October 8, 2021 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Supporting chained assignment e.g. a = b = 0 in ti.kernel
3 participants