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

Use "dynamo" to rewrite int(...), float(...), and bool(...) #62

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

makslevental
Copy link
Collaborator

@makslevental makslevental commented Jul 5, 2023

This PR uses https://github.com/makslevental/pyframe_eval to implement our own version of dynamo and support int(...), float(...), and bool(...), i.e., a custom stackframe evaluator that "just-in-time" rewrites the ast such that

class MyIntModule(nn.Module):
    def forward(self, x):
        y = int(x)
        return y

becomes (roughly)

class MyIntModule(nn.Module):
    def forward(self, x):
        y = pi.pi_int(x)
        return y

which then gets lowered to

func.func @forward(%arg0: !torch.vtensor<[1],si64>) -> !torch.int {
  %0 = torch.aten.Int.Tensor %arg0 : !torch.vtensor<[1],si64> -> !torch.int
  return %0 : !torch.int
}

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:

total # of tests: 930
total # of skips: 18
total # of passes: 741
total # of xfails: 16
total # of exceptions: 90
total # of lower to linalg failures: 25
total # of ir differences: 40

cc @powderluv

Comment on lines 46 to 65
# 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,
# )
Copy link
Collaborator Author

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

Comment on lines 19 to 33
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)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is the key

requirements.txt Outdated

-f https://github.com/makslevental/pyframe_eval/releases/expanded_assets/0.0.1
pyframe_eval

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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...)

@makslevental
Copy link
Collaborator Author

@123epsilon this isn't done yet...

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.

3 participants