Skip to content

Conversation

@zhhsplendid
Copy link
Member

@zhhsplendid zhhsplendid commented Mar 23, 2021

PR types

Bug fixes

PR changes

APIs

Describe

Our old loop_body function may return single element when loop_vars just contains only 1 element, which can cause bug. Let's see an example, the origin code is the new testcase TestForwardContainsForLayer of this PR. The translated code would be like below before this PR

def forward(self, x):
    y = paddle.zeros([10, 2, 3])
    z = []
    i = 0

    def for_loop_condition_0(i):
        return i < self.high - self.low

    def for_loop_body_0(i):
        paddle.jit.dy2static.convert_call(z.append)(paddle.jit.dy2static.
            convert_call(y[i].clone)())
        i += 1
        return i # This PR changes `return i` to `return i,` as a tuple

    [i] = paddle.jit.dy2static.convert_while_loop(for_loop_condition_0,
        for_loop_body_0, [i])
    return z

You could notice the for_loop_body_0 returns i, then when the python for loop is running, the loop_vars [i] would be updated to i, which can cause bug.

The key point of this PR is forcing loop_body functions always return tuple.

@paddle-bot-old
Copy link

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

Copy link
Contributor

@liym27 liym27 left a comment

Choose a reason for hiding this comment

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

LGTM

@zhhsplendid zhhsplendid merged commit 649868f into PaddlePaddle:develop Mar 24, 2021
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