Implement torch.broadcast_tensors #10075

zou3519 · 2018-07-31T15:36:58Z

This exposes expand_outplace to python. Fixes #8076. Fixes #10041.

I didn't name it torch.broadcast because numpy.broadcast does something
slightly different (it returns an object with the correct shape
information).

Test Plan: new test_torch, test_autograd tests.

cc @soumith @fritzo

fmassa · 2018-07-31T16:04:44Z

Nice!
I believe the equivalent numpy function is numpy.broadcast_arrays, should we aim at maintaining the same name?

soumith · 2018-07-31T16:12:39Z

yes let's match numpy and do broadcast_arrays

zou3519 · 2018-07-31T16:20:36Z

Should it be named "broadcast_arrays" or "broadcast_tensors"? @fmassa and I were thinking broadcast_tensors because we call our data "tensors" but numpy calls their data "arrays".

I'll also try to implement varargs for this via a python wrapper function, it shouldn't be too bad.

fritzo · 2018-07-31T16:30:57Z

Nice! The additional behavior of torch.distributions.utils.broadcast_all() is to handle python floats. I believe we can update broadcast_all() to use your broadcast_arrays() as follows (in either this PR or a follow-up PR):

# in torch/distributions/utils.py
def broadcast_all(*values):
    """docstring"""
    if not all(map(torch.is_tensor, values)):
        # promote floats to tensors
        new_tensor = torch.tensor
        for value in values:
            if torch.is_tensor(value):
                new_tensor = value.new_tensor
                break
        values = [v if torch.is_tensor(v) else new_tensor(v) for v in values]
    return torch.broadcast_arrays(*values)

This would also be a great test to see that distributions are compatible with the new version 😄

vadimkantorov · 2018-07-31T16:47:15Z

Or even put the scalar broadcasting in torch.broadcast_all/arrays? :) If torch.where uses it, it would automatically solve the torch.where not accepting scalars.

fmassa · 2018-07-31T17:36:37Z

@vadimkantorov I think there was a discussion around using torch.as_tensor (or something equivalent) in the beginning of every PyTorch function, so that it also works for numbers and numpy arrays.
Not sure what's the status of that. @gchanan did we decide on something about that?

This exposes expand_outplace to python. Fixes pytorch#8076. Fixes pytorch#10041. I didn't name it torch.broadcast because numpy.broadcast does something slightly different (it returns an object with the correct shape information). Test Plan: new test_torch, test_autograd tests.

- s/broadcast_all/broadcast_tensors/ - broadcast_tensors now takes varargs

zou3519 · 2018-08-01T18:58:03Z

This should be good for review, despite the hanging tests.

I updated the following:

renamed torch.broadcast_all to torch.broadcast_tensors (@fmassa)
Changed torch.distributions.utils.broadcast_all() to use torch.broadcast_tensors (@fritzo). All the distribution tests pass so it looks like it's good to go :)

torch/distributions/utils.py

-    return values
+    if not all(map(torch.is_tensor, values)):
+        # promote numbers to tensors of dtype torch.get_default_dtype()
+        def default_promotion(v):


torch/distributions/utils.py

-    scalar_idxs = [i for i in range(len(values)) if isinstance(values[i], Number)]
-    tensor_idxs = [i for i in range(len(values)) if values[i].__class__.__name__ == 'Tensor']
-    if len(scalar_idxs) + len(tensor_idxs) != len(values):
+    if not all(map(lambda v: torch.is_tensor(v) or isinstance(v, Number), values)):


facebook-github-bot

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

Summary: This exposes expand_outplace to python. Fixes #8076. Fixes #10041. I didn't name it torch.broadcast because numpy.broadcast does something slightly different (it returns an object with the correct shape information). Pull Request resolved: pytorch/pytorch#10075 Differential Revision: D9125816 Pulled By: zou3519 fbshipit-source-id: ebe17c8bb54a73ec84b8f76ce14aff3e9c56f4d1

fritzo · 2018-08-07T17:10:38Z

This change is great. How can I start to use it? It does not appear to have landed in master yet.

EDIT Sorry, I was pointing to a fork 😊

zou3519 · 2018-08-07T17:34:43Z

It should be on master, the following works for me on a latest checkout:

import torch
x = torch.randn(2, 2)
y = torch.randn(1)
torch.broadcast_tensors(x, y)

Summary: This exposes expand_outplace to python. Fixes pytorch#8076. Fixes pytorch#10041. I didn't name it torch.broadcast because numpy.broadcast does something slightly different (it returns an object with the correct shape information). Pull Request resolved: pytorch#10075 Differential Revision: D9125816 Pulled By: zou3519 fbshipit-source-id: ebe17c8bb54a73ec84b8f76ce14aff3e9c56f4d1

Summary: This uses zou3519's new `torch.broadcast_tensors()` #10075 to make `Categorical.log_prob()` and the `*Normal.__init__()` methods jittable. Previously `.log_prob()` was failing due to calls to `torch._C.infer_size()` with errors like ``` def log_prob(self, value): if self._validate_args: self._validate_sample(value) > value_shape = torch._C._infer_size(value.size(), self.batch_shape) if self.batch_shape else value.size() E RuntimeError: expected int at position 0, but got: Tensor ``` After this change I'm able to jit many more of Pyro's tests. Reviewed By: ezyang Differential Revision: D9477487 Pulled By: apaszke fbshipit-source-id: 5f39b29c6b8fa606ad30b02fefe2dfb618e883d6

Summary: This uses zou3519's new `torch.broadcast_tensors()` pytorch#10075 to make `Categorical.log_prob()` and the `*Normal.__init__()` methods jittable. Previously `.log_prob()` was failing due to calls to `torch._C.infer_size()` with errors like ``` def log_prob(self, value): if self._validate_args: self._validate_sample(value) > value_shape = torch._C._infer_size(value.size(), self.batch_shape) if self.batch_shape else value.size() E RuntimeError: expected int at position 0, but got: Tensor ``` After this change I'm able to jit many more of Pyro's tests. Reviewed By: ezyang Differential Revision: D9477487 Pulled By: apaszke fbshipit-source-id: 5f39b29c6b8fa606ad30b02fefe2dfb618e883d6

zou3519 requested review from apaszke, colesbury, ezyang, gchanan, soumith and zdevito as code owners July 31, 2018 15:36

zou3519 added 2 commits July 31, 2018 11:14

Some changes:

c5e418b

- s/broadcast_all/broadcast_tensors/ - broadcast_tensors now takes varargs

zou3519 force-pushed the pytorch-broadcast branch from 2909f12 to c5e418b Compare July 31, 2018 18:14

zou3519 changed the title ~~Implement torch.broadcast_all~~ Implement torch.broadcast_tensors Jul 31, 2018

zou3519 added 2 commits July 31, 2018 12:04

Reimplement broadcast_all in torch/distributions/utils.

fb6b76d

Python 2 compatibility

f28068f

fritzo reviewed Aug 1, 2018

View reviewed changes

torch/distributions/utils.py Outdated

return values

if not all(map(torch.is_tensor, values)):

# promote numbers to tensors of dtype torch.get_default_dtype()

def default_promotion(v):

This comment was marked as off-topic.

Sign in to view

fritzo reviewed Aug 1, 2018

View reviewed changes

Address nits

de7de5c

fmassa approved these changes Aug 1, 2018

View reviewed changes

soumith approved these changes Aug 1, 2018

View reviewed changes

facebook-github-bot reviewed Aug 1, 2018

View reviewed changes

fritzo approved these changes Aug 1, 2018

View reviewed changes

facebook-github-bot closed this in 6b338c8 Aug 2, 2018

zou3519 deleted the pytorch-broadcast branch August 2, 2018 02:33

soumith mentioned this pull request Aug 2, 2018

[JIT]torch._C._infer_size throws an exception when traced #7996

Closed

fritzo mentioned this pull request Aug 7, 2018

[distributions] Make more distributions jittable #10321

Closed

zou3519 mentioned this pull request Aug 23, 2018

Add itertools.{prod, combinations, combinations_with_replacement} like op to pytorch #9393

Closed

ezyang added the merged label Jun 26, 2019

Implement torch.broadcast_tensors #10075

Implement torch.broadcast_tensors #10075

Uh oh!

Conversation

zou3519 commented Jul 31, 2018

Uh oh!

fmassa commented Jul 31, 2018

Uh oh!

soumith commented Jul 31, 2018

Uh oh!

zou3519 commented Jul 31, 2018

Uh oh!

fritzo commented Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vadimkantorov commented Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fmassa commented Jul 31, 2018

Uh oh!

zou3519 commented Aug 1, 2018

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

fritzo commented Aug 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zou3519 commented Aug 7, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

fritzo commented Jul 31, 2018 •

edited

Loading

vadimkantorov commented Jul 31, 2018 •

edited

Loading

fritzo commented Aug 7, 2018 •

edited

Loading