Skip to content

Conversation

@driazati
Copy link
Contributor

@driazati driazati commented May 10, 2019

Fixes #20017

This wraps the torch._C.Function currently returned from torch.jit.script and torch.jit.trace in a ScriptFunction and TracedFunction respectively, both of which are just wrappers to hold the function.

Differential Revision: D15403161

davidriazati added 10 commits May 6, 2019 16:50
@pytorchbot pytorchbot added the oncall: jit Add this issue/PR to JIT oncall triage queue label May 10, 2019
@pytorchbot pytorchbot added the module: pybind Related to our Python bindings / interactions with other Python libraries label May 13, 2019
@driazati driazati changed the title [jit] Add TopLevelTracedFunction wrapper [jit] Add ScriptFunction wrapper May 13, 2019
@driazati driazati requested review from zdevito and removed request for zdevito May 13, 2019 21:37
@driazati driazati changed the title [jit] Add ScriptFunction wrapper [jit] Add save() to torch._C.Function May 14, 2019
@driazati driazati changed the title [jit] Add save() to torch._C.Function [wip] [jit] Add save() to torch._C.Function May 14, 2019
@driazati driazati requested a review from zdevito May 17, 2019 23:53
@driazati driazati changed the title [wip] [jit] Add save() to torch._C.Function [jit] Add save() to torch._C.Function May 17, 2019
Copy link
Contributor

@zdevito zdevito left a comment

Choose a reason for hiding this comment

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

Looks good, minor comments.

Module module;
// Make a graph with a fake self argument
auto graph = self->graph()->copy();
graph->insertInput(0, "self");
Copy link
Contributor

Choose a reason for hiding this comment

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

The type of input 0 is not set here (it is also never checked, which is why it doesn't fail).

const ExtraFilesMap& _extra_files = ExtraFilesMap()) {
std::ostringstream buf;
Module module;
auto graph = self->graph()->copy();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we put the implementation in Function::save, and avoid duplicating the logic between save and save_to_buffer?

@facebook-github-bot
Copy link
Contributor

@driazati merged this pull request in 410c721.

@apaszke
Copy link
Contributor

apaszke commented May 21, 2019

Does load(save(f)) give you back a function (as it should) or a module (as it seems to do from what I see)?

@driazati
Copy link
Contributor Author

It does return a module for now, our serialization doesn't know about functions afaik and we wanted to at least get the save/load functionality back before fixing that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged module: pybind Related to our Python bindings / interactions with other Python libraries oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

torch.jit.trace returns unwrapped C type

7 participants