{"action":"created","issue":{"url":"https://api.github.com/repos/fchollet/keras/issues/1721","repository_url":"https://api.github.com/repos/fchollet/keras","labels_url":"https://api.github.com/repos/fchollet/keras/issues/1721/labels{/name}","comments_url":"https://api.github.com/repos/fchollet/keras/issues/1721/comments","events_url":"https://api.github.com/repos/fchollet/keras/issues/1721/events","html_url":"https://github.com/fchollet/keras/pull/1721","id":133561185,"number":1721,"title":"Make __call__ work properly for Graph","user":{"login":"neggert","id":1271014,"avatar_url":"https://avatars.githubusercontent.com/u/1271014?v=3","gravatar_id":"","url":"https://api.github.com/users/neggert","html_url":"https://github.com/neggert","followers_url":"https://api.github.com/users/neggert/followers","following_url":"https://api.github.com/users/neggert/following{/other_user}","gists_url":"https://api.github.com/users/neggert/gists{/gist_id}","starred_url":"https://api.github.com/users/neggert/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/neggert/subscriptions","organizations_url":"https://api.github.com/users/neggert/orgs","repos_url":"https://api.github.com/users/neggert/repos","events_url":"https://api.github.com/users/neggert/events{/privacy}","received_events_url":"https://api.github.com/users/neggert/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":5,"created_at":"2016-02-14T17:50:48Z","updated_at":"2016-02-15T21:05:40Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/fchollet/keras/pulls/1721","html_url":"https://github.com/fchollet/keras/pull/1721","diff_url":"https://github.com/fchollet/keras/pull/1721.diff","patch_url":"https://github.com/fchollet/keras/pull/1721.patch"},"body":"Previously, `Graph` inherited `__call__` from `Layer`, which doesn't work as expected. I added a bunch of tests that previously failed, but the basic problem is that this example throws an `UnusedInputError`\r\n\r\n```{python}\r\ngraph = Graph()\r\ngraph.add_input(\"input\", input_shape=(10, ))\r\ngraph.add_node(Dense(10), input=\"input\", name=\"output\", create_output=True)\r\n\r\ninputA = K.placeholder(ndim=2)\r\noutputA = graph(inputA)\r\nfA = K.function([inputA], outputA)\r\n```\r\n\r\nIt also breaks Siamese graphs, since the Siamese implementation relies on `__call__`. This example also throws an `UnusedInputError`\r\n\r\n```{python}\r\ngraph = Graph()\r\ngraph.add_input(\"input\", input_shape=(10, ))\r\ngraph.add_node(Dense(10), input=\"input\", name=\"output\", create_output=True)\r\n\r\nmodel = Graph()\r\nmodel.add_input(\"inputA\", input_shape=(10, ))\r\nmodel.add_input(\"inputB\", input_shape=(10, ))\r\nmodel.add_shared_node(name=\"output\", layer=graph, inputs=[\"inputA\", \"inputB\"],\r\n                      merge_mode=\"cos\", dot_axes=1, create_output=True)\r\nmodel.compile(loss={\"output\": \"binary_crossentropy\"}, optimizer=\"sgd\")\r\n```\r\n\r\nThis ended up requiring a number of improvements\r\n\r\n### Short version\r\n\r\n* Changed implementation of `Layer.__call__`\r\n* Remove overridden `Sequential.__call__` (new inherited implementation works)\r\n* `Layer.set_previous` takes a new parameter to disable rebuild\r\n* Cases that previously set the `previous` attribute manually now use `set_previous`\r\n* Added `clear_previous` methods\r\n* Added `Graph.input_shape` property\r\n* Implemented `Graph.__call__` for multiple inputs\r\n* Implemented `Graph.cache_enabled` property, which passes the property to graph nodes\r\n* Added new tests for `__call__` on `Graph` models\r\n\r\n### Long Version\r\nI changed the implementation of `Layer.__call__`, so that instead of messing with `get_input`, it creates a dummy layer and uses `set_previous` to set the bottom layer's input. This implementation works for normal layers as well as `Sequential` and `Graph`s with only 1 input. The reason that this approach works better is that it respects layers with custom `set_previous` methods (like Graph)\r\n\r\nTo make this work, I ended up adding an option to `Layer.set_previous` to disable the rebuild, so that a layer's input can be changed without re-generating its trainable parameters. This allows us to get rid of a few places where `previous` attributes are set manually, which causes problems for layers that have custom `set_previous` implementations (like Graph).\r\n\r\nI also needed to add `clear_previous` methods all around, add an `input_shape` method to `Graph`, and add an implementation of the `Graph.cache_enabled` that passes the property to all the graph nodes.\r\n\r\nFinally, I added an override implementation of `Graph.__call__` for cases with multiple inputs. I figured as long as I'm fixing it, I may as well make it feature-complete.\r\n\r\nThere are new tests to cover all of the things I fixed. Since there were a few preparatory steps before the main change, I split this into a few commits, but I'd be happy to squash them if you'd prefer."},"comment":{"url":"https://api.github.com/repos/fchollet/keras/issues/comments/184393464","html_url":"https://github.com/fchollet/keras/pull/1721#issuecomment-184393464","issue_url":"https://api.github.com/repos/fchollet/keras/issues/1721","id":184393464,"user":{"login":"fchollet","id":710255,"avatar_url":"https://avatars.githubusercontent.com/u/710255?v=3","gravatar_id":"","url":"https://api.github.com/users/fchollet","html_url":"https://github.com/fchollet","followers_url":"https://api.github.com/users/fchollet/followers","following_url":"https://api.github.com/users/fchollet/following{/other_user}","gists_url":"https://api.github.com/users/fchollet/gists{/gist_id}","starred_url":"https://api.github.com/users/fchollet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fchollet/subscriptions","organizations_url":"https://api.github.com/users/fchollet/orgs","repos_url":"https://api.github.com/users/fchollet/repos","events_url":"https://api.github.com/users/fchollet/events{/privacy}","received_events_url":"https://api.github.com/users/fchollet/received_events","type":"User","site_admin":false},"created_at":"2016-02-15T21:05:40Z","updated_at":"2016-02-15T21:05:40Z","body":"The addition of the `build` argument to `set_previous`, and the addition of `clear_previous`, is not necessary and adds undesirable complexity to the code. You can simply follow the pattern in use for the existing implementation of `__call__` (overriding `get_input`). "}}