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

[Dy2Stat]Polish for zip in dy2stat #37846

Merged
merged 5 commits into from
Dec 7, 2021

Conversation

0x45f
Copy link
Contributor

@0x45f 0x45f commented Dec 3, 2021

PR types

Others

PR changes

Others

Describe

polish for zip in dy2stat

@paddle-bot-old
Copy link

paddle-bot-old bot commented Dec 3, 2021

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

"""
assert isinstance(node, gast.Call)
if is_paddle_api(node):
return True

func_str = ast_to_source_code(node.func).strip()
try:
from paddle.fluid.dygraph.dygraph_to_static.convert_call_func import is_builtin_len, is_builtin
from paddle.fluid.dygraph.dygraph_to_static.convert_call_func import is_builtin_len, is_builtin, is_builtin_zip
Copy link
Contributor

Choose a reason for hiding this comment

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

We should avoid Runtime import, please put it ahead of the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这里应该会有循环import的问题,放在前面会报错

def is_builtin_zip(func):
if is_builtin(func) and func.__name__ == 'zip':
return True
return False
Copy link
Contributor

Choose a reason for hiding this comment

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

return is_builtin(func) and func.__name__ == 'zip'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

if isinstance(arg, Variable) and arg.shape[0] == -1:
raise ValueError(
"If the parameter is Variable in 'zip', its first dimension should not be -1. "
"But args[{}].shape[0] == -1 in 'zip'".format(str(i)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"But args[{}].shape[0] == -1 in 'zip'".format(str(i)))
Not support zip(tensor, ...) when tensor.shape[0] == -1, but found args[{}].shape[0] == -1 in 'zip'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

def convert_zip(*args):
for i, arg in enumerate(args):
if isinstance(arg, Variable) and arg.shape[0] == -1:
raise ValueError(
Copy link
Contributor

Choose a reason for hiding this comment

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

raise RuntimeError

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@Aurelius84 Aurelius84 merged commit 4e63d69 into PaddlePaddle:develop Dec 7, 2021
@0x45f 0x45f deleted the dy2stat_for_zip branch December 7, 2021 06:56
0x45f added a commit to 0x45f/Paddle that referenced this pull request Dec 7, 2021
* polish for zip in dy2stat

* polish comment

* polish is_builtin_len

* fix comment
lanxianghit pushed a commit that referenced this pull request Dec 9, 2021
Zjq9409 pushed a commit to Zjq9409/Paddle that referenced this pull request Dec 10, 2021
* polish for zip in dy2stat

* polish comment

* polish is_builtin_len

* fix comment
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.

2 participants