[JIT] Store schema in serialized modules and check arguments in function call#10872
[JIT] Store schema in serialized modules and check arguments in function call#10872goldsborough wants to merge 2 commits intopytorch:masterfrom
Conversation
c79774b to
cca3f1c
Compare
zdevito
left a comment
There was a problem hiding this comment.
Looks good. There are two small fixes.
torch/csrc/jit/script/module.h
Outdated
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
torch/csrc/jit/export.cpp
Outdated
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
cca3f1c to
a22729a
Compare
facebook-github-bot
left a comment
There was a problem hiding this comment.
goldsborough has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
a22729a to
edac01c
Compare
Add test for missing argument Always store schema Add default arguments to method input and warn against default arguments on export
edac01c to
526175a
Compare
facebook-github-bot
left a comment
There was a problem hiding this comment.
goldsborough is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
| } | ||
| types.push_back(std::move(type)); | ||
| } | ||
| void parseArgumentType(Argument& arg) { |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
facebook-github-bot
left a comment
There was a problem hiding this comment.
goldsborough is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
| template<typename ... T> | ||
| static TupleTypePtr create( T&& ... all ) { | ||
| return TupleTypePtr(new TupleType( std::forward<T>(all)... )); | ||
| static TupleTypePtr create(std::vector<TypePtr> types) { |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
…ll (pytorch#10872) Summary: This PR adds argument checking for script method invocation from C++. For this I had to: 1. The schema of a method is currently not serialized in script modules, so we now store the function schema in the `doc_string` field of the ONNX proto. Upon loading of a serialized script module, we parse the schema into the structured C++ form and assign it to the loaded method, 2. Inside `Method::operator()`, we now verify the number and types of arguments. CC The controller you requested could not be found. zdevito Pull Request resolved: pytorch#10872 Differential Revision: D9521219 Pulled By: goldsborough fbshipit-source-id: 5cb3d710af6f500e7579dad176652c9b11a0487d
This PR adds argument checking for script method invocation from C++. For this I had to:
doc_stringfield of the ONNX proto. Upon loading of a serialized script module, we parse the schema into the structured C++ form and assign it to the loaded method,Method::operator(), we now verify the number and types of arguments.CC @li-roy
@zdevito