Skip to content

[jit] Add resolver for 'torch' module#10847

Closed
driazati wants to merge 4 commits intopytorch:masterfrom
driazati:cpp_script
Closed

[jit] Add resolver for 'torch' module#10847
driazati wants to merge 4 commits intopytorch:masterfrom
driazati:cpp_script

Conversation

@driazati
Copy link
Contributor

@driazati driazati commented Aug 24, 2018 •
edited
Loading

This lets you compile builtin functions from C++ without having a dependence on Python

auto module = torch::jit::compile(JIT"(
def my_script_method(x, y):
    return torch.relu(x) + y
)");
IValue result = module->run_method("my_script_method", 1, 2);

@goldsborough @zdevito @apaszke

@zou3519 zou3519 added the oncall: jit Add this issue/PR to JIT oncall triage queue label Aug 28, 2018
facebook-github-bot
facebook-github-bot reviewed Aug 28, 2018
View reviewed changes
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.

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

goldsborough
goldsborough suggested changes Aug 28, 2018
View reviewed changes
Copy link
Contributor

@goldsborough goldsborough left a comment

Choose a reason for hiding this comment

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

The implementation of compileFunction looks great, but it needs more documentation and possibly some usability improvements for running the function. We want to expose this method to public C++ users so it should be easy to use

torch/csrc/jit/script/compiler.cpp Outdated

This comment was marked as off-topic.

Sign in to view

torch/csrc/jit/script/compiler.h Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

zdevito
zdevito reviewed Aug 29, 2018
View reviewed changes
torch/csrc/jit/script/compiler.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

@driazati driazati requested a review from ebetica as a code owner August 29, 2018 20:15
@driazati
Copy link
Contributor Author

driazati commented Aug 29, 2018

The latest commit pulls the functionality out of torch/jit and into the C++ API with compile and run functions.

driazati
driazati commented Aug 29, 2018
View reviewed changes
torch/csrc/api/include/torch/jit.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
goldsborough reviewed Aug 29, 2018
View reviewed changes
torch/csrc/api/include/torch/jit.h Outdated

This comment was marked as off-topic.

Sign in to view

torch/csrc/api/include/torch/jit.h Outdated

This comment was marked as off-topic.

Sign in to view

torch/csrc/api/include/torch/jit.h Outdated

This comment was marked as off-topic.

Sign in to view

torch/csrc/api/include/torch/jit.h Outdated

This comment was marked as off-topic.

Sign in to view

torch/csrc/jit/script/compiler.h Outdated

This comment was marked as off-topic.

Sign in to view

torch/csrc/api/include/torch/jit.h Outdated

This comment was marked as off-topic.

Sign in to view

test/cpp/api/jit.cpp Outdated

This comment was marked as off-topic.

Sign in to view

zdevito
zdevito reviewed Aug 31, 2018
View reviewed changes
torch/csrc/api/include/torch/jit.h Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

@driazati
Copy link
Contributor Author

driazati commented Sep 3, 2018

@pytorchbot retest this please

goldsborough
goldsborough approved these changes Sep 5, 2018
View reviewed changes
Copy link
Contributor

@goldsborough goldsborough left a comment

Choose a reason for hiding this comment

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

I made some small suggestions that improve legibility/correctness, but they're nits. You can address them if you want, otherwise feel free to land already

torch/csrc/api/src/jit.cpp Outdated

This comment was marked as off-topic.

Sign in to view

torch/csrc/api/include/torch/jit.h Outdated

This comment was marked as off-topic.

Sign in to view

torch/csrc/jit/script/compiler.h Outdated

This comment was marked as off-topic.

Sign in to view

torch/csrc/jit/script/compiler.h Outdated

This comment was marked as off-topic.

Sign in to view

facebook-github-bot
facebook-github-bot reviewed Sep 5, 2018
View reviewed changes
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.

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

David Riazati added 4 commits September 5, 2018 16:45
[jit] Add resolver for 'torch' module
ad70f5a 
Summary:
This lets you use builtin functions from C++ without having a dependence
on Python

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
Add JIT to C++ API
a9eee03 
Summary:
* Adds jit.h and jit.cpp to csrc/api
    * Adds torch::jit::compile and torch::jit::run methods to expose
script compiler and interpreter to C++ users
* Add test_api test for JIT API

Test Plan:
`./build/bin/test_api "torch script"`

Reviewers:

Subscribers:

Tasks:

Tags:
Change to universal references
1200f7b 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
Address nits
5f1ad65 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
facebook-github-bot
facebook-github-bot reviewed Sep 6, 2018
View reviewed changes
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.

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

@jekbradbury jekbradbury mentioned this pull request Sep 8, 2018
Closed
PenghuiCheng pushed a commit to PenghuiCheng/pytorch that referenced this pull request Sep 11, 2018
@PenghuiCheng
Add resolver for 'torch' module (pytorch#10847)
de4215d 
Summary:
This lets you compile builtin functions from C++ without having a dependence on Python

```cpp
auto module = torch::jit::compile(JIT"(
def my_script_method(x, y):
    return torch.relu(x) + y
)");
IValue result = module->run_method("my_script_method", 1, 2);
```

goldsborough zdevito apaszke
Pull Request resolved: pytorch#10847

Differential Revision: D9543461

Pulled By: driazati

fbshipit-source-id: 6160dae094030ca144a0df93cb9f26aa78c8cf27
@ezyang ezyang added the merged label Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zdevito zdevito zdevito left review comments

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@colesbury colesbury Awaiting requested review from colesbury

@ezyang ezyang Awaiting requested review from ezyang

@gchanan gchanan Awaiting requested review from gchanan

@soumith soumith Awaiting requested review from soumith

@ebetica ebetica Awaiting requested review from ebetica

+2 more reviewers

@apaszke apaszke apaszke approved these changes

@goldsborough goldsborough goldsborough approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@driazati @zdevito @apaszke @facebook-github-bot @goldsborough @ezyang @zou3519