Skip to content

[FX] Support wrapping builtins with wrap#50184

Closed
jamesr66a wants to merge 11 commits intogh/jamesr66a/346/basefrom
gh/jamesr66a/346/head
Closed

[FX] Support wrapping builtins with wrap#50184
jamesr66a wants to merge 11 commits intogh/jamesr66a/346/basefrom
gh/jamesr66a/346/head

Conversation

@jamesr66a
Copy link
Copy Markdown
Collaborator

@jamesr66a jamesr66a commented Jan 7, 2021

Stack from ghstack:

  • Do some trickery to detect if we're patching a builtin that hasn't been overridden in the global frame of the wrap() caller. In this case, patch the builtin function into the global namespace but add a None sentinel to indicate we should delete the function after we're done.
  • Make wrap() more exception safe

Differential Revision: D25819832

@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Jan 7, 2021

💊 CI failures summary and remediations

As of commit 8898821 (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

This comment has been revised 26 times.

jamesr66a pushed a commit that referenced this pull request Jan 7, 2021
ghstack-source-id: 16e7758
Pull Request resolved: #50184
@jamesr66a jamesr66a requested a review from zdevito January 7, 2021 23:06
jamesr66a pushed a commit that referenced this pull request Jan 8, 2021
ghstack-source-id: 618085c
Pull Request resolved: #50184
jamesr66a pushed a commit that referenced this pull request Jan 9, 2021
ghstack-source-id: 051d0ee
Pull Request resolved: #50184
@jamesr66a jamesr66a changed the title [FX] Make len traceable and scriptable with wrap [FX] Test that len is symtraceable and scriptable with wrap() Jan 12, 2021
Comment thread test/test_fx.py Outdated
def fx_len(item):
return len(item)

wrap(fx_len)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't wrap(len) work?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Builtins don't have __code__:

Traceback (most recent call last):
  File "test/test_fx.py", line 58, in <module>
    wrap(len)
  File "/data/users/jamesreed/pytorch/torch/fx/symbolic_trace.py", line 415, in wrap
    fn_name = fn_or_name.__code__.co_name
AttributeError: 'builtin_function_or_method' object has no attribute '__code__'

@jamesr66a jamesr66a changed the title [FX] Test that len is symtraceable and scriptable with wrap() [FX] Support wrapping builtins with wrap Jan 14, 2021
- Do some trickery to detect if we're patching a builtin that hasn't been overridden in the global frame of the wrap() caller. In this case, patch the builtin function into the global namespace but add a None sentinel to indicate we should delete the function after we're done.
- Make wrap() more exception safe

Differential Revision: [D25819832](https://our.internmc.facebook.com/intern/diff/D25819832)

[ghstack-poisoned]
- Do some trickery to detect if we're patching a builtin that hasn't been overridden in the global frame of the wrap() caller. In this case, patch the builtin function into the global namespace but add a None sentinel to indicate we should delete the function after we're done.
- Make wrap() more exception safe

Differential Revision: [D25819832](https://our.internmc.facebook.com/intern/diff/D25819832)

[ghstack-poisoned]
jamesr66a pushed a commit that referenced this pull request Jan 15, 2021
ghstack-source-id: 71e37d4
Pull Request resolved: #50184
- Do some trickery to detect if we're patching a builtin that hasn't been overridden in the global frame of the wrap() caller. In this case, patch the builtin function into the global namespace but add a None sentinel to indicate we should delete the function after we're done.
- Make wrap() more exception safe

Differential Revision: [D25819832](https://our.internmc.facebook.com/intern/diff/D25819832)

[ghstack-poisoned]
jamesr66a pushed a commit that referenced this pull request Jan 15, 2021
ghstack-source-id: 0789140
Pull Request resolved: #50184
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@jamesr66a merged this pull request in 0291f35.

@facebook-github-bot facebook-github-bot deleted the gh/jamesr66a/346/head branch January 19, 2021 15:16
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
Summary: Pull Request resolved: pytorch#50184

Test Plan: Imported from OSS

Reviewed By: bertmaher

Differential Revision: D25819832

Pulled By: jamesr66a

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants