Skip to content

Conversation

izdeby
Copy link
Contributor

@izdeby izdeby commented Feb 21, 2019

This PR enables bool tensor creation and some basic operations for the CPU backend. This is a part of Bool Tensor feature implementation work. The whole plan looks like this:
1. Storage Implementation [Done]
2. Tensor Creation.
a) CPU (this PR)
b) CUDA
3. Tensor Conversions.
4. Tensor Indexing.
5. Tensor Operations.
6. Back compatibility related changes.

Change:
Enable CPU tensors and these operations:

  • torch.zeros
  • torch.tensor
  • torch.ones
  • torch.randint
  • torch.full
  • torch.full_like
  • torch.empty
  • torch.empty_like

Tested via:

  1. unit tests

torch.zeros(2,2, dtype=torch.bool)
torch.tensor([True, False], dtype=torch.bool)
torch.tensor([-1, -1.1, 0, 1, 1.1, 2], dtype=torch.bool)
torch.ones([1,2], dtype=torch.bool)
torch.randint(10, (2, 2), dtype=torch.bool)
torch.full((2, 3), True, dtype=torch.bool)
torch.empty(4, dtype=torch.bool)

a = torch.tensor([0,0,1])
b = torch.full_like(a, True)

@izdeby izdeby changed the title [WIP] Bool tensor creation (cpu) Bool tensor creation (cpu) Feb 22, 2019
@izdeby izdeby requested review from ezyang and gchanan and removed request for gchanan February 25, 2019 15:30
@izdeby izdeby self-assigned this Feb 25, 2019
@izdeby izdeby force-pushed the BoolTensorCreation_CPU branch from eabf001 to 26e35e6 Compare February 25, 2019 20:45
@ezyang
Copy link
Contributor

ezyang commented Feb 27, 2019

Test failures are real

@izdeby izdeby changed the title Bool tensor creation (cpu) [WIP]Bool tensor creation (cpu) Feb 27, 2019
Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

I most heavily reviewed the test code. The main code changes seem basically reasonable.

@ezyang
Copy link
Contributor

ezyang commented Feb 27, 2019

I took a look at the scalar dispatch macro situation, because I am a bit to blame for the situation.

Here's a summary of what the FORALL macros do:

  • AT_FORALL_SCALAR_TYPES_WITH_COMPLEX this is the "no actually, this contains every scalar type, and I mean EVERY scalar type" enum. We could probably rename this to AT_FORALL_SCALAR_TYPES_EVERYTHING to make its semantics clearer. Note that you very rarely use this macro (basically never for kernels), because it contains complex and we don't currently generate kernels for complex ever.

  • AT_FORALL_SCALAR_TYPES_WITH_COMPLEX_EXCEPT_COMPLEX_HALF is a very specialized macro that was introduced specifically to hack around having to add ComplexHalf accessor support to Scalar. Context is here [pytorch][PR] Conversions to and from complex numbers. #11420:

    - We grow a even yet more ludicrous macro
      AT_FORALL_SCALAR_TYPES_WITH_COMPLEX_EXCEPT_COMPLEX_HALF
      which does what it says on the tin.  This is because I was
      too lazy to figure out how to define the necessary conversions
      in and out of ComplexHalf without triggering ambiguity problems.
      It doesn't seem to be as simple as just Half.  Leave it for
      when someone actually wants this.
  • AT_FORALL_SCALAR_TYPES is the traditional "everything" macro, predating introduction of complex. It includes half. Since it includes half, it is basically never used to actually do dispatch either, since we don't have half kernels for most things, except in the few cases where we universally support half.

  • AT_FORALL_SCALAR_TYPES_EXCEPT_HALF is the workhorse macro; it doesn't include complex, it doesn't include half, so if you are working with an operator that doesn't know how to support complex or half specifically, this is the one you'll use.

Hopefully that makes the situation clearer. To summarize, we can roughly classify dtypes into support levels:

  • Classic floating point: float, double. Supported universally
  • Classic integral: byte, char, int, long, short. Supported universally
  • Half. Supported everywhere in CUDA, and nearly nowhere in CPU except a few situations
  • Complex. Supported nowhere, except in a few situations
  • Bool. I'm actually not sure if the intention is to support universally, or it will be supporting on operators on a case-by-case basis.

facebook-github-bot pushed a commit that referenced this pull request Feb 27, 2019
…ill (#17536)

Summary:
For some additional context on this change, please, see this [PR](#17376)

As a part of work on Bool Tensor, we will need to add support for a bool type to _fill() and _zero() methods that are currently located in THTensorMath. As we don't need anything else and those methods are not really math related - we are moving them out into separate THTensorFill for simplicity.

Change:
-moved _fill() and _zero() from THTensorMath.h to THTensorFill
-enabled _fill() and _zero() for HALF type.
Pull Request resolved: #17536

Differential Revision: D14242130

Pulled By: izdeby

fbshipit-source-id: 1d8bd806f0f5510723b9299d360b70cc4ab96afb
zdevito pushed a commit to zdevito/ATen that referenced this pull request Feb 27, 2019
…ill (#17536)

Summary:
For some additional context on this change, please, see this [PR](pytorch/pytorch#17376)

As a part of work on Bool Tensor, we will need to add support for a bool type to _fill() and _zero() methods that are currently located in THTensorMath. As we don't need anything else and those methods are not really math related - we are moving them out into separate THTensorFill for simplicity.

Change:
-moved _fill() and _zero() from THTensorMath.h to THTensorFill
-enabled _fill() and _zero() for HALF type.
Pull Request resolved: pytorch/pytorch#17536

Differential Revision: D14242130

Pulled By: izdeby

fbshipit-source-id: 1d8bd806f0f5510723b9299d360b70cc4ab96afb
@izdeby izdeby force-pushed the BoolTensorCreation_CPU branch 2 times, most recently from c9dc84e to 11e9016 Compare February 27, 2019 18:18
@izdeby izdeby changed the title [WIP]Bool tensor creation (cpu) Bool tensor creation (cpu) Feb 27, 2019
@izdeby izdeby requested a review from ezyang February 27, 2019 21:51
@gchanan
Copy link
Contributor

gchanan commented Feb 28, 2019

@ezyang I was actually referring to the dispatch macros here: https://github.com/pytorch/pytorch/blob/master/aten/src/ATen/Dispatch.h

I don't know if we have a name for the macros you are referencing -- ScalarType macros or something?

In any case, I'm less worried about those because users don't usually touch them. But they do touch the dispatch macros.

@gchanan
Copy link
Contributor

gchanan commented Mar 1, 2019

how about something like this: consider both Half and Bool special.

So we can keep e.g. AT_DISPATCH_FLOATING_TYPES_AND_HALF because I doubt anyone will ever want AT_DISPATCH_FLOATING_TYPES_AND_HALF_AND_BOOL. And we can add AT_DISPATCH_INTEGRAL_TYPES_AND_BOOL as the analog of AT_DISPATCH_FLOATING_TYPES_AND_HALF.

The issue is with the AT_DISPATCH_ALL_TYPES* macros. Instead of having AT_DISPATCH_ALL_TYPES_AND_HALF, how about:
AT_DISPATCH_ALL_TYPES_AND(SCALARTYPE, ...)
AT_DISPATCH_ALL_TYPES_AND(SCALARTYPE1, SCALARTYPE2)

so AT_DISPATCH_ALL_TYPES_AND_HALF would just be AT_DISPATCH_ALL_TYPES_AND(ScalarType::HALF,...) and what would logically have been AT_DISPATCH_ALL_TYPES_AND_HALF_AND_BOOL would be AT_DISPATCH_ALL_TYPES_AND(ScalarType::HALF, ScalarType::BOOL, ...)

you can implement this statically with templates.

Similarly with AT_DISPATCH_ALL_TYPES_AND_COMPLEX -- just add:
AT_DISPATCH_ALL_TYPES_AND_COMPLEX_AND(SCALARTYPE,...)
AT_DISPATCH_ALL_TYPES_AND_COMPLEX_AND(SCALARTYPE1, SCALARTYPE2,...)

@izdeby izdeby force-pushed the BoolTensorCreation_CPU branch from 92129c2 to e60f856 Compare March 7, 2019 01:34
@izdeby izdeby mentioned this pull request Mar 7, 2019
Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

GO GO GO

@izdeby izdeby force-pushed the BoolTensorCreation_CPU branch from e3cc209 to 150373c Compare March 11, 2019 20:18
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.

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 is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

zdevito pushed a commit to zdevito/ATen that referenced this pull request Mar 12, 2019
Summary:
This PR enables bool tensor creation and some basic operations for the CPU backend. This is a part of Bool Tensor feature implementation work. The whole plan looks like this:
    1. Storage Implementation [Done]
    2. Tensor Creation.
        a) CPU (this PR)
        b) CUDA
    3. Tensor Conversions.
    4. Tensor Indexing.
    5. Tensor Operations.
    6. Back compatibility related changes.

**Change**:
Enable CPU tensors and these operations:
- torch.zeros
- torch.tensor
- torch.ones
- torch.randint
- torch.full
- torch.full_like
- torch.empty
- torch.empty_like

**Tested via**:
1) unit tests

2)
torch.zeros(2,2, dtype=torch.bool)
torch.tensor([True, False], dtype=torch.bool)
torch.tensor([-1, -1.1, 0, 1, 1.1, 2], dtype=torch.bool)
torch.ones([1,2], dtype=torch.bool)
torch.randint(10, (2, 2), dtype=torch.bool)
torch.full((2, 3), True, dtype=torch.bool)
torch.empty(4, dtype=torch.bool)

a = torch.tensor([0,0,1])
b = torch.full_like(a, True)
Pull Request resolved: pytorch/pytorch#17376

Reviewed By: ezyang

Differential Revision: D14375995

Pulled By: izdeby

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants