[JIT] Add registerOperator overloads that infer the schema by goldsborough · Pull Request #10048 · pytorch/pytorch

goldsborough · 2018-07-31T02:32:30Z

This PR adds a way to infer the JIT/script schema of a function from its signature, and then create an operator from the schema and implementation. The implementation function is wrapped into another function, which pops values from the stack into an argument tuple, then invokes the function and pushes the return value back onto the stack, sometimes unpacking the return value if it is a tuple.

Currently the method is called createOperator. We may want to think of a nicer way of registering ops in tandem with RegisterOperators. It might be very cumbersome to add a template constructor to Operator, so maybe we can come up with a chaining method on RegisterOperators like RegisterOperators(schema, func).op(schema.func).op(schema, func) -- it has to work at startup time (for a static variable) though. We can solve this in another PR.

@zdevito @apaszke @smessmer @dzhulgakov

torch/csrc/jit/custom_operator.h

goldsborough · 2018-08-01T17:10:59Z

@zdevito can you take another look?
@smessmer can you approve the changes to your Metaprogramming.h?

zdevito

Looks good! Minor comments. Template meta-programming is just on the border of being understandable, which is nice.

torch/csrc/jit/custom_operator.h

torch/csrc/jit/test_jit.cpp

facebook-github-bot

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

goldsborough · 2018-08-02T01:06:24Z

@zdevito check the latest commit (Better support for lists)

smessmer

LGTM

caffe2/utils/TypeList.h

caffe2/utils/Metaprogramming.h

torch/csrc/jit/custom_operator.h

caffe2/utils/Metaprogramming.h

goldsborough · 2018-08-02T21:04:45Z

@pytorchbot retest this please

goldsborough · 2018-08-03T00:28:03Z

@pytorchbot retest this please

facebook-github-bot

goldsborough is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot

zdevito is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Summary: This PR adds a way to infer the JIT/script schema of a function from its signature, and then create an operator from the schema and implementation. The implementation function is wrapped into another function, which pops values from the stack into an argument tuple, then invokes the function and pushes the return value back onto the stack, sometimes unpacking the return value if it is a tuple. Currently the method is called `createOperator`. We may want to think of a nicer way of registering ops in tandem with `RegisterOperators`. It might be very cumbersome to add a template constructor to `Operator`, so maybe we can come up with a chaining method on `RegisterOperators` like `RegisterOperators(schema, func).op(schema.func).op(schema, func)` -- it has to work at startup time (for a static variable) though. We can solve this in another PR. zdevito apaszke smessmer dzhulgakov Pull Request resolved: pytorch#10048 Differential Revision: D9125975 Pulled By: goldsborough fbshipit-source-id: de9e59888757573284a43787ae5d94384bfe8f9a

goldsborough requested review from apaszke, colesbury, ezyang, gchanan, soumith and zdevito as code owners July 31, 2018 02:32

goldsborough force-pushed the register-op branch from f2eea8d to 009c4ea Compare July 31, 2018 02:33

zdevito reviewed Jul 31, 2018

View reviewed changes

torch/csrc/jit/custom_operator.h Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

goldsborough added the oncall: jit Add this issue/PR to JIT oncall triage queue label Jul 31, 2018

goldsborough force-pushed the register-op branch from 009c4ea to 5bba7ba Compare August 1, 2018 01:09

goldsborough changed the title ~~[WIP][JIT] Add registerOperator overloads that infer the schema~~ [JIT] Add registerOperator overloads that infer the schema Aug 1, 2018

goldsborough force-pushed the register-op branch 2 times, most recently from c8920a2 to 85a0270 Compare August 1, 2018 17:10

zdevito approved these changes Aug 1, 2018

View reviewed changes

torch/csrc/jit/custom_operator.h Outdated

This comment was marked as off-topic.

Sign in to view

torch/csrc/jit/custom_operator.h Outdated

This comment was marked as off-topic.

Sign in to view

torch/csrc/jit/test_jit.cpp Outdated

This comment was marked as off-topic.

Sign in to view

facebook-github-bot reviewed Aug 1, 2018

View reviewed changes

goldsborough force-pushed the register-op branch from 85a0270 to 15746cc Compare August 2, 2018 01:05

smessmer reviewed Aug 2, 2018

View reviewed changes

caffe2/utils/Metaprogramming.h Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

goldsborough force-pushed the register-op branch 2 times, most recently from 6985525 to e230aeb Compare August 2, 2018 21:10

goldsborough added 7 commits August 2, 2018 16:24

registerOperator

2abc4bc

Infer schema for custom operators

5b6980f

registerOperator -> createOperator

4348ca0

Merge with-schema and schema-less createOperator

4d95b44

Test execution of ops

8f391ee

Rebase

25354a4

Better support for lists

e6bac75

goldsborough added 2 commits August 2, 2018 16:24

Fix Metaprogramming stuff a little

2c060e3

Fix list tests

0f99f49

goldsborough force-pushed the register-op branch from e230aeb to 0f99f49 Compare August 3, 2018 00:00

facebook-github-bot reviewed Aug 3, 2018

View reviewed changes

goldsborough mentioned this pull request Aug 3, 2018

[JIT] Add tracing to custom op and simplify tracer overall #10212

Closed

facebook-github-bot closed this in cb0e72e Aug 3, 2018

goldsborough deleted the register-op branch August 3, 2018 18:51

ezyang added the merged label Jun 26, 2019

Conversation

goldsborough commented Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

goldsborough commented Aug 1, 2018

Uh oh!

zdevito left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

goldsborough commented Aug 2, 2018

Uh oh!

smessmer left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

goldsborough commented Aug 2, 2018

Uh oh!

goldsborough commented Aug 3, 2018

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

goldsborough commented Jul 31, 2018 •

edited

Loading