{"action":"created","issue":{"url":"https://api.github.com/repos/allenai/allennlp/issues/518","repository_url":"https://api.github.com/repos/allenai/allennlp","labels_url":"https://api.github.com/repos/allenai/allennlp/issues/518/labels{/name}","comments_url":"https://api.github.com/repos/allenai/allennlp/issues/518/comments","events_url":"https://api.github.com/repos/allenai/allennlp/issues/518/events","html_url":"https://github.com/allenai/allennlp/pull/518","id":275547275,"number":518,"title":"Encoder abstractions","user":{"login":"DeNeutoy","id":16001974,"avatar_url":"https://avatars1.githubusercontent.com/u/16001974?v=4","gravatar_id":"","url":"https://api.github.com/users/DeNeutoy","html_url":"https://github.com/DeNeutoy","followers_url":"https://api.github.com/users/DeNeutoy/followers","following_url":"https://api.github.com/users/DeNeutoy/following{/other_user}","gists_url":"https://api.github.com/users/DeNeutoy/gists{/gist_id}","starred_url":"https://api.github.com/users/DeNeutoy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/DeNeutoy/subscriptions","organizations_url":"https://api.github.com/users/DeNeutoy/orgs","repos_url":"https://api.github.com/users/DeNeutoy/repos","events_url":"https://api.github.com/users/DeNeutoy/events{/privacy}","received_events_url":"https://api.github.com/users/DeNeutoy/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":7,"created_at":"2017-11-21T00:41:43Z","updated_at":"2017-11-29T18:29:36Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/allenai/allennlp/pulls/518","html_url":"https://github.com/allenai/allennlp/pull/518","diff_url":"https://github.com/allenai/allennlp/pull/518.diff","patch_url":"https://github.com/allenai/allennlp/pull/518.patch"},"body":"- Adds a `Seq2StackEncoder` abstraction\r\n- Pulls out the sorting of the batch, forward pass through the model and statefulness for all encoders into a `_EncoderBase` class. \r\n- Refactors `Seq2SeqEncoders` and `Seq2StackEncoders` to allow them to be stateful.\r\n\r\nDepends on #509 \r\n"},"comment":{"url":"https://api.github.com/repos/allenai/allennlp/issues/comments/347952251","html_url":"https://github.com/allenai/allennlp/pull/518#issuecomment-347952251","issue_url":"https://api.github.com/repos/allenai/allennlp/issues/518","id":347952251,"user":{"login":"matt-peters","id":619844,"avatar_url":"https://avatars1.githubusercontent.com/u/619844?v=4","gravatar_id":"","url":"https://api.github.com/users/matt-peters","html_url":"https://github.com/matt-peters","followers_url":"https://api.github.com/users/matt-peters/followers","following_url":"https://api.github.com/users/matt-peters/following{/other_user}","gists_url":"https://api.github.com/users/matt-peters/gists{/gist_id}","starred_url":"https://api.github.com/users/matt-peters/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/matt-peters/subscriptions","organizations_url":"https://api.github.com/users/matt-peters/orgs","repos_url":"https://api.github.com/users/matt-peters/repos","events_url":"https://api.github.com/users/matt-peters/events{/privacy}","received_events_url":"https://api.github.com/users/matt-peters/received_events","type":"User","site_admin":false},"created_at":"2017-11-29T18:29:35Z","updated_at":"2017-11-29T18:29:35Z","author_association":"CONTRIBUTOR","body":"@matt-gardner thanks for the review.  Two thoughts:\r\n\r\n* I wouldn't optimize the code until after benchmarking and confirming it's a bottleneck.  These operations should be cheap in the context of all the other computation needed to compute ELMo.\r\n\r\n* All of the complex stateful stuff isn't needed for ELMo anyway, the single use case we have for stateful now.  If all the sorting/`index_selects` in the stateful piece turns out to be a bottleneck the easiest approach is to just not run it for ELMo... so take that into consideration when deciding how much time to spend on it."}}
{"actor":{"display_login":"matt-peters"}}