{"action":"created","issue":{"url":"https://api.github.com/repos/lihaofd/incubator-mxnet/issues/6","repository_url":"https://api.github.com/repos/lihaofd/incubator-mxnet","labels_url":"https://api.github.com/repos/lihaofd/incubator-mxnet/issues/6/labels{/name}","comments_url":"https://api.github.com/repos/lihaofd/incubator-mxnet/issues/6/comments","events_url":"https://api.github.com/repos/lihaofd/incubator-mxnet/issues/6/events","html_url":"https://github.com/lihaofd/incubator-mxnet/pull/6","id":381000172,"node_id":"MDExOlB1bGxSZXF1ZXN0MjMxMDY4NTU4","number":6,"title":"fix issue when backward after forward(is_train=false)","user":{"login":"lihaofd","id":1567049,"node_id":"MDQ6VXNlcjE1NjcwNDk=","avatar_url":"https://avatars3.githubusercontent.com/u/1567049?v=4","gravatar_id":"","url":"https://api.github.com/users/lihaofd","html_url":"https://github.com/lihaofd","followers_url":"https://api.github.com/users/lihaofd/followers","following_url":"https://api.github.com/users/lihaofd/following{/other_user}","gists_url":"https://api.github.com/users/lihaofd/gists{/gist_id}","starred_url":"https://api.github.com/users/lihaofd/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lihaofd/subscriptions","organizations_url":"https://api.github.com/users/lihaofd/orgs","repos_url":"https://api.github.com/users/lihaofd/repos","events_url":"https://api.github.com/users/lihaofd/events{/privacy}","received_events_url":"https://api.github.com/users/lihaofd/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":6,"created_at":"2018-11-15T04:55:49Z","updated_at":"2018-11-16T09:45:57Z","closed_at":null,"author_association":"OWNER","pull_request":{"url":"https://api.github.com/repos/lihaofd/incubator-mxnet/pulls/6","html_url":"https://github.com/lihaofd/incubator-mxnet/pull/6","diff_url":"https://github.com/lihaofd/incubator-mxnet/pull/6.diff","patch_url":"https://github.com/lihaofd/incubator-mxnet/pull/6.patch"},"body":"## Description ##\r\nIn this PR, it fixed issue about https://github.com/apache/incubator-mxnet/issues/13264\r\n@pengzhao-intel, @TaoLv , @ciyongch \r\n\r\n## Feature changes ##\r\n### New features ###\r\n- When requesting gradients during inference, since there is no allocated space and report warning rather than crash.\r\n\r\n### Unit-test changes ###\r\n- Add testcase robust_check_backward_after_inference in tests/python/unittest/test_operator.py for making sure there is no crash in this case.\r\n\r\n\r\n## Checklist ##\r\n - [X] Passed code style checking (make lint).\r\n - [X] All changes have test coverage.\r\n - [ ] Code is well-documented."},"comment":{"url":"https://api.github.com/repos/lihaofd/incubator-mxnet/issues/comments/439338651","html_url":"https://github.com/lihaofd/incubator-mxnet/pull/6#issuecomment-439338651","issue_url":"https://api.github.com/repos/lihaofd/incubator-mxnet/issues/6","id":439338651,"node_id":"MDEyOklzc3VlQ29tbWVudDQzOTMzODY1MQ==","user":{"login":"taliesinb","id":365157,"node_id":"MDQ6VXNlcjM2NTE1Nw==","avatar_url":"https://avatars1.githubusercontent.com/u/365157?v=4","gravatar_id":"","url":"https://api.github.com/users/taliesinb","html_url":"https://github.com/taliesinb","followers_url":"https://api.github.com/users/taliesinb/followers","following_url":"https://api.github.com/users/taliesinb/following{/other_user}","gists_url":"https://api.github.com/users/taliesinb/gists{/gist_id}","starred_url":"https://api.github.com/users/taliesinb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/taliesinb/subscriptions","organizations_url":"https://api.github.com/users/taliesinb/orgs","repos_url":"https://api.github.com/users/taliesinb/repos","events_url":"https://api.github.com/users/taliesinb/events{/privacy}","received_events_url":"https://api.github.com/users/taliesinb/received_events","type":"User","site_admin":false},"created_at":"2018-11-16T09:45:57Z","updated_at":"2018-11-16T09:45:57Z","author_association":"NONE","body":"@pengzhao-intel no problem! \r\n\r\nHere is some more evidence that the is_train flag is not intended to be used by layers to decide whether they need to compute gradients:\r\n\r\n```\r\nvoid GraphExecutor::RunOps(bool is_train, size_t topo_start, size_t topo_end) {\r\n  // Update context\r\n  const auto& idx = graph_.indexed_graph();\r\n  for (size_t nid = topo_start; nid < topo_end; ++nid) {\r\n    OpNode& opnode = op_nodes_[nid];\r\n    if (opnode.skip_exec_node) continue;\r\n    const auto& inode = idx[nid];\r\n    if (inode.source->is_variable()) continue;\r\n    opnode.exec->op_ctx.is_train = is_train;\r\n    opnode.exec->op_ctx.need_grad = need_grad_;\r\n  }\r\n```\r\nNotice that the `need_grad_` flag is separate from the `is_train` flag. It turns out that `need_grad_` is set in `InitFullGraph` based on whether any gradient arrays had a non-null request type. So that's what we need!\r\n\r\nTherefore the solution I think for the RNN bug straightforward: the check `if (ctx.is_train)` on line 417 of `rnn-inl.h` hould be replaced by the check `if (ctx.need_grad_)`\r\n\r\nSimilar replacements can be made elsewhere too. "}}
{"actor":{"display_login":"taliesinb"}}