Skip to content

[Py3.10] Allow floats to be imported as Long#81372

Closed
malfet wants to merge 4 commits intomasterfrom
malfet/py310-add-float-to-int-conversion
Closed

[Py3.10] Allow floats to be imported as Long#81372
malfet wants to merge 4 commits intomasterfrom
malfet/py310-add-float-to-int-conversion

Conversation

@malfet
Copy link
Copy Markdown
Contributor

@malfet malfet commented Jul 12, 2022 •
edited
Loading

Thus avoiding TypeError: 'float' object cannot be interpreted as an integer when trying to create integer tensor from floating point values

Use c10::checked_convert to detect overflows during tensor construction from scalars. Modify sparse_csr test that violated this rule

Fixes #69319

Tested in #81233

@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Jul 12, 2022 •
edited
Loading

🔗 Helpful links

❌ 1 New Failures

As of commit 0c350c0 (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 / linux-focal-py3.7-gcc7-mobile-lightweight-dispatch-build / build (1/1)

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

2022-07-15T20:42:31.2515435Z ##[error]Process completed with exit code 137. 
2022-07-15T20:37:17.2926497Z [ 96%] �[32mBuilding CXX object caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/RegisterCodegenUnboxedKernels_1.cpp.o�[0m
2022-07-15T20:37:19.2086015Z [ 96%] �[32mBuilding CXX object caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/RegisterCodegenUnboxedKernels_2.cpp.o�[0m
2022-07-15T20:37:21.0686934Z [ 96%] �[32mBuilding CXX object caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/RegisterCodegenUnboxedKernels_3.cpp.o�[0m
2022-07-15T20:37:27.0622745Z [ 96%] �[32mBuilding CXX object caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/RegisterCodegenUnboxedKernels_4.cpp.o�[0m
2022-07-15T20:37:27.8624288Z [ 97%] �[32mBuilding CXX object caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/RegisterCodegenUnboxedKernels_5.cpp.o�[0m
2022-07-15T20:38:48.1952936Z [ 97%] �[32mBuilding CXX object caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/RegisterCodegenUnboxedKernels_6.cpp.o�[0m
2022-07-15T20:39:29.3082099Z [ 97%] �[32mBuilding CXX object caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/RegisterCodegenUnboxedKernels_7.cpp.o�[0m
2022-07-15T20:41:26.4514691Z [ 97%] �[32mBuilding CXX object caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/RegisterCodegenUnboxedKernels_8.cpp.o�[0m
2022-07-15T20:41:50.4272688Z [ 97%] �[32mBuilding CXX object caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/RegisterCodegenUnboxedKernels_9.cpp.o�[0m
2022-07-15T20:42:01.0299253Z [ 97%] �[32mBuilding CXX object caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/UnboxingFunctions_0.cpp.o�[0m
2022-07-15T20:42:31.2515435Z ##[error]Process completed with exit code 137.
2022-07-15T20:42:31.3895024Z Prepare all required actions
2022-07-15T20:42:31.4094086Z ##[group]Run ./.github/actions/teardown-linux
2022-07-15T20:42:31.4094381Z with:
2022-07-15T20:42:31.4094587Z ##[endgroup]
2022-07-15T20:42:31.4133297Z ##[group]Run .github/scripts/wait_for_ssh_to_drain.sh
2022-07-15T20:42:31.4133668Z �[36;1m.github/scripts/wait_for_ssh_to_drain.sh�[0m
2022-07-15T20:42:31.4941567Z shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
2022-07-15T20:42:31.4941960Z ##[endgroup]
2022-07-15T20:42:31.5237241Z Holding runner for 2 hours until all ssh sessions have logged out
2022-07-15T20:42:31.5340855Z ##[group]Run # ignore expansion of "docker ps -q" since it could be empty
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.

@malfet malfet requested review from ezyang, ngimel and suo July 12, 2022 23:22
malfet added 2 commits July 15, 2022 10:20
@malfet
[Py3.10] Allow floats to be imported as Long
56cced7 
Thus avoiding `TypeError: 'float' object cannot be interpreted as an integer` when trying to create integer tensor from floating point values

Fixes #69319
@malfet
Revert "[Py3.10] Allow floats to be imported as Long"
aa8574b 
This reverts commit f64ca34.
@malfet malfet force-pushed the malfet/py310-add-float-to-int-conversion branch from f64ca34 to 7e9cfbb Compare July 15, 2022 17:20
@malfet malfet force-pushed the malfet/py310-add-float-to-int-conversion branch from 7e9cfbb to dde1ce5 Compare July 15, 2022 17:26
@malfet malfet added ciflow/trunk Trigger trunk jobs on your pull request release notes: python_frontend python frontend release notes category topic: bc breaking topic category topic: improvements topic category module: python frontend For issues relating to PyTorch's Python frontend and removed module: python frontend For issues relating to PyTorch's Python frontend labels Jul 15, 2022
@malfet
Copy link
Copy Markdown
Contributor Author

malfet commented Jul 15, 2022

@pytorchbot merge -f

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

pytorchmergebot commented Jul 15, 2022

@pytorchbot successfully started a merge job. Check the current status here

@malfet malfet deleted the malfet/py310-add-float-to-int-conversion branch July 17, 2022 18:12
@DanilBaibak
Copy link
Copy Markdown
Contributor

DanilBaibak commented Jul 18, 2022

Hi @malfet! Your PR breaks the internal release D37919741. Could you pls take a look:

torch.tensor([0x01, 0x03, 0xFFFFFFF0, 0x5], dtype=torch.int32),
RuntimeError: value cannot be converted to type int32 without overflow

@DanilBaibak
Copy link
Copy Markdown
Contributor

DanilBaibak commented Jul 18, 2022

@pytorchbot revert -m "Break internal build" -c ghfirst

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

pytorchmergebot commented Jul 18, 2022

@pytorchbot successfully started a revert job. Check the current status here

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

pytorchmergebot commented Jul 18, 2022

@malfet your PR has been successfully reverted.

pytorchmergebot added a commit that referenced this pull request Jul 18, 2022
@pytorchmergebot
Revert "[Py3.10] Allow floats to be imported as Long (#81372)"
ec1b3a4 
This reverts commit 69d7334.

Reverted #81372 on behalf of https://github.com/DanilBaibak due to Break internal build
@malfet
Copy link
Copy Markdown
Contributor Author

malfet commented Jul 18, 2022

@DanilBaibak you'll need to revert one that enables py-3.10
As well

@ezyang
Copy link
Copy Markdown
Contributor

ezyang commented Jul 20, 2022

What's the new plan on this

@janeyx99 janeyx99 mentioned this pull request Jul 22, 2022
Closed
@MeowZheng MeowZheng mentioned this pull request Jul 26, 2022
Merged
@malfet malfet mentioned this pull request Jul 27, 2022
Closed
malfet added a commit that referenced this pull request Jul 27, 2022
@malfet
[Py3.10] Allow floats to be imported as Long (#81372)
45c5aa5 
Thus avoiding `TypeError: 'float' object cannot be interpreted as an integer` when trying to create integer tensor from floating point values

Use `c10::checked_convert` to detect overflows during tensor construction from scalars. Modify sparse_csr test that violated this rule

Fixes #69319

Tested in #81233

Pull Request resolved: #81372
Approved by: https://github.com/ezyang, https://github.com/ngimel

(cherry picked from commit 69d7334)
@malfet
Copy link
Copy Markdown
Contributor Author

malfet commented Jul 27, 2022

What's the new plan on this

@ezyang I'm going to reland the change with range checks disabled for pre-3.10 and then work on internal regressions in separate PR

@malfet
Copy link
Copy Markdown
Contributor Author

malfet commented Jul 27, 2022

Your PR breaks the internal release D37919741. Could you pls take a look:

torch.tensor([0x01, 0x03, 0xFFFFFFF0, 0x5], dtype=torch.int32),
RuntimeError: value cannot be converted to type int32 without overflow

@DanilBaibak wrong diff number, isn't it?

pytorchmergebot pushed a commit that referenced this pull request Jul 27, 2022
@malfet @pytorchmergebot
[Reland] Add py-3.10 config (#82329)
d80fe49 
This is a re-land of #81372 and #81233 with the exception that it does not force the range-checks on older Python runtime versions and as such should not affect the internal workloads, which were the reason for revert, see #81372 (comment)

- [Py3.10] Allow floats to be imported as Long (#81372)
- [CI] Move CUDA-11.6 to Python-3.10 configuration (#81233)
- Don't do anything about range checks for pre-py3.10
Pull Request resolved: #82329
Approved by: https://github.com/kit1980
facebook-github-bot pushed a commit that referenced this pull request Jul 28, 2022
@malfet @facebook-github-bot
[Reland] Add py-3.10 config (#82329) (#82329)
057361e 
Summary:
This is a re-land of #81372 and #81233 with the exception that it does not force the range-checks on older Python runtime versions and as such should not affect the internal workloads, which were the reason for revert, see #81372 (comment)

- [Py3.10] Allow floats to be imported as Long (#81372)
- [CI] Move CUDA-11.6 to Python-3.10 configuration (#81233)
- Don't do anything about range checks for pre-py3.10

Pull Request resolved: #82329
Approved by: https://github.com/kit1980

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

Reviewed By: osalpekar

Differential Revision: D38234293

Pulled By: malfet

fbshipit-source-id: 3f7ffe729ab2a3ef692bc95f5398dfdf395b2397
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ezyang ezyang ezyang approved these changes

@ngimel ngimel ngimel approved these changes

@suo suo Awaiting requested review from suo

Assignees

No one assigned

Labels

ciflow/trunk Trigger trunk jobs on your pull request cla signed Merged release notes: python_frontend python frontend release notes category Reverted topic: bc breaking topic category topic: improvements topic category

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@malfet @facebook-github-bot @pytorchmergebot @DanilBaibak @ezyang @ngimel