{"action":"created","issue":{"url":"https://api.github.com/repos/tensorflow/tensorflow/issues/5813","repository_url":"https://api.github.com/repos/tensorflow/tensorflow","labels_url":"https://api.github.com/repos/tensorflow/tensorflow/issues/5813/labels{/name}","comments_url":"https://api.github.com/repos/tensorflow/tensorflow/issues/5813/comments","events_url":"https://api.github.com/repos/tensorflow/tensorflow/issues/5813/events","html_url":"https://github.com/tensorflow/tensorflow/pull/5813","id":191326054,"number":5813,"title":"Fix a bug which repeated variable creation in optimizer when using bucketing","user":{"login":"Syndrome777","id":6788909,"avatar_url":"https://avatars.githubusercontent.com/u/6788909?v=3","gravatar_id":"","url":"https://api.github.com/users/Syndrome777","html_url":"https://github.com/Syndrome777","followers_url":"https://api.github.com/users/Syndrome777/followers","following_url":"https://api.github.com/users/Syndrome777/following{/other_user}","gists_url":"https://api.github.com/users/Syndrome777/gists{/gist_id}","starred_url":"https://api.github.com/users/Syndrome777/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Syndrome777/subscriptions","organizations_url":"https://api.github.com/users/Syndrome777/orgs","repos_url":"https://api.github.com/users/Syndrome777/repos","events_url":"https://api.github.com/users/Syndrome777/events{/privacy}","received_events_url":"https://api.github.com/users/Syndrome777/received_events","type":"User","site_admin":false},"labels":[{"id":404586594,"url":"https://api.github.com/repos/tensorflow/tensorflow/labels/awaiting%20googler","name":"awaiting googler","color":"b60205","default":false},{"id":419840263,"url":"https://api.github.com/repos/tensorflow/tensorflow/labels/awaiting%20testing%20(then%20merge)","name":"awaiting testing (then merge)","color":"c2e0c6","default":false},{"id":300136587,"url":"https://api.github.com/repos/tensorflow/tensorflow/labels/cla:%20yes","name":"cla: yes","color":"009800","default":false}],"state":"open","locked":false,"assignee":{"login":"lukaszkaiser","id":684901,"avatar_url":"https://avatars.githubusercontent.com/u/684901?v=3","gravatar_id":"","url":"https://api.github.com/users/lukaszkaiser","html_url":"https://github.com/lukaszkaiser","followers_url":"https://api.github.com/users/lukaszkaiser/followers","following_url":"https://api.github.com/users/lukaszkaiser/following{/other_user}","gists_url":"https://api.github.com/users/lukaszkaiser/gists{/gist_id}","starred_url":"https://api.github.com/users/lukaszkaiser/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lukaszkaiser/subscriptions","organizations_url":"https://api.github.com/users/lukaszkaiser/orgs","repos_url":"https://api.github.com/users/lukaszkaiser/repos","events_url":"https://api.github.com/users/lukaszkaiser/events{/privacy}","received_events_url":"https://api.github.com/users/lukaszkaiser/received_events","type":"User","site_admin":false},"assignees":[{"login":"lukaszkaiser","id":684901,"avatar_url":"https://avatars.githubusercontent.com/u/684901?v=3","gravatar_id":"","url":"https://api.github.com/users/lukaszkaiser","html_url":"https://github.com/lukaszkaiser","followers_url":"https://api.github.com/users/lukaszkaiser/followers","following_url":"https://api.github.com/users/lukaszkaiser/following{/other_user}","gists_url":"https://api.github.com/users/lukaszkaiser/gists{/gist_id}","starred_url":"https://api.github.com/users/lukaszkaiser/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lukaszkaiser/subscriptions","organizations_url":"https://api.github.com/users/lukaszkaiser/orgs","repos_url":"https://api.github.com/users/lukaszkaiser/repos","events_url":"https://api.github.com/users/lukaszkaiser/events{/privacy}","received_events_url":"https://api.github.com/users/lukaszkaiser/received_events","type":"User","site_admin":false}],"milestone":null,"comments":19,"created_at":"2016-11-23T16:48:44Z","updated_at":"2016-11-30T07:36:51Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/tensorflow/tensorflow/pulls/5813","html_url":"https://github.com/tensorflow/tensorflow/pull/5813","diff_url":"https://github.com/tensorflow/tensorflow/pull/5813.diff","patch_url":"https://github.com/tensorflow/tensorflow/pull/5813.patch"},"body":"Hi, all,\r\n\r\nIt's a pull request mentioned in [#5786](https://github.com/tensorflow/tensorflow/issues/5786).\r\n\r\nWhen we use adam or momentum as optimizer in bucketing, the variables in optimizer will not be shared between different buckets.\r\n\r\nBecause it uses `name_scope` to create scope name (and potentially add a suffix to the scope name if a variable with such name already exists) and uses `Variable` to create variable.\r\nSee [_create_slot_var](https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/training/slot_creator.py#L47) and [create_slot](https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/training/slot_creator.py#L67).\r\n```python\r\ndef _create_slot_var(primary, val, scope):\r\n  \"\"\"Helper function for creating a slot variable.\"\"\"\r\n  slot = variables.Variable(val, name=scope, trainable=False)\r\n  ...\r\n  return slot\r\n\r\n\r\ndef create_slot(primary, val, name, colocate_with_primary=True):\r\n  with ops.name_scope(primary.op.name + \"/\" + name) as scope:\r\n    if colocate_with_primary:\r\n      with ops.colocate_with(primary):\r\n        return _create_slot_var(primary, val, scope)\r\n    else:\r\n      return _create_slot_var(primary, val, scope)\r\n```\r\n\r\nBut there is a potential bug when using bucketing in seq2seq task. \r\nEvery bucket will create a list of variables in its optimizer and they should be shared with each other. But now these variables will not be shared.\r\nDiscussion is [#5786](https://github.com/tensorflow/tensorflow/issues/5786).\r\n\r\nFor example, \r\nwhen I use two buckets and adam optimizer in my task, it will create two repeated variable lists:\r\n```\r\nembedding_attention_seq2seq/RNN/EmbeddingWrapper/embedding/Adam:0 (50000, 256)\r\nembedding_attention_seq2seq/RNN/EmbeddingWrapper/embedding/Adam_1:0 (50000, 256)\r\nembedding_attention_seq2seq/RNN/MultiRNNCell/Cell0/GRUCell/Gates/Linear/Matrix/Adam:0 (1056, 1600)\r\nembedding_attention_seq2seq/RNN/MultiRNNCell/Cell0/GRUCell/Gates/Linear/Matrix/Adam_1:0 (1056, 1600)\r\nembedding_attention_seq2seq/RNN/MultiRNNCell/Cell0/GRUCell/Gates/Linear/Bias/Adam:0 (1600,)\r\nembedding_attention_seq2seq/RNN/MultiRNNCell/Cell0/GRUCell/Gates/Linear/Bias/Adam_1:0 (1600,)\r\nembedding_attention_seq2seq/RNN/EmbeddingWrapper/embedding/Adam_2:0 (50000, 256)\r\nembedding_attention_seq2seq/RNN/EmbeddingWrapper/embedding/Adam_3:0 (50000, 256)\r\nembedding_attention_seq2seq/RNN/MultiRNNCell/Cell0/GRUCell/Gates/Linear/Matrix/Adam_2:0 (1056, 1600)\r\nembedding_attention_seq2seq/RNN/MultiRNNCell/Cell0/GRUCell/Gates/Linear/Matrix/Adam_3:0 (1056, 1600)\r\nembedding_attention_seq2seq/RNN/MultiRNNCell/Cell0/GRUCell/Gates/Linear/Bias/Adam_2:0 (1600,)\r\nembedding_attention_seq2seq/RNN/MultiRNNCell/Cell0/GRUCell/Gates/Linear/Bias/Adam_3:0 (1600,)\r\n```\r\nAs a solution, I remove `name_scope` and replace `Variable` to `get_variable` for sharing variables in bucketing.\r\n\r\nAnd meanwhile adam optimizer will create two different lists of variables for parameter update.\r\n[Adam](https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/training/adam.py#L118)\r\n```python\r\n    for v in var_list:\r\n      self._zeros_slot(v, \"m\", self._name)\r\n      self._zeros_slot(v, \"v\", self._name)\r\n```\r\nAnd we have to distinguish the creations of these two variables' lists.\r\nSo I think add `slot_name` as a suffix to `op_name` is a suitable solution. It use `op_name + '/' + slot_name` to create different shared variables.\r\n\r\n```python\r\n  def _zeros_slot(self, var, slot_name, op_name):\r\n    named_slots = self._slot_dict(slot_name)\r\n    if var not in named_slots:\r\n      named_slots[var] = slot_creator.create_zeros_slot(var, op_name)\r\n    return named_slots[var]\r\n```\r\nSo I add `op_name += '/' + slot_name` in [Optimizer._zeros_slot](https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/training/optimizer.py#L561\r\n) and [Optimizer._zeros_slot](https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/training/optimizer.py#L579\r\n).\r\n\r\n## Testing\r\nI have run the tests in [training](https://github.com/tensorflow/tensorflow/tree/master/tensorflow/python/training) and there are all OK.\r\nAnd I have run my code, and it seems that it works well.\r\nAnd I have run the example of mnist, the log is below:\r\n```\r\n#Result of MNIST in tensorflow/models/image/mnist/convolutional.py\r\nStep 8500 (epoch 9.89), 122.8 ms\r\nMinibatch loss: 1.618, learning rate: 0.006302\r\nMinibatch error: 0.0%\r\nValidation error: 0.9%\r\nTest error: 0.8%\r\n```\r\n\r\nI'm not sure if I miss some important things. \r\nAnd please read my pull request and give some advices if it could fix this bug. \r\n\r\nGetting a lot of help from @lukaszkaiser .Thanks so much.\r\n\r\n\r\n\r\n\r\n\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/tensorflow/tensorflow/issues/comments/263803946","html_url":"https://github.com/tensorflow/tensorflow/pull/5813#issuecomment-263803946","issue_url":"https://api.github.com/repos/tensorflow/tensorflow/issues/5813","id":263803946,"user":{"login":"lukaszkaiser","id":684901,"avatar_url":"https://avatars.githubusercontent.com/u/684901?v=3","gravatar_id":"","url":"https://api.github.com/users/lukaszkaiser","html_url":"https://github.com/lukaszkaiser","followers_url":"https://api.github.com/users/lukaszkaiser/followers","following_url":"https://api.github.com/users/lukaszkaiser/following{/other_user}","gists_url":"https://api.github.com/users/lukaszkaiser/gists{/gist_id}","starred_url":"https://api.github.com/users/lukaszkaiser/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lukaszkaiser/subscriptions","organizations_url":"https://api.github.com/users/lukaszkaiser/orgs","repos_url":"https://api.github.com/users/lukaszkaiser/repos","events_url":"https://api.github.com/users/lukaszkaiser/events{/privacy}","received_events_url":"https://api.github.com/users/lukaszkaiser/received_events","type":"User","site_admin":false},"created_at":"2016-11-30T07:36:51Z","updated_at":"2016-11-30T07:36:51Z","body":"Do the tests pass now? Why aren't they completing?"}}
{"actor":{"display_login":"lukaszkaiser"}}