{"action":"created","issue":{"url":"https://api.github.com/repos/uber/pyro/issues/1544","repository_url":"https://api.github.com/repos/uber/pyro","labels_url":"https://api.github.com/repos/uber/pyro/issues/1544/labels{/name}","comments_url":"https://api.github.com/repos/uber/pyro/issues/1544/comments","events_url":"https://api.github.com/repos/uber/pyro/issues/1544/events","html_url":"https://github.com/uber/pyro/pull/1544","id":381821068,"node_id":"MDExOlB1bGxSZXF1ZXN0MjMxNzAyMTIw","number":1544,"title":"Revise gp module","user":{"login":"fehiepsi","id":4736342,"node_id":"MDQ6VXNlcjQ3MzYzNDI=","avatar_url":"https://avatars1.githubusercontent.com/u/4736342?v=4","gravatar_id":"","url":"https://api.github.com/users/fehiepsi","html_url":"https://github.com/fehiepsi","followers_url":"https://api.github.com/users/fehiepsi/followers","following_url":"https://api.github.com/users/fehiepsi/following{/other_user}","gists_url":"https://api.github.com/users/fehiepsi/gists{/gist_id}","starred_url":"https://api.github.com/users/fehiepsi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fehiepsi/subscriptions","organizations_url":"https://api.github.com/users/fehiepsi/orgs","repos_url":"https://api.github.com/users/fehiepsi/repos","events_url":"https://api.github.com/users/fehiepsi/events{/privacy}","received_events_url":"https://api.github.com/users/fehiepsi/received_events","type":"User","site_admin":false},"labels":[{"id":704871691,"node_id":"MDU6TGFiZWw3MDQ4NzE2OTE=","url":"https://api.github.com/repos/uber/pyro/labels/awaiting%20review","name":"awaiting review","color":"fbca04","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":4,"created_at":"2018-11-17T02:54:41Z","updated_at":"2018-11-28T17:45:53Z","closed_at":null,"author_association":"COLLABORATOR","pull_request":{"url":"https://api.github.com/repos/uber/pyro/pulls/1544","html_url":"https://github.com/uber/pyro/pull/1544","diff_url":"https://github.com/uber/pyro/pull/1544.diff","patch_url":"https://github.com/uber/pyro/pull/1544.patch"},"body":"This pull request revises gp module. I will sum up all the changes here:\r\n\r\n## Changes in API:\r\n\r\n- Get value of a parameter of Reparameterized class:\r\n```\r\n# previously\r\ngpmodel.get_param(\"noise\")\r\n\r\n# now\r\ngpmodel.noise\r\n```\r\n\r\n- GP model is indeed a `nn.Module` now. So no need to use SVI. If user wants to use SVI, simply use `pyro.module(...)` to cast its parameters to Pyro ParamStoreDict\r\n```\r\n# previously\r\nloss = Trace_ELBO()\r\noptimizer = pyro.optim.Adam({\"lr\": 0.01})\r\nsvi = SVI(gpmodel.model, gpmodel.guide, optimizer, loss)\r\nfor i in range(1000):\r\n    svi.step()\r\n\r\n# now\r\noptimizer = torch.optim.Adam(gpmodel.parameters(), lr=0.01)\r\nloss_fn = Trace_ELBO.differentiable_loss\r\nfor i in range(1000):\r\n    optimizer.zero_grad()\r\n    loss = loss_fn(gpmodel.model, gpmodel.guide)\r\n    loss.backward()\r\n    optimizer.step()\r\n```\r\n\r\n- `gpmodel.optimize()` method is removed. The training step is quite similar to how to train a PyTorch module, so it is good to keep the interface simple. If needed, users can use the helper `contrib.gp.util.train` to quickly train a gpmodel though.\r\n\r\n- No requirement to specify names for kernel/model. The problem of name confliction (for combined kernels) is resolved thanks to `contrib.autoname.name_count` context manager.\r\n\r\n## Notes for reviewers\r\n\r\n- Although there is a lot of file changes, most of them are just a replace from `self.get_param(\"foo\")` to `self.foo`.\r\n\r\n- Reparameterized class has been completely revised. It will no longer interact with Pyro ParamStoreDict. We can still use SVI the following way though it is unnecessary now:\r\n```\r\ndef model():\r\n    pyro.module(gpmodel)\r\n    gpmodel.model()\r\n\r\ndef guide():\r\n    pyro.module(gpmodel)\r\n    gpmodel.guide()\r\n```\r\n\r\n- Experimental support for autoguide. This is required by a user in Pyro Forum a few months back. I also intend to use it in GPLVM tutorial.\r\n\r\n## Todo\r\n\r\n- [x] Use `contrib.autoname`. So no need to define name for each module.\r\n- [x] No need to use `self.get_param(\"foo\")` to get value for parameter `foo` in Reparameterized class, just simply uses `self.foo`. The previous pattern `foo = self.get_param(\"foo\")` is a little bit weird.\r\n- [x] Implement `autoguide` method for parameters in Reparameterized class, which mimics the pattern of `autoguide` module.\r\n- [x] ~Use KL in #1543 by default in `optimize` function. This will help the model converges much faster.~ It seems that `whitening` does a good job for optimization. KL does not help much.\r\n- [x] Fix bugs, tests\r\n- [x] Fix docs\r\n- [x] Fix sv-dkl and gp, bo tutorials"},"comment":{"url":"https://api.github.com/repos/uber/pyro/issues/comments/442539542","html_url":"https://github.com/uber/pyro/pull/1544#issuecomment-442539542","issue_url":"https://api.github.com/repos/uber/pyro/issues/1544","id":442539542,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0MjUzOTU0Mg==","user":{"login":"martinjankowiak","id":22105218,"node_id":"MDQ6VXNlcjIyMTA1MjE4","avatar_url":"https://avatars1.githubusercontent.com/u/22105218?v=4","gravatar_id":"","url":"https://api.github.com/users/martinjankowiak","html_url":"https://github.com/martinjankowiak","followers_url":"https://api.github.com/users/martinjankowiak/followers","following_url":"https://api.github.com/users/martinjankowiak/following{/other_user}","gists_url":"https://api.github.com/users/martinjankowiak/gists{/gist_id}","starred_url":"https://api.github.com/users/martinjankowiak/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/martinjankowiak/subscriptions","organizations_url":"https://api.github.com/users/martinjankowiak/orgs","repos_url":"https://api.github.com/users/martinjankowiak/repos","events_url":"https://api.github.com/users/martinjankowiak/events{/privacy}","received_events_url":"https://api.github.com/users/martinjankowiak/received_events","type":"User","site_admin":false},"created_at":"2018-11-28T17:45:53Z","updated_at":"2018-11-28T17:45:53Z","author_association":"COLLABORATOR","body":"@fehiepsi i'm happy to take a closer look at and clean-up the discussion in gp.ipynb. but let me do that in a follow-up PR after this is merged"}}
{"actor":{"display_login":"martinjankowiak"}}