{"action":"created","issue":{"url":"https://api.github.com/repos/keras-team/keras/issues/11021","repository_url":"https://api.github.com/repos/keras-team/keras","labels_url":"https://api.github.com/repos/keras-team/keras/issues/11021/labels{/name}","comments_url":"https://api.github.com/repos/keras-team/keras/issues/11021/comments","events_url":"https://api.github.com/repos/keras-team/keras/issues/11021/events","html_url":"https://github.com/keras-team/keras/pull/11021","id":354850807,"node_id":"MDExOlB1bGxSZXF1ZXN0MjExNTE2MDE1","number":11021,"title":"Update the RNN cell API to be explicit about output_size.","user":{"login":"qlzh727","id":5118881,"node_id":"MDQ6VXNlcjUxMTg4ODE=","avatar_url":"https://avatars3.githubusercontent.com/u/5118881?v=4","gravatar_id":"","url":"https://api.github.com/users/qlzh727","html_url":"https://github.com/qlzh727","followers_url":"https://api.github.com/users/qlzh727/followers","following_url":"https://api.github.com/users/qlzh727/following{/other_user}","gists_url":"https://api.github.com/users/qlzh727/gists{/gist_id}","starred_url":"https://api.github.com/users/qlzh727/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/qlzh727/subscriptions","organizations_url":"https://api.github.com/users/qlzh727/orgs","repos_url":"https://api.github.com/users/qlzh727/repos","events_url":"https://api.github.com/users/qlzh727/events{/privacy}","received_events_url":"https://api.github.com/users/qlzh727/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":6,"created_at":"2018-08-28T18:30:16Z","updated_at":"2018-09-01T00:40:55Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/keras-team/keras/pulls/11021","html_url":"https://github.com/keras-team/keras/pull/11021","diff_url":"https://github.com/keras-team/keras/pull/11021.diff","patch_url":"https://github.com/keras-team/keras/pull/11021.patch"},"body":"The existing contract of state_size[0] to be output size is bit\r\nweird and might cause unnecessary contrain for supporting higher\r\ndimension input/output/states. Adding explicit output_size to cell\r\nso that the contract is not inferred by state_size anymore.\r\n\r\nFor backward compat reason, if a cell does not implement\r\noutput_size, the state_size[0] will be used as output_size.\r\n\r\nThe stackedRNNCells is also updated to by default return natural\r\norder state_size as the cells' state, instead of reverse order.\r\nFor any user who rely on the states of the RNN, eg\r\nRNN(return_state=True), please add the extra param\r\nreverse_state_order=True while constructing the StackedRNNCells.\r\n\r\n### Summary\r\n\r\n### Related Issues\r\n\r\n### PR Overview\r\n\r\n- [x] This PR requires new unit tests [y/n] (make sure tests are included)\r\n- [x] This PR requires to update the documentation [y/n] (make sure the docs are up-to-date)\r\n- [x] This PR is backwards compatible [y/n]\r\n- [x] This PR changes the current API [y/n] (all API changes need to be approved by fchollet)\r\n"},"comment":{"url":"https://api.github.com/repos/keras-team/keras/issues/comments/417820320","html_url":"https://github.com/keras-team/keras/pull/11021#issuecomment-417820320","issue_url":"https://api.github.com/repos/keras-team/keras/issues/11021","id":417820320,"node_id":"MDEyOklzc3VlQ29tbWVudDQxNzgyMDMyMA==","user":{"login":"souptc","id":11306809,"node_id":"MDQ6VXNlcjExMzA2ODA5","avatar_url":"https://avatars1.githubusercontent.com/u/11306809?v=4","gravatar_id":"","url":"https://api.github.com/users/souptc","html_url":"https://github.com/souptc","followers_url":"https://api.github.com/users/souptc/followers","following_url":"https://api.github.com/users/souptc/following{/other_user}","gists_url":"https://api.github.com/users/souptc/gists{/gist_id}","starred_url":"https://api.github.com/users/souptc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/souptc/subscriptions","organizations_url":"https://api.github.com/users/souptc/orgs","repos_url":"https://api.github.com/users/souptc/repos","events_url":"https://api.github.com/users/souptc/events{/privacy}","received_events_url":"https://api.github.com/users/souptc/received_events","type":"User","site_admin":false},"created_at":"2018-09-01T00:40:55Z","updated_at":"2018-09-01T00:40:55Z","author_association":"CONTRIBUTOR","body":"Spend some time to debugging, two issues here:\r\n1. In cntk_backend.py, rnn function, I made a mistake when setup the output and states. @qlzh727 , do you mind to change the [this line](url) to\r\n`new_output = n_s[-1]`\r\nthat should help you to pass most cases.\r\n\r\n2. For the new test case 'test_inconsistent_output_state_size' you added, I think you have to disable it for cntk. My understanding is that cntk does not support multiple input/output variables, so we can't handle the case that input and state has different shape. The current \"RNN\" api implementation in cntk_backend is kind of compromise to Keras api, base on the assumption that the last states is the output. "}}
{"actor":{"display_login":"souptc"}}