Skip to content

Conversation

@eellison
Copy link
Contributor

@eellison eellison commented Nov 14, 2019

Fix for #21545

We we were silently giving wrong semantics previously:

Python behavior:

def test(x=[]):
   x.append(1)
   return len(x)

print(test()) # 1
print(test()) # 2

By checking at the python layer, we prevent any new models from serializing this behavior but do not break existing serialized models.

@eellison eellison requested a review from driazati November 14, 2019 20:50
@eellison eellison requested a review from apaszke as a code owner November 14, 2019 20:50
@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Nov 14, 2019
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@eellison has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@eellison has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@eellison merged this pull request in 902c1f9.

csarofeen pushed a commit to mruberry/pytorch that referenced this pull request Nov 18, 2019
Summary:
Fix for pytorch#21545

We we were silently giving wrong semantics previously:

Python behavior:
```
def test(x=[]):
   x.append(1)
   return len(x)

print(test()) # 1
print(test()) # 2
```

By checking at the python layer, we prevent any new models from serializing this behavior but do not break existing serialized models.
Pull Request resolved: pytorch#29833

Differential Revision: D18513168

Pulled By: eellison

fbshipit-source-id: 6fe73f28e1f9d39dedeaf67a04718089d14401a1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants