Resolve changes in tensor/variable API with PyTorch master by neerajprad · Pull Request #824 · pyro-ppl/pyro

neerajprad · 2018-02-27T01:02:01Z

This resolves a few minor issues so as to keep in sync with PyTorch master.

tensor[0] returns a Variable scalar instead of a python numeric type. We need to use .item() to convert a PyTorch scalar into a python numeric type.
.data returns an instance of type Variable and not torch.Tensor. Instead of using .data.cpu().numpy(), we are now using .detach().cpu().numpy().

NOTE: Tests will fail until we update the PyTorch wheels for travis. The core changes are really small, so would suggest reviewing only once the tests pass.

Only partially resolves #815. We will need more renaming changes later.

neerajprad · 2018-02-27T01:46:29Z

PyTorch wheels are updated; running tests against the updated wheel now

fritzo

Thanks for doing this, Neeraj!

My comments are honest questions, and I'm fine if they result in no changes, only edifying answers 🙂 .

fritzo · 2018-02-27T01:47:16Z

    # misleading, as it incorrectly suggests objects occlude one
    # another.
-    clipped = np.clip(imgarr.data.cpu().numpy(), 0, 1)
+    clipped = np.clip(imgarr.detach().cpu().numpy(), 0, 1)


Is it safe to replace .detach().cpu().numpy() with .cpu().numpy() everywhere?

That's what I started with. If the node has requires_grad=True, it will throw an exception when we convert it to numpy without detaching.

fritzo · 2018-02-27T01:49:09Z

            logits = Variable(logits)
        ix = dist.Categorical(logits=logits).sample()
-        return traces[ix.data[0]]
+        return traces[ix]


Why not ix.item() here? (This may not be perfectly tested)

Because a scalar is a valid index value. We can change it ix.item() but it's not necessary.

This is great! It will be so much easier to write mixture models now.

Completely; scalars will simplify a number of clunky looking stuff.

fritzo · 2018-02-27T01:53:10Z

    (cost + cost.detach() * dist.score_parts(z)[1]).backward()
-    mean_alpha_grad = alphas.grad.data.mean()
-    mean_beta_grad = betas.grad.data.mean()
+    mean_alpha_grad = alphas.grad.data.mean().item()


Why not simply alphas.grad.mean().item()?

Yup, in this case we can just use that. Will change here and other patterns that I see.

fritzo · 2018-02-27T01:59:21Z

@neerajprad has #780 been merged into this branch? I believe that introduces some new helpers like MultiViewTensor that will need to be updated.

fritzo · 2018-02-27T02:07:52Z

Update README.md to recommend installing PyTorch commit 05269b5?

neerajprad · 2018-02-27T02:12:14Z

Update README.md to recommend installing PyTorch commit 05269b5?

Eeks..I always forget that. Thanks for the reminder!

neerajprad · 2018-02-27T02:14:04Z

@neerajprad has #780 been merged into this branch? I believe that introduces some new helpers like MultiViewTensor that will need to be updated.

Thanks for the heads up. Will take a look and update.

neerajprad · 2018-02-27T05:08:12Z

Ready to merge, unless there are any further pending comments.

fehiepsi · 2018-02-27T06:39:42Z

@neerajprad I would like to know about the changes on this. As a summary, we will:

Use .item() to get Python value of a scalar.
Use .tolist() to convert a tensor to Python list.
Use .data to make requires_grad=False. Its difference to .detach() is that: it still uses the same memory as the host tensor.
Use .detach() to separate a tensor from the graph and returns a non-grad tensor. It works like a combination of .data and .clone (.clone alone on a has-grad tensor still requires grad).
Use .numpy() on a non-grad non-cuda tensor to convert it to numpy array. They use the same memory.
No need to use pyro's utils ng_zeros, ng_ones. Use torch.ones, torch.zeros instead.
Use torch.tensor(..., dtype=..., requires_grad=...) to create a tensor with the corresponding type, grad. The previous ways torch.FloatTensor(...), torch.cuda.DoubleTensor(...) also work.
b = torch.tensor(...).type_as(a) is the same as b = torch.tensor(..., dtype=a.dtype).
.sum() always return a scalar tensor.

If the points 3 and 4 are correct, then we should not change a.data.cpu().numpy() to a.detach().cpu().numpy(): no need to make a clone. One of the point to use .detach() is: we don't want numpy array use the same memory as a cpu tensor. However, in many cases of this pull request, we don't have to worry about the memory issue.

neerajprad · 2018-02-27T07:39:14Z

Thanks for the summary, @fehiepsi.

Use .tolist() to convert a tensor to Python list.

Interesting, didn't know about this.

Use .detach() to separate a tensor from the graph and returns a non-grad tensor. It works like a combination of .data and .clone (.clone alone on a has-grad tensor still requires grad).

Is this new behavior, or planned for the future? .detach() used to share the same underlying data, and still does on the PyTorch branch that I am using. This is also what PyTorch recommends in the error that it throws (RuntimeError: Can't call numpy() on Variable that requires grad. Use var.detach().numpy() instead.)

Use torch.tensor(..., dtype=..., requires_grad=...) to create a tensor with the corresponding type, grad. The previous ways torch.FloatTensor(...), torch.cuda.DoubleTensor(...) also work.

Still waiting on these changes in PyTorch master. We will use torch.tensor everywhere once the changes are merged.

fehiepsi · 2018-02-27T08:02:20Z

@neerajprad That it my mistake. :( I will ask on PyTorch's slack what is the difference between x.data and x.detach() and get back to you. They seem identical now. For torch.tensor, it is already committed to Pytorch master. :)

fehiepsi · 2018-02-27T20:09:17Z

@neerajprad As answered by Adam, they are very similar except that x.detach() has some additional checks for in-place operators (I am not clear how these checks work on a detached Variable though). So in my opinion, they can be used interchangeably. :) Btw, with torch.tensor, I think that the interface of pytorch 0.4 is ready to use now (bug fixes and enhancement are still on-going). I expect there will not be many changes affect Pyro (except things related to Distribution, or bugs).

neerajprad added the easy label Feb 27, 2018

neerajprad mentioned this pull request Feb 27, 2018

Remove networkx from Trace #822

Closed

neerajprad added the awaiting review label Feb 27, 2018

neerajprad requested a review from fritzo February 27, 2018 01:45

neerajprad added 2 commits February 26, 2018 17:48

Resolve changes in tensor/variable API with PyTorch master

433ac04

detach and call numpy

0b7ea4a

fritzo reviewed Feb 27, 2018

View reviewed changes

address comments

d353f22

neerajprad force-pushed the pytorch-master branch from 3b8243c to d353f22 Compare February 27, 2018 02:11

update readme

3f8c787

fix broken examples

f8eee29

fritzo approved these changes Feb 27, 2018

View reviewed changes

fritzo merged commit a72f9ac into pyro-ppl:dev Feb 27, 2018

fritzo mentioned this pull request Feb 27, 2018

Adopt strict batch shape semantics for distributions #806

Merged

7 tasks

Uh oh!

Conversation

neerajprad commented Feb 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neerajprad commented Feb 27, 2018

Uh oh!

fritzo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fritzo commented Feb 27, 2018

Uh oh!

fritzo commented Feb 27, 2018

Uh oh!

neerajprad commented Feb 27, 2018

Uh oh!

neerajprad commented Feb 27, 2018

Uh oh!

neerajprad commented Feb 27, 2018

Uh oh!

fehiepsi commented Feb 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neerajprad commented Feb 27, 2018

Uh oh!

fehiepsi commented Feb 27, 2018

Uh oh!

fehiepsi commented Feb 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

neerajprad commented Feb 27, 2018 •

edited

Loading

fehiepsi commented Feb 27, 2018 •

edited

Loading

fehiepsi commented Feb 27, 2018 •

edited

Loading