[nativert] Move Graph to pytorch core#153994
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/153994
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New Failure, 1 Unrelated FailureAs of commit 11332f6 with merge base 6706751 ( NEW FAILURE - The following job has failed:
UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
This pull request was exported from Phabricator. Differential Revision: D74749418 |
|
This pull request was exported from Phabricator. Differential Revision: D74749418 |
f942ad3 to
dda18ea
Compare
|
This pull request was exported from Phabricator. Differential Revision: D74749418 |
dda18ea to
20a13bf
Compare
|
This pull request was exported from Phabricator. Differential Revision: D74749418 |
20a13bf to
4530aee
Compare
Skylion007
left a comment
There was a problem hiding this comment.
lots of performance nits here
torch/nativert/graph/Graph.cpp
Outdated
There was a problem hiding this comment.
Beware of signed char, most of these helpers only take unsigned char
|
This pull request was exported from Phabricator. Differential Revision: D74749418 |
4530aee to
96ab988
Compare
|
This pull request was exported from Phabricator. Differential Revision: D74749418 |
96ab988 to
c8e1190
Compare
|
This pull request was exported from Phabricator. Differential Revision: D74749418 |
c8e1190 to
5c59c57
Compare
|
This pull request was exported from Phabricator. Differential Revision: D74749418 |
5c59c57 to
2a471d2
Compare
99f557c to
bad9464
Compare
|
This pull request was exported from Phabricator. Differential Revision: D74749418 |
bad9464 to
889dc4b
Compare
|
This pull request was exported from Phabricator. Differential Revision: D74749418 |
889dc4b to
8301cbb
Compare
|
This pull request was exported from Phabricator. Differential Revision: D74749418 |
8301cbb to
e0cf10b
Compare
@swolchok Thank you for all the comments and suggestions! At this point, our primary focus and top priority is on getting this open-sourced, and there are other ongoing efforts to improve efficiency in other parts of the runtime. |
|
This pull request was exported from Phabricator. Differential Revision: D74749418 |
e0cf10b to
dc94dc8
Compare
|
This pull request was exported from Phabricator. Differential Revision: D74749418 |
dc94dc8 to
8cba4f9
Compare
Summary: Pull Request resolved: pytorch#153994 Move Graph to pytorch core. Test Plan: buck2 test mode/opt caffe2/test/cpp/nativert:graph_test Differential Revision: D74749418
|
This pull request was exported from Phabricator. Differential Revision: D74749418 |
8cba4f9 to
11332f6
Compare
| explicit Type(Kind kind, const std::string& classFqn) | ||
| : kind_(CustomObjData{classFqn}) { | ||
| TORCH_CHECK(kind == Kind::CustomObj); |
There was a problem hiding this comment.
Not sure why you need to pass kind argument if it's unsued
| explicit Type(Kind kind, const std::string& classFqn) | |
| : kind_(CustomObjData{classFqn}) { | |
| TORCH_CHECK(kind == Kind::CustomObj); | |
| explicit Type(const std::string& classFqn) | |
| : kind_(CustomObjData{classFqn}) { |
There was a problem hiding this comment.
@malfet Here we want to increase the readability when people calling the constructor of Type explicitly to ensure only CustomObj can be passed in.
|
splitting this PR into smaller ones. see stack #154532 |
do you need to close this one then? |
Summary:
Torch Native Runtime RFC: pytorch/rfcs#72
4 classes have been introduced:
Graph,Node,Value,TypeTyperepresents the kind of aValueValuerepresents a single symbolic value, it could be any kind that exists inType. Values are inputs and outputs of aNode.Noderepresents a single unit of execution, typically a PyTorch op.Graphrepresents a model's computation graph, which is designed to facilitate transformation/analysis.Test Plan: Added test under
test/cpp/nativert/test_graph.cppDifferential Revision: D74749418