-
Notifications
You must be signed in to change notification settings - Fork 4
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
Use "dynamo" to rewrite int(...)
, float(...)
, and bool(...)
#62
base: main
Are you sure you want to change the base?
Conversation
pi/utils/ast_rewrite.py
Outdated
# f_code_o = f_code_o.replace( | ||
# co_argcount=fun.__code__.co_argcount, | ||
# co_posonlyargcount=fun.__code__.co_posonlyargcount, | ||
# co_kwonlyargcount=fun.__code__.co_kwonlyargcount, | ||
# co_nlocals=fun.__code__.co_nlocals, | ||
# co_stacksize=fun.__code__.co_stacksize, | ||
# co_flags=fun.__code__.co_flags, | ||
# co_firstlineno=fun.__code__.co_firstlineno, | ||
# # co_code=fun.__code__.co_code, | ||
# co_consts=fun.__code__.co_consts, | ||
# co_names=fun.__code__.co_names, | ||
# co_varnames=fun.__code__.co_varnames, | ||
# co_freevars=fun.__code__.co_freevars, | ||
# co_cellvars=fun.__code__.co_cellvars, | ||
# co_filename=fun.__code__.co_filename, | ||
# co_name=fun.__code__.co_name, | ||
# co_qualname=fun.__code__.co_qualname, | ||
# co_linetable=fun.__code__.co_linetable, | ||
# co_exceptiontable=fun.__code__.co_exceptiontable, | ||
# ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this still has to be figured out
pi/utils/ast_rewrite.py
Outdated
fun = frame.f_func | ||
try: | ||
src = dedent(inspect.getsource(fun)) | ||
except Exception as e: | ||
warnings.warn(f"couldn't parse {fun.__name__} because {e}") | ||
return fun.__code__ | ||
|
||
src = src.replace("torch.", "pi.") | ||
tree = ast.parse(src) | ||
|
||
assert isinstance( | ||
tree.body[0], ast.FunctionDef | ||
), f"unexpected ast node {tree.body[0]}" | ||
old_tree_dump = ast.dump(tree) | ||
tree = PiIntFloatBool().visit(tree) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the key
de78151
to
510d2a7
Compare
510d2a7
to
6b66623
Compare
requirements.txt
Outdated
|
||
-f https://github.com/makslevental/pyframe_eval/releases/expanded_assets/0.0.1 | ||
pyframe_eval |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we copy a version of it into PI ? Also you probably want to set the license for that. Ideally Apache 2.0 is best for PI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you/we/I can copy but do you really want it here? It's hairy cpython internals - might as well pawn maintenance off on me into perpetuity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay I licensed it WTFPL (not specifically for you but you know just for everyone...)
@123epsilon this isn't done yet... |
This PR uses https://github.com/makslevental/pyframe_eval to implement our own version of dynamo and support
int(...)
,float(...)
, andbool(...)
, i.e., a custom stackframe evaluator that "just-in-time" rewrites the ast such thatbecomes (roughly)
which then gets lowered to
Why go through all of this trouble when a decorator would accomplish the same thing? Because this will handle all follow-on calls from within
forward
too (e.g., some helper that we can't decorate).Just to be very clear: there's still no PyTorch in the mix here, I simply implemented the same thing that they did (and packaged it seperately in
pyframe_eval
) and instead of custom interpreting the bytecode of the stackframe, I made it rewrite the ast of the stackframe.This and (
pyframe_eval
) is still not fully tested (so it might segfault or assert inside of cpython) but it basically works:cc @powderluv