-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Build] Refactor backends to adhere to stricter build warnings #442
Conversation
d07374b
to
ad04349
Compare
Codecov Report
@@ Coverage Diff @@
## master #442 +/- ##
=======================================
Coverage 88.23% 88.23%
=======================================
Files 101 101
Lines 6356 6356
=======================================
Hits 5608 5608
Misses 748 748
Continue to review full report at Codecov.
|
{ | ||
return std::dynamic_pointer_cast<NativeDataContainer<torch::jit::IValue>>(tensor)->get_native_data(); | ||
} | ||
torch::jit::IValue get_ivalue_from_torch_tensor(const std::shared_ptr<NeuropodValue> &tensor); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could make it "inline", this is legal way to provide definition in header w/o confusing compiler. But I see you added .cc file that is also fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup! Previously, I marked it as inline
in an internal patch on top of Neuropod, but decided to pull it into a .cc
file here.
I was planning on pulling more things into the .cc
file, but taking another look, it probably makes sense to just inline
this in the header and leave it alone
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
ad04349
to
39a9c4c
Compare
Summary:
Refactor code under
source/neuropod/backends
to adhere to the stricter set of warnings introduced in #437Test Plan:
CI