8000 [torch::deploy] remove asserts from deploy by PaliC · Pull Request #73456 · pytorch/pytorch · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[torch::deploy] remove asserts from deploy #73456

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

Closed
wants to merge 12 commits into from

Conversation

PaliC
Copy link
Contributor
@PaliC PaliC commented Feb 25, 2022

Stack from ghstack (oldest at bottom):

Replaces at::Error with a more simple implementation of exceptions in order to reduce the dependency of torch::deploy on torch.

Note: Internal Testing / changes are still needed

Differential Revision: D34868005

@pytorch-bot
Copy link
pytorch-bot bot commented Feb 25, 2022
CI Flow Status

⚛️ CI Flow

Ruleset - Version: v1
Ruleset - File: https://github.com/pytorch/pytorch/blob/ea0297cb2d1d9f681bcecc3659fdb722985af684/.github/generated-ciflow-ruleset.json
PR ciflow labels: ciflow/default
Add ciflow labels to this PR to trigger more builds:

Workflows Labels (bold enabled) Status
Triggered Workflows
linux-binary-conda ciflow/binaries, ciflow/binaries_conda, ciflow/default ✅ triggered
linux-binary-libtorch-cxx11-abi ciflow/binaries, ciflow/binaries_libtorch, ciflow/default ✅ triggered
linux-binary-libtorch-pre-cxx11 ciflow/binaries, ciflow/binaries_libtorch, ciflow/default ✅ triggered
linux-binary-manywheel ciflow/binaries, ciflow/binaries_wheel, ciflow/default ✅ triggered
linux-bionic-py3.7-clang9 ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/noarch, ciflow/trunk, ciflow/xla ✅ triggered
linux-docs ciflow/all, ciflow/cpu, ciflow/default, ciflow/docs, ciflow/linux, ciflow/trunk ✅ triggered
linux-vulkan-bionic-py3.7-clang9 ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/trunk, ciflow/vulkan ✅ triggered
linux-xenial-cuda11.3-py3.7-gcc7 ciflow/all, ciflow/cuda, ciflow/default, ciflow/linux, ciflow/trunk ✅ triggered
linux-xenial-cuda11.3-py3.7-gcc7-bazel-test ciflow/all, ciflow/bazel, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/trunk ✅ triggered
linux-xenial-py3-clang5-mobile-build ciflow/all, ciflow/default, ciflow/linux, ciflow/mobile, ciflow/trunk ✅ triggered
linux-xenial-py3-clang5-mobile-custom-build-static ciflow/all, ciflow/default, ciflow/linux, ciflow/mobile, ciflow/trunk ✅ triggered
linux-xenial-py3.7-clang7-asan ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/sanitizers, ciflow/trunk ✅ triggered
linux-xenial-py3.7-clang7-onnx ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/onnx, ciflow/trunk ✅ triggered
linux-xenial-py3.7-gcc5.4 ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/trunk ✅ triggered
linux-xenial-py3.7-gcc7 ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/trunk ✅ triggered
linux-xenial-py3.7-gcc7-no-ops ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/trunk ✅ triggered
pytorch-linux-xenial-py3-clang5-android-ndk-r19c-gradle-custom-build-single ciflow/all, ciflow/android, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/trunk ✅ triggered
pytorch-linux-xenial-py3-clang5-android-ndk-r19c-gradle-custom-build-single-full-jit ciflow/all, ciflow/android, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/trunk ✅ triggered
win-vs2019-cpu-py3 ciflow/all, ciflow/cpu, ciflow/default, ciflow/trunk, ciflow/win ✅ triggered
win-vs2019-cuda11.3-py3 ciflow/all, ciflow/cuda, ciflow/default, ciflow/trunk, ciflow/win ✅ triggered
Skipped Workflows
caffe2-linux-xenial-py3.7-gcc5.4 ciflow/all, ciflow/cpu, ciflow/linux, ciflow/trunk 🚫 skipped
docker-builds ciflow/all, ciflow/trunk 🚫 skipped
ios-12-5-1-arm64 ciflow/all, ciflow/ios, ciflow/macos, ciflow/trunk 🚫 skipped
ios-12-5-1-arm64-coreml ciflow/all, ciflow/ios, ciflow/macos, ciflow/trunk 🚫 skipped
ios-12-5-1-arm64-custom-ops ciflow/all, ciflow/ios, ciflow/macos, ciflow/trunk 🚫 skipped
ios-12-5-1-arm64-full-jit ciflow/all, ciflow/ios, ciflow/macos, ciflow/trunk 🚫 skipped
ios-12-5-1-arm64-metal ciflow/all, ciflow/ios, ciflow/macos, ciflow/trunk 🚫 skipped
ios-12-5-1-x86-64 ciflow/all, ciflow/ios, ciflow/macos, ciflow/trunk 🚫 skipped
ios-12-5-1-x86-64-coreml ciflow/all, ciflow/ios, ciflow/macos, ciflow/trunk 🚫 skipped
ios-12-5-1-x86-64-full-jit ciflow/all, ciflow/ios, ciflow/macos, ciflow/trunk 🚫 skipped
libtorch-linux-xenial-cuda10.2-py3.7-gcc7 ciflow/all, ciflow/cuda, ciflow/libtorch, ciflow/linux, ciflow/trunk 🚫 skipped
libtorch-linux-xenial-cuda11.3-py3.7-gcc7 ciflow/all, ciflow/cuda, ciflow/libtorch, ciflow/linux, ciflow/trunk 🚫 skipped
linux-bionic-cuda10.2-py3.9-gcc7 ciflow/all, ciflow/cuda, ciflow/linux, ciflow/slow, ciflow/trunk 🚫 skipped
linux-bionic-rocm4.5-py3.7 ciflow/all, ciflow/linux, ciflow/rocm, ciflow/trunk 🚫 skipped
linux-docs-push ciflow/all, ciflow/cpu, ciflow/linux, ciflow/scheduled 🚫 skipped
linux-xenial-cuda11.3-py3.7-gcc7-no-ops ciflow/all, ciflow/cuda, ciflow/linux, ciflow/trunk 🚫 skipped
macos-10-15-py3-arm64 ciflow/all, ciflow/macos, ciflow/trunk 🚫 skipped
macos-10-15-py3-lite-interpreter-x86-64 ciflow/all, ciflow/macos, ciflow/trunk 🚫 skipped
macos-11-py3-x86-64 ciflow/all, ciflow/macos, ciflow/trunk 🚫 skipped
parallelnative-linux-xenial-py3.7-gcc5.4 ciflow/all, ciflow/cpu, ciflow/linux, ciflow/trunk 🚫 skipped
periodic-libtorch-linux-bionic-cuda11.5-py3.7-gcc7 ciflow/all, ciflow/cuda, ciflow/libtorch, ciflow/linux, ciflow/scheduled 🚫 skipped
periodic-libtorch-linux-xenial-cuda11.1-py3.7-gcc7 ciflow/all, ciflow/cuda, ciflow/libtorch, ciflow/linux, ciflow/scheduled 🚫 skipped
periodic-linux-bionic-cuda11.5-py3.7-gcc7 ciflow/all, ciflow/cuda, ciflow/linux, ciflow/scheduled 🚫 skipped
periodic-linux-xenial-cuda10.2-py3-gcc7-slow-gradcheck ciflow/all, ciflow/cuda, ciflow/linux, ciflow/scheduled, ciflow/slow, ciflow/slow-gradcheck 🚫 skipped
periodic-linux-xenial-cuda11.1-py3.7-gcc7-debug ciflow/all, ciflow/cuda, ciflow/linux, ciflow/scheduled 🚫 skipped
periodic-win-vs2019-cuda11.1-py3 ciflow/all, ciflow/cuda, ciflow/scheduled, ciflow/win 🚫 skipped
periodic-win-vs2019-cuda11.5-py3 ciflow/all, ciflow/cuda, ciflow/scheduled, ciflow/win 🚫 skipped
pytorch-linux-xenial-py3-clang5-android-ndk-r19c-build ciflow/all, ciflow/android, ciflow/cpu, ciflow/linux, ciflow/trunk 🚫 skipped

@facebook-github-bot
Copy link
Contributor
facebook-github-bot commented Feb 25, 2022

🔗 Helpful links

💊 CI failures summary and remediations

As of commit 3d2f8b7 (more details on the Dr. CI page):


  • 2/2 failures introduced in this PR

🕵️ 2 new failures recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

See GitHub Actions build linux-xenial-cuda11.3-py3.7-gcc7 / build (1/2)

Step: "Build" (full log | diagnosis details | 🔁 rerun)

2022-03-14T23:01:57.7418303Z �[36;1m echo "ERR...t available for the merge-base of your branch"�[0m
2022-03-14T23:01:57.7415299Z �[36;1mfi�[0m
2022-03-14T23:01:57.7415547Z �[36;1m# Covers the case where a previous tag doesn't exist for the tree�[0m
2022-03-14T23:01:57.7415899Z �[36;1m# this is only really applicable on trees that don't have `.circleci/docker` at its merge base, i.e. nightly�[0m
2022-03-14T23:01:57.7416232Z �[36;1mif ! git rev-parse "$MERGE_BASE:.circleci/docker"; then�[0m
2022-03-14T23:01:57.7416579Z �[36;1m  echo "Directory '.circleci/docker' not found in commit $MERGE_BASE, you should probably rebase onto a more recent commit"�[0m
2022-03-14T23:01:57.7416852Z �[36;1m  exit 1�[0m
2022-03-14T23:01:57.7417019Z �[36;1mfi�[0m
2022-03-14T23:01:57.7417257Z �[36;1mPREVIOUS_DOCKER_TAG=$(git rev-parse "$MERGE_BASE:.circleci/docker")�[0m
2022-03-14T23:01:57.7417658Z �[36;1m# If no image exists but the hash is the same as the previous hash then we should error out here�[0m
2022-03-14T23:01:57.7417966Z �[36;1mif [[ "${PREVIOUS_DOCKER_TAG}" = "${DOCKER_TAG}" ]]; then�[0m
2022-03-14T23:01:57.7418303Z �[36;1m  echo "ERROR: Something has gone wrong and the previous image isn't available for the merge-base of your branch"�[0m
2022-03-14T23:01:57.7418656Z �[36;1m  echo "       contact the PyTorch team to restore the original images"�[0m
2022-03-14T23:01:57.7418883Z �[36;1m  exit 1�[0m
2022-03-14T23:01:57.7419048Z �[36;1mfi�[0m
2022-03-14T23:01:57.7419247Z �[36;1mecho ::set-output name=rebuild::yes�[0m
2022-03-14T23:01:57.7429254Z shell: /usr/bin/bash -e {0}
2022-03-14T23:01:57.7429444Z env:
2022-03-14T23:01:57.7429677Z   BUILD_ENVIRONMENT: linux-xenial-cuda11.3-py3.7-gcc7
2022-03-14T23:01:57.7430064Z   DOCKER_IMAGE_BASE: 308535385114.dkr.ecr.us-east-1.amazonaws.com/pytorch/pytorch-linux-xenial-cuda11.3-cudnn8-py3-gcc7
2022-03-14T23:01:57.7430455Z   SCCACHE_BUCKET: ossci-compiler-cache-circleci-v2
2022-03-14T23:01:57.7430782Z   XLA_CLANG_CACHE_S3_BUCKET_NAME: ossci-compiler-clang-cache-circleci-xla

See GitHub Actions build Lint / clang-tidy (2/2)

Step: "Check for warnings" (full log | diagnosis details | 🔁 rerun)

2022-03-14T23:06:32.0737513Z /__w/pytorch/pytor...ng-diagnostic-unused-variable,-warnings-as-errors]
2022-03-14T23:06:31.8440356Z �[36;1mset -eu�[0m
2022-03-14T23:06:31.8440586Z �[36;1mcat "${GITHUB_WORKSPACE}"/clang-tidy-output.txt�[0m
2022-03-14T23:06:31.8440900Z �[36;1mif grep -Fq "Warnings detected!" "${GITHUB_WORKSPACE}"/clang-tidy-output.txt; then�[0m
2022-03-14T23:06:31.8441216Z �[36;1m  echo 'Please fix the above clang-tidy warnings.'�[0m
2022-03-14T23:06:31.8441418Z �[36;1m  false�[0m
2022-03-14T23:06:31.8441578Z �[36;1mfi�[0m
2022-03-14T23:06:31.8441781Z shell: sh -e {0}
2022-03-14T23:06:31.8441966Z ##[endgroup]
2022-03-14T23:06:32.0736255Z Processing 6 clang-tidy jobs
2022-03-14T23:06:32.0736757Z Processing 45 clang-tidy jobs
2022-03-14T23:06:32.0737513Z /__w/pytorch/pytorch/torch/csrc/deploy/interpreter/builtin_registry.cpp:93:7: error: unused variable 'r' [clang-diagnostic-unused-variable,-warnings-as-errors]
2022-03-14T23:06:32.0738029Z   int r = PyRun_SimpleString(metaPathSetupScript.c_str());
2022-03-14T23:06:32.0738239Z       ^
2022-03-14T23:06:32.0740360Z Warnings detected!
2022-03-14T23:06:32.0740650Z Summary:
2022-03-14T23:06:32.0741170Z [clang-diagnostic-unused-variable] occurred 1 times
2022-03-14T23:06:32.0741738Z     /__w/pytorch/pytorch/torch/csrc/deploy/interpreter/builtin_registry.cpp:93
2022-03-14T23:06:32.0742060Z 
2022-03-14T23:06:32.0773467Z Please fix the above clang-tidy warnings.
2022-03-14T23:06:32.0817989Z ##[error]Process completed with exit code 1.
2022-03-14T23:06:32.0864104Z ##[group]Run pytorch/add-annotations-github-action@master

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

PaliC added a commit that referenced this pull request Feb 25, 2022
ghstack-source-id: 4572b80
Pull Request resolved: #73456
@PaliC PaliC requested review from suo, kiukchung, d4l3k and aivanou February 28, 2022 20:51
@PaliC
Copy link
Contributor Author
PaliC commented Mar 2, 2022

@PaliC has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Replaces `at::Error` with a more simple implementation of exceptions in order to reduce the dependency of torch::deploy on torch.

Note: Internal Testing / changes are still needed

Differential Revision: [D34568590](https://our.internmc.facebook.com/intern/diff/D34568590)

[ghstack-poisoned]
Replaces `at::Error` with a more simple implementation of exceptions in order to reduce the dependency of torch::deploy on torch.

Note: Internal Testing / changes are still needed

Differential Revision: [D34568590](https://our.internmc.facebook.com/intern/diff/D34568590)

[ghstack-poisoned]
Replaces `at::Error` with a more simple implementation of exceptions in order to reduce the dependency of torch::deploy on torch.

Note: Internal Testing / changes are still needed

Differential Revision: [D34568590](https://our.internmc.facebook.com/intern/diff/D34568590)

[ghstack-poisoned]
PaliC added a commit that referenced this pull request Mar 9, 2022
ghstack-source-id: 5b5c47c
Pull Request resolved: #73456
Replaces `at::Error` with a more simple implementation of exceptions in order to reduce the dependency of torch::deploy on torch.

Note: Internal Testing / changes are still needed

Differential Revision: [D34568590](https://our.internmc.facebook.com/intern/diff/D34568590)

[ghstack-poisoned]
@PaliC
Copy link
Contributor Author
PaliC commented Mar 9, 2022

@PaliC has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Replaces `at::Error` with a more simple implementation of exceptions in order to reduce the dependency of torch::deploy on torch.

Note: Internal Testing / changes are still needed

Differential Revision: [D34568590](https://our.internmc.facebook.com/intern/diff/D34568590)

[ghstack-poisoned]
@PaliC
Copy link
Contributor Author
PaliC commented Mar 10, 2022

@PaliC has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Replaces `at::Error` with a more simple implementation of exceptions in order to reduce the dependency of torch::deploy on torch.

Note: Internal Testing / changes are still needed

Differential Revision: [D34568590](https://our.internmc.facebook.com/intern/diff/D34568590)

[ghstack-poisoned]
@PaliC
Copy link
Contributor Author
PaliC commented Mar 11, 2022

@PaliC has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Replaces `at::Error` with a more simple implementation of exceptions in order to reduce the dependency of torch::deploy on torch.

Note: Internal Testing / changes are still needed

Differential Revision: [D34568590](https://our.internmc.facebook.com/intern/diff/D34568590)

[ghstack-poisoned]
@PaliC
Copy link
Contributor Author
PaliC commented Mar 12, 2022

@PaliC has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@PaliC
Copy link
Contributor Author
PaliC commented Mar 14, 2022

@PaliC has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Replaces `at::Error` with a more simple implementation of exceptions in order to reduce the dependency of torch::deploy on torch.

Note: Internal Testing / changes are still needed

Differential Revision: [D34868005](https://our.internmc.facebook.com/intern/diff/D34868005)

[ghstack-poisoned]
@PaliC
Copy link
Contributor Author
PaliC commented Mar 14, 2022

@PaliC has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Replaces `at::Error` with a more simple implementation of exceptions in order to reduce the dependency of torch::deploy on torch.

Note: Internal Testing / changes are still needed

Differential Revision: [D34868005](https://our.internmc.facebook.com/intern/diff/D34868005)

[ghstack-poisoned]
@PaliC
Copy link
Contributor Author
PaliC commented Mar 14, 2022

@PaliC has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Replaces `at::Error` with a more simple implementation of exceptions in order to reduce the dependency of torch::deploy on torch.

Note: Internal Testing / changes are still needed

Differential Revision: [D34868005](https://our.internmc.facebook.com/intern/diff/D34868005)

[ghstack-poisoned]
@PaliC
Copy link
Contributor Author
PaliC commented Mar 14, 2022

@PaliC has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

1 similar comment
@PaliC
Copy link
Contributor Author
PaliC commented Mar 14, 2022

@PaliC has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit that referenced this pull request Mar 15, 2022
Summary:
Pull Request resolved: #73456

Replaces `at::Error` with a more simple implementation of exceptions in order to reduce the dependency of torch::deploy on torch.

Note: Internal Testing / changes are still needed

Test Plan: Imported from OSS

Reviewed By: samdow

Differential Revision: D34868005

Pulled By: PaliC

fbshipit-source-id: c8bb1f7a2b169b5a8e3b63a697e0ced748a0524c
@github-actions
Copy link
Contributor

Hey @PaliC.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

@facebook-github-bot
Copy link
Contributor

This pull request has been reverted by cc5f8ae. To re-land this change, please open another pull request, assignthe same reviewers, fix the CI failures that caused the revert and make sure that the failing CI runs on the PR by applying the proper ciflow label (e.g., ciflow/trunk).

PaliC added a commit to PaliC/pytorch that referenced this pull request Mar 16, 2022
Summary:
Pull Request resolved: pytorch#74283

Remove `c10::errors` from torch::deploy and replace them with `multipy::errors` which is effectively a wrapper around `std::runtime_error.

Review History can be found with pytorch#73456

Test Plan: buck test //caffe2/torch/csrc/deploy:test_deploy

Differential Revision: D34905174

fbshipit-source-id: 528d9b3a010360645c669efa3c7852d7ae5e649c
facebook-github-bot pushed a commit that referenced this pull request Mar 16, 2022
Summary:
Pull Request resolved: #74283

Remove `c10::errors` from torch::deploy and replace them with `multipy::errors` which is effectively a wrapper around `std::runtime_error.

Review History can be found with #73456

Test Plan: buck test //caffe2/torch/csrc/deploy:test_deploy

Reviewed By: aivanou

Differential Revision: D34905174

fbshipit-source-id: 8883fc77dce66c489fa3fa9d14a71d1de1e0cc5f
pytorchmergebot pushed a commit that referenced this pull request Mar 16, 2022
Summary:
Pull Request resolved: #74283

Remove `c10::errors` from torch::deploy and replace them with `multipy::errors` which is effectively a wrapper around `std::runtime_error.

Review History can be found with #73456

Test Plan: buck test //caffe2/torch/csrc/deploy:test_deploy

Reviewed By: aivanou

Differential Revision: D34905174

fbshipit-source-id: 8883fc77dce66c489fa3fa9d14a71d1de1e0cc5f
(cherry picked from commit 7fffcdf)
@facebook-github-bot facebook-github-bot deleted the gh/PaliC/142/head branch March 19, 2022 14:17
@facebook-github-bot
Copy link
Contributor

This pull request has been reverted by cc5f8ae. To re-land this change, please open another pull request, assignthe same reviewers, fix the CI failures that caused the revert and make sure that the failing CI runs on the PR by applying the proper ciflow label (e.g., ciflow/trunk).

@suo suo mentioned this pull request Mar 22, 2022
NesrineMHB pushed a commit to NesrineMHB/pytorch that referenced this pull request Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0