-
Notifications
You must be signed in to change notification settings - Fork 372
Pytorch 1.7 update API changes #225
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
Conversation
06972cd
to
d848676
Compare
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.
Code conforms to C++ style guidelines
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.
Code conforms to Python style guidelines
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.
Code conforms to C++ style guidelines
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.
Code conforms to Python style guidelines
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.
Code conforms to Python style guidelines
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.
Code conforms to C++ style guidelines
d848676
to
3ecf5d0
Compare
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.
Code conforms to Python style guidelines
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.
Code conforms to C++ style guidelines
@@ -11,7 +11,7 @@ GraphParams get_named_params(c10::ArrayRef<torch::jit::Value*> inputs, std::vect | |||
GraphParams named_params; | |||
auto param_it = params.begin(); | |||
for (auto in : inputs) { | |||
if (in->type() != c10::TensorType::get() && in->isCompleteTensor() && param_it != params.end()) { | |||
if (in->type() != c10::TensorType::get() && param_it != params.end()) { |
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.
Did this end up being fixed?
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.
Facing this error
Unexpected specifier 'strides':
Looks like the strides specifier did not make it into 1.7 branch. General branch stride support, 1.7 branch code comparison
From my understanding, the isCompleteTensor() doesn't seem to be absolutely necessary especially when we pass in constants to the graph assuming they have static shapes. The testcase usages don't absolutely need it.
Can you let me know if you have any other necessary usecases in mind where this would help ?
This definitely adds a extra layer of check which is good. I tried looking into pytorch 1.7 source and building it to understand why isCompleteTensor
function changed, however that was blocked by some unrelated issues.
https://github.com/NVIDIA/TRTorch/blob/master/README.md#dependencies Can you update the dependency info here |
Can you bump up to the latest bazel version? https://github.com/NVIDIA/TRTorch/blob/master/.bazelversion |
https://github.com/NVIDIA/TRTorch/blob/pytorch_update/tests/py/requirements.txt bump to latest torchvision |
https://github.com/NVIDIA/TRTorch/blob/pytorch_update/py/requirements.txt bump to latest torch |
3ecf5d0
to
217413e
Compare
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.
Code conforms to Python style guidelines
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.
Code conforms to C++ style guidelines
217413e
to
8bee7b8
Compare
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.
Code conforms to C++ style guidelines
@peri044 was there anything outstanding on your side or is this ready for a final review and merge? |
Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>
Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>
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.
Code conforms to C++ style guidelines
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.
Code conforms to Python style guidelines
Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>
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.
Code conforms to Python style guidelines
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.
Code conforms to C++ style guidelines
Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>
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.
Code conforms to C++ style guidelines
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.
Code conforms to Python style guidelines
WORKSPACE
Outdated
build_file = "@//third_party/tensorrt/archive:BUILD", | ||
sha256 = "c7d73b2585b18aae68b740249efa8c8ba5ae852abe9a023720595432a8eb4efd", | ||
strip_prefix = "TensorRT-7.0.0.11" | ||
sha256 = "818622c7bccfdb0fff362cdd65dc42af4502dcb24e045eb911a4132e8d40a66c", |
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.
I think this sha is incorrect. I have been getting this 8def6b03b0c8c3751f560df21b3e99668ae05aab5140b1d38b8e51e4a0ffbbb8
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.
thanks. Fixed it
Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>
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.
Code conforms to Python style guidelines
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.
Code conforms to C++ style guidelines
README.md
Outdated
- cuDNN 7.6.5 (by default, cuDNN 8 supported with compatable PyTorch build) | ||
- TensorRT 7.0.0 (by default, TensorRT 7.1 supported with compatable PyTorch build) | ||
- Bazel 3.7.0 | ||
- Libtorch 1.7.0 (built with CUDA 11.0) |
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.
There is now a PyTorch 1.7.1, so lets bump to that.
WORKSPACE
Outdated
urls = ["https://download.pytorch.org/libtorch/cu102/libtorch-cxx11-abi-shared-with-deps-1.6.0.zip"], | ||
sha256 = "fded948bd2dbee625cee33ebbd4843a69496729389e0200a90fbb667cdaeeb69" | ||
sha256 = "656db919c00e99dac81bc21598845ba9231e57cbc8e1570bea017dfb9298236d", | ||
urls = ["https://download.pytorch.org/libtorch/cu110/libtorch-cxx11-abi-shared-with-deps-1.7.0%2Bcu110.zip"], |
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.
Lets move to pytorch 1.7.1
py/setup.py
Outdated
@@ -204,7 +204,7 @@ def run(self): | |||
long_description=long_description, | |||
ext_modules=ext_modules, | |||
install_requires=[ | |||
'torch==1.6.0', | |||
'torch==1.7.0', |
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.
Torch 1.7.1
tests/py/requirements.txt
Outdated
torchvision==0.8.0 |
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.
torchvision 0.8.2
@narendasan Updated to pytorch 1.7.1 and torchvision 0.8.2 |
Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>
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.
Code conforms to Python style guidelines
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.
Code conforms to C++ style guidelines
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.
Awesome LGTM!
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.
Code conforms to C++ style guidelines
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.
Code conforms to Python style guidelines
Description
Changes related to Pytorch 1.7 update
Fixes (#207)
Type of change
Checklist: