Skip to content

lshift and rshift stop support floating types#77146

Closed
zasdfgbnm wants to merge 6 commits intomasterfrom
shift
Closed

lshift and rshift stop support floating types#77146
zasdfgbnm wants to merge 6 commits intomasterfrom
shift

Conversation

@zasdfgbnm
Copy link
Collaborator

@zasdfgbnm zasdfgbnm commented May 10, 2022 •
edited by ngimel
Loading

Fixes #74358

@zasdfgbnm zasdfgbnm requested review from mruberry and ngimel as code owners May 10, 2022 05:38
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented May 10, 2022 •
edited
Loading

🔗 Helpful links

❌ 1 New Failures

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

Expand to see more
  • 1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

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

See GitHub Actions build pull / pytorch-xla-linux-bionic-py3.7-clang8 / test (xla, 1, 1, linux.2xlarge) (1/1)

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

2022-05-11T20:05:36.0614409Z /var/lib/jenkins/w... virtual member functions can be marked 'override' 
2022-05-11T20:05:36.0608647Z In file included from /var/lib/jenkins/workspace/xla/torch_xla/csrc/aten_xla_bridge.cpp:11:
2022-05-11T20:05:36.0609513Z /var/lib/jenkins/workspace/xla/torch_xla/csrc/tensor_impl.h:33:40: error: only virtual member functions can be marked 'override'
2022-05-11T20:05:36.0610113Z   at::IntArrayRef sizes_custom() const override;
2022-05-11T20:05:36.0610471Z                                        ^~~~~~~~
2022-05-11T20:05:36.0611193Z /var/lib/jenkins/workspace/xla/torch_xla/csrc/tensor_impl.h:34:42: error: only virtual member functions can be marked 'override'
2022-05-11T20:05:36.0611766Z   at::IntArrayRef strides_custom() const override;
2022-05-11T20:05:36.0612152Z                                          ^~~~~~~~
2022-05-11T20:05:36.0612835Z /var/lib/jenkins/workspace/xla/torch_xla/csrc/tensor_impl.h:36:30: error: only virtual member functions can be marked 'override'
2022-05-11T20:05:36.0613373Z   int64_t dim_custom() const override;
2022-05-11T20:05:36.0613715Z                              ^~~~~~~~
2022-05-11T20:05:36.0614409Z /var/lib/jenkins/workspace/xla/torch_xla/csrc/tensor_impl.h:38:32: error: only virtual member functions can be marked 'override'
2022-05-11T20:05:36.0614936Z   int64_t numel_custom() const override;
2022-05-11T20:05:36.0615268Z                                ^~~~~~~~
2022-05-11T20:05:36.0615726Z 4 errors generated.
2022-05-11T20:05:38.3895811Z [10/179] clang++-8 -MMD -MF /var/lib/jenkins/workspace/xla/build/temp.linux-x86_64-3.7/torch_xla/csrc/convert_ops.o.d -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -D_GLIBCXX_USE_CXX11_ABI=1 -fPIC -I/var/lib/jenkins/workspace/xla -I/var/lib/jenkins/workspace/xla/third_party/tensorflow/bazel-tensorflow -I/var/lib/jenkins/workspace/xla/third_party/tensorflow/bazel-bin -I/var/lib/jenkins/workspace/xla/third_party/tensorflow/bazel-tensorflow/external/protobuf_archive/src -I/var/lib/jenkins/workspace/xla/third_party/tensorflow/bazel-tensorflow/external/com_google_protobuf/src -I/var/lib/jenkins/workspace/xla/third_party/tensorflow/bazel-tensorflow/external/eigen_archive -I/var/lib/jenkins/workspace/xla/third_party/tensorflow/bazel-tensorflow/external/com_google_absl -I/var/lib/jenkins/workspace -I/var/lib/jenkins/workspace/torch/csrc -I/var/lib/jenkins/workspace/torch/lib/tmp_install/include -I/opt/conda/lib/python3.7/site-packages/torch/include -I/opt/conda/lib/python3.7/site-packages/torch/include/torch/csrc/api/include -I/opt/conda/lib/python3.7/site-packages/torch/include/TH -I/opt/conda/lib/python3.7/site-packages/torch/include/THC -I/opt/conda/include/python3.7m -c -c /var/lib/jenkins/workspace/xla/torch_xla/csrc/convert_ops.cpp -o /var/lib/jenkins/workspace/xla/build/temp.linux-x86_64-3.7/torch_xla/csrc/convert_ops.o -std=c++14 -Wno-sign-compare -Wno-deprecated-declarations -Wno-return-type -Wno-macro-redefined -Wno-return-std-move -DNDEBUG -DTORCH_API_INCLUDE_EXTENSION_H '-DPYBIND11_COMPILER_TYPE="_clang"' '-DPYBIND11_STDLIB="_libstdcpp"' '-DPYBIND11_BUILD_ABI="_cxxabi1002"' -DTORCH_EXTENSION_NAME=_XLAC -D_GLIBCXX_USE_CXX11_ABI=1
2022-05-11T20:06:02.0952794Z [11/179] clang++-8 -MMD -MF /var/lib/jenkins/workspace/xla/build/temp.linux-x86_64-3.7/torch_xla/csrc/RegisterXLA.o.d -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -D_GLIBCXX_USE_CXX11_ABI=1 -fPIC -I/var/lib/jenkins/workspace/xla -I/var/lib/jenkins/workspace/xla/third_party/tensorflow/bazel-tensorflow -I/var/lib/jenkins/workspace/xla/third_party/tensorflow/bazel-bin -I/var/lib/jenkins/workspace/xla/third_party/tensorflow/bazel-tensorflow/external/protobuf_archive/src -I/var/lib/jenkins/workspace/xla/third_party/tensorflow/bazel-tensorflow/external/com_google_protobuf/src -I/var/lib/jenkins/workspace/xla/third_party/tensorflow/bazel-tensorflow/external/eigen_archive -I/var/lib/jenkins/workspace/xla/third_party/tensorflow/bazel-tensorflow/external/com_google_absl -I/var/lib/jenkins/workspace -I/var/lib/jenkins/workspace/torch/csrc -I/var/lib/jenkins/workspace/torch/lib/tmp_install/include -I/opt/conda/lib/python3.7/site-packages/torch/include -I/opt/conda/lib/python3.7/site-packages/torch/include/torch/csrc/api/include -I/opt/conda/lib/python3.7/site-packages/torch/include/TH -I/opt/conda/lib/python3.7/site-packages/torch/include/THC -I/opt/conda/include/python3.7m -c -c /var/lib/jenkins/workspace/xla/torch_xla/csrc/RegisterXLA.cpp -o /var/lib/jenkins/workspace/xla/build/temp.linux-x86_64-3.7/torch_xla/csrc/RegisterXLA.o -std=c++14 -Wno-sign-compare -Wno-deprecated-declarations -Wno-return-type -Wno-macro-redefined -Wno-return-std-move -DNDEBUG -DTORCH_API_INCLUDE_EXTENSION_H '-DPYBIND11_COMPILER_TYPE="_clang"' '-DPYBIND11_STDLIB="_libstdcpp"' '-DPYBIND11_BUILD_ABI="_cxxabi1002"' -DTORCH_EXTENSION_NAME=_XLAC -D_GLIBCXX_USE_CXX11_ABI=1
2022-05-11T20:06:02.0955336Z /var/lib/jenkins/workspace/xla/torch_xla/csrc/RegisterXLA.cpp:63:6: warning: unused function 'resize_out' [-Wunused-function]
2022-05-11T20:06:02.0955788Z void resize_out(const Tensor &out, IntArrayRef sizes, IntArrayRef strides, const TensorOptions &options) {
2022-05-11T20:06:02.0956055Z      ^
2022-05-11T20:06:02.0956502Z /var/lib/jenkins/workspace/xla/torch_xla/csrc/RegisterXLA.cpp:82:6: warning: unused function 'check_inplace' [-Wunused-function]
2022-05-11T20:06:02.0956890Z void check_inplace(const Tensor &self, IntArrayRef sizes, const TensorOptions &options) {
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.

@zasdfgbnm zasdfgbnm mentioned this pull request May 10, 2022
Closed
@samdow samdow added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label May 10, 2022
@ngimel
Copy link
Collaborator

ngimel commented May 10, 2022

Test failures are real (I don't know what TestLazyOps is doing, must be some silly stuff)

@ngimel
Copy link
Collaborator

ngimel commented May 10, 2022

xla failure is also real, and they also for some reason testing lshift on float

@ngimel
Copy link
Collaborator

ngimel commented May 10, 2022

@JackCaoG can you change the inputs from FLoat to integer here and in similar places https://github.com/pytorch/xla/blob/3c83269f8b163243263078c2f1e93d7617321712/test/cpp/test_aten_xla_tensor.cpp#L9157? It doesn't make sense to do lshift operations on floating point types.

@JackCaoG
Copy link
Collaborator

JackCaoG commented May 10, 2022

I will draft a xla pr for this.

@ngimel
Copy link
Collaborator

ngimel commented May 10, 2022

Thanks! Note that it can (and should) be landed without waiting for this PR, because lshift/rshift should work on integral inputs only.

@JackCaoG JackCaoG mentioned this pull request May 10, 2022
Merged
@JackCaoG
Copy link
Collaborator

JackCaoG commented May 11, 2022

@ngimel XLA PR is merged, we should be good to go for this one.

@ngimel ngimel reopened this May 11, 2022
@JackCaoG
Copy link
Collaborator

JackCaoG commented May 11, 2022 •
edited
Loading

ehh, sorry press the wrong bottom when I tried to comment.
I think I need to avoid fix xxx in pytorch/xla pr.. I thought it will only close pytorch/xla issue.

@ngimel
Copy link
Collaborator

ngimel commented May 11, 2022

@pytorchbot merge this

@github-actions
Copy link
Contributor

github-actions bot commented May 11, 2022

Hey @zasdfgbnm.
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.

@zasdfgbnm zasdfgbnm deleted the shift branch May 11, 2022 23:26
@atalman atalman mentioned this pull request May 12, 2022
Closed
facebook-github-bot pushed a commit that referenced this pull request May 13, 2022
@zasdfgbnm @facebook-github-bot
lshift and rshift stop support floating types (#77146) (#77146)
daeb2fc 
Summary:
Fixes #74358

Pull Request resolved: #77146
Approved by: https://github.com/ngimel

Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/cc9d0f309ec72cc53ffbbca3044ddb1a61c8ecf3

Reviewed By: atalman

Differential Revision: D36353737

Pulled By: atalman

fbshipit-source-id: 8b778725c3121e4dcb959a5b7d77707862a5fbff
@zasdfgbnm zasdfgbnm added the topic: bc breaking topic category label May 18, 2022
@zasdfgbnm
Copy link
Collaborator Author

zasdfgbnm commented May 18, 2022 •
edited
Loading

Adding topic:bc_breaking label as floating-point types support was previously mistakenly enabled and is now disabled.

@zasdfgbnm zasdfgbnm mentioned this pull request May 18, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ngimel ngimel ngimel approved these changes

@mruberry mruberry Awaiting requested review from mruberry

@BowenBao BowenBao Awaiting requested review from BowenBao

@shubhambhokare1 shubhambhokare1 Awaiting requested review from shubhambhokare1

Assignees

No one assigned

Labels

cla signed Merged module: primTorch open source topic: bc breaking topic category triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

lshift and rshift are supported for non-integral inputs and shifts

7 participants

@zasdfgbnm @facebook-github-bot @ngimel @JackCaoG @samdow @pytorchbot @pytorchmergebot