Skip to content

Conversation

@izdeby
Copy link
Contributor

@izdeby izdeby commented Apr 10, 2019

This is bc-breaking change
Change dtype of a tensor which was created from bool data.
Old behavior: torch.tensor([True, False]) -> uint8 tensor
Now: torch.tensor([True, False]) -> bool tensor

Tested via tests.

@izdeby
Copy link
Contributor Author

izdeby commented Apr 10, 2019

@gchanan, i think this shows the JIT issue that you were mentioning yesterday. the failing test(test_jit.py::test_torch_tensor) shows that JIT.CompilationUnit is behaving differently. Should i update JITs logic or is it a BC issue and i should add a deprecation warning instead?

if (PyBool_Check(obj)) {
// TODO: infer Bool when we have Bool ScalarType
return ScalarType::Byte;
return ScalarType::Bool;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all of the non-test changes besides this look correct and can go in (because they don't break backcompat), right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this and jit changes

@gchanan
Copy link
Contributor

gchanan commented Apr 11, 2019

you should figure out where the JIT logic is first.

@izdeby izdeby force-pushed the changeTypeOfATensorWithBools branch from 5d36304 to 4947864 Compare April 15, 2019 23:54
@izdeby izdeby force-pushed the changeTypeOfATensorWithBools branch from 4947864 to 59824ea Compare June 4, 2019 18:22
@pytorchbot pytorchbot added oncall: jit Add this issue/PR to JIT oncall triage queue module: autograd Related to torch.autograd, and the autograd engine in general module: dataloader Related to torch.utils.data.DataLoader and Sampler module: internals Related to internal abstractions in c10 and ATen module: operators module: pybind Related to our Python bindings / interactions with other Python libraries labels Jun 4, 2019
@izdeby izdeby changed the title [WIP] Change type of a tensor with bools Сhange type of a tensor with bools Jun 4, 2019
@izdeby izdeby requested review from VitalyFedyunin, eellison, ezyang and gchanan and removed request for gchanan June 4, 2019 21:20
@izdeby izdeby added the module: bc-breaking Related to a BC-breaking change label Jun 4, 2019
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@facebook-github-bot
Copy link
Contributor

@izdeby merged this pull request in 0361757.

zdevito pushed a commit to zdevito/ATen that referenced this pull request Jun 5, 2019
Summary:
**This is **bc-breaking** change**
Change dtype of a tensor which was created from bool data.
Old behavior: torch.tensor([True, False]) -> uint8 tensor
Now: torch.tensor([True, False]) -> bool tensor

Tested via tests.
Pull Request resolved: pytorch/pytorch#19097

Reviewed By: ezyang

Differential Revision: D15632553

Pulled By: izdeby

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

Labels

module: autograd Related to torch.autograd, and the autograd engine in general module: bc-breaking Related to a BC-breaking change module: dataloader Related to torch.utils.data.DataLoader and Sampler module: internals Related to internal abstractions in c10 and ATen module: pybind Related to our Python bindings / interactions with other Python libraries oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants