{"action":"created","issue":{"url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8760","repository_url":"https://api.github.com/repos/RobotLocomotion/drake","labels_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8760/labels{/name}","comments_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8760/comments","events_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8760/events","html_url":"https://github.com/RobotLocomotion/drake/pull/8760","id":320129960,"number":8760,"title":"Upgrades input port handling to system/context base classes.","user":{"login":"sherm1","id":4088016,"avatar_url":"https://avatars1.githubusercontent.com/u/4088016?v=4","gravatar_id":"","url":"https://api.github.com/users/sherm1","html_url":"https://github.com/sherm1","followers_url":"https://api.github.com/users/sherm1/followers","following_url":"https://api.github.com/users/sherm1/following{/other_user}","gists_url":"https://api.github.com/users/sherm1/gists{/gist_id}","starred_url":"https://api.github.com/users/sherm1/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sherm1/subscriptions","organizations_url":"https://api.github.com/users/sherm1/orgs","repos_url":"https://api.github.com/users/sherm1/repos","events_url":"https://api.github.com/users/sherm1/events{/privacy}","received_events_url":"https://api.github.com/users/sherm1/received_events","type":"User","site_admin":false},"labels":[{"id":470891318,"url":"https://api.github.com/repos/RobotLocomotion/drake/labels/status:%20curate%20commits%20before%20merging","name":"status: curate commits before merging","color":"d93f0b","default":false}],"state":"open","locked":false,"assignee":{"login":"jwnimmer-tri","id":17596505,"avatar_url":"https://avatars3.githubusercontent.com/u/17596505?v=4","gravatar_id":"","url":"https://api.github.com/users/jwnimmer-tri","html_url":"https://github.com/jwnimmer-tri","followers_url":"https://api.github.com/users/jwnimmer-tri/followers","following_url":"https://api.github.com/users/jwnimmer-tri/following{/other_user}","gists_url":"https://api.github.com/users/jwnimmer-tri/gists{/gist_id}","starred_url":"https://api.github.com/users/jwnimmer-tri/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jwnimmer-tri/subscriptions","organizations_url":"https://api.github.com/users/jwnimmer-tri/orgs","repos_url":"https://api.github.com/users/jwnimmer-tri/repos","events_url":"https://api.github.com/users/jwnimmer-tri/events{/privacy}","received_events_url":"https://api.github.com/users/jwnimmer-tri/received_events","type":"User","site_admin":false},"assignees":[{"login":"jwnimmer-tri","id":17596505,"avatar_url":"https://avatars3.githubusercontent.com/u/17596505?v=4","gravatar_id":"","url":"https://api.github.com/users/jwnimmer-tri","html_url":"https://github.com/jwnimmer-tri","followers_url":"https://api.github.com/users/jwnimmer-tri/followers","following_url":"https://api.github.com/users/jwnimmer-tri/following{/other_user}","gists_url":"https://api.github.com/users/jwnimmer-tri/gists{/gist_id}","starred_url":"https://api.github.com/users/jwnimmer-tri/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jwnimmer-tri/subscriptions","organizations_url":"https://api.github.com/users/jwnimmer-tri/orgs","repos_url":"https://api.github.com/users/jwnimmer-tri/repos","events_url":"https://api.github.com/users/jwnimmer-tri/events{/privacy}","received_events_url":"https://api.github.com/users/jwnimmer-tri/received_events","type":"User","site_admin":false},{"login":"SeanCurtis-TRI","id":22159799,"avatar_url":"https://avatars3.githubusercontent.com/u/22159799?v=4","gravatar_id":"","url":"https://api.github.com/users/SeanCurtis-TRI","html_url":"https://github.com/SeanCurtis-TRI","followers_url":"https://api.github.com/users/SeanCurtis-TRI/followers","following_url":"https://api.github.com/users/SeanCurtis-TRI/following{/other_user}","gists_url":"https://api.github.com/users/SeanCurtis-TRI/gists{/gist_id}","starred_url":"https://api.github.com/users/SeanCurtis-TRI/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SeanCurtis-TRI/subscriptions","organizations_url":"https://api.github.com/users/SeanCurtis-TRI/orgs","repos_url":"https://api.github.com/users/SeanCurtis-TRI/repos","events_url":"https://api.github.com/users/SeanCurtis-TRI/events{/privacy}","received_events_url":"https://api.github.com/users/SeanCurtis-TRI/received_events","type":"User","site_admin":false}],"milestone":null,"comments":36,"created_at":"2018-05-04T01:25:14Z","updated_at":"2018-05-09T23:29:06Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/RobotLocomotion/drake/pulls/8760","html_url":"https://github.com/RobotLocomotion/drake/pull/8760","diff_url":"https://github.com/RobotLocomotion/drake/pull/8760.diff","patch_url":"https://github.com/RobotLocomotion/drake/pull/8760.patch"},"body":"This is the next snippet of code from the caching branch #7668. Changes:\r\n- splits out type-independent InputPortDescriptor code into InputPortBase\r\n- simplifies Context-side handling of input port values to represent only fixed values\r\n- removes unneeded DiagramContext code maintaining duplicative connectivity information\r\n- fills in subsystem names in Context for use in logging and error messages\r\n- eliminates \"freestanding\" to describe fixed-value input ports in favor of \"fixed\" (we were mixing the terms somewhat randomly)\r\n- renames input_port_value.h header to fixed_input_port_value.h. \r\n\r\nMore than half of the changes here are just:\r\n- relocated code, moving handling of input ports up into the type-independent base classes, both on the System and Context sides, and\r\n- renames of Freestanding -> Fixed.\r\n```\r\nCategory            added  modified  removed  \r\n----------------------------------------------\r\ncode                494    109       385      \r\ncomments            269    59        182      \r\nblank               156    0         93       \r\n----------------------------------------------\r\nTOTAL               919    168       660      \r\n```    \r\n<!-- Reviewable:start -->\r\n---\r\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/robotlocomotion/drake/8760)\r\n<!-- Reviewable:end -->\r\n"},"comment":{"url":"https://api.github.com/repos/RobotLocomotion/drake/issues/comments/387905106","html_url":"https://github.com/RobotLocomotion/drake/pull/8760#issuecomment-387905106","issue_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8760","id":387905106,"user":{"login":"jwnimmer-tri","id":17596505,"avatar_url":"https://avatars3.githubusercontent.com/u/17596505?v=4","gravatar_id":"","url":"https://api.github.com/users/jwnimmer-tri","html_url":"https://github.com/jwnimmer-tri","followers_url":"https://api.github.com/users/jwnimmer-tri/followers","following_url":"https://api.github.com/users/jwnimmer-tri/following{/other_user}","gists_url":"https://api.github.com/users/jwnimmer-tri/gists{/gist_id}","starred_url":"https://api.github.com/users/jwnimmer-tri/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jwnimmer-tri/subscriptions","organizations_url":"https://api.github.com/users/jwnimmer-tri/orgs","repos_url":"https://api.github.com/users/jwnimmer-tri/repos","events_url":"https://api.github.com/users/jwnimmer-tri/events{/privacy}","received_events_url":"https://api.github.com/users/jwnimmer-tri/received_events","type":"User","site_admin":false},"created_at":"2018-05-09T23:29:06Z","updated_at":"2018-05-09T23:29:06Z","author_association":"COLLABORATOR","body":"\n\n\n\nReviewed 5 of 8 files at r12.\nReview status: all files reviewed at latest revision, 41 unresolved discussions.\n\n---\n\n*[systems/framework/BUILD.bazel, line 282 at r9](https://reviewable.io/reviews/robotlocomotion/drake/8760#-LC4MRSL6zkXNJPkczE2:-LC6B6YO4cbjqx_p4gpe:bjg0d04) ([raw file](https://github.com/robotlocomotion/drake/blob/e3ec7ae0f046e8202429221dc15126326eae42b8/systems/framework/BUILD.bazel#L282)):*\n<details><summary><i>Previously, SeanCurtis-TRI (Sean Curtis) wrote…</i></summary><blockquote>\n\nFYI This change seems surprising. Is it relevant to this PR? It obviously passes CI.\n</blockquote></details>\n\nOK You were are r9 when you wrote this -- its evolved since then, which was maybe the confusion?\n\n---\n\n*[systems/framework/diagram_context.h, line 172 at r6](https://reviewable.io/reviews/robotlocomotion/drake/8760#-LC1ZjtC0SI8G8p9ZSNv:-LC6Do0h5SVJR5m-orNm:bktgrid) ([raw file](https://github.com/robotlocomotion/drake/blob/ab77e32a3ef3d975f51bc5693e3c768003c40284/systems/framework/diagram_context.h#L172)):*\n<details><summary><i>Previously, sherm1 (Michael Sherman) wrote…</i></summary><blockquote>\n\nOK -- there is no particular relationship because this isn't creating ports any more, just wiring up dependencies. The call doesn't actually make sense without the port index so I'd rather leave it here. Probably the method needs a more-specific name, like SubscribeExportedPortToDiagramPort() but that doesn't make sense here without the guts. My preference is to leave this as-is since it has the right structure and will make it easy for me to fill in the missing bits in the next PR. With the guts to look at we can negotiate a good name for what it's doing.\n</blockquote></details>\n\nPerhaps put (the second half of) that response into a TODO here?\n\n---\n\n*[systems/framework/input_port_descriptor.h, line 36 at r9](https://reviewable.io/reviews/robotlocomotion/drake/8760#-LC59qvyAKIOajqJyGyE:-LC6Cugh-wrKweMT2ZkI:bwy0sns) ([raw file](https://github.com/robotlocomotion/drake/blob/e3ec7ae0f046e8202429221dc15126326eae42b8/systems/framework/input_port_descriptor.h#L36)):*\n<details><summary><i>Previously, SeanCurtis-TRI (Sean Curtis) wrote…</i></summary><blockquote>\n\nBTW Surely this `DRAKE_DEMAND` will never fail. If `system` is null, evaluating the initializer list will throw an exception. Yes?\n</blockquote></details>\n\nOK Nope -- derefencing a nullptr is UB, not an exception.  Is it possible that this DEMAND could still trip, if the compiler just blitted the pointer bits into the reference member bits.\n\n---\n\n*[systems/framework/input_port_descriptor.h, line 37 at r9](https://reviewable.io/reviews/robotlocomotion/drake/8760#-LC5AFPm0QUZzfhYuuyq:-LC6DCAUAYh4qa0drHGS:bpwqi77) ([raw file](https://github.com/robotlocomotion/drake/blob/e3ec7ae0f046e8202429221dc15126326eae42b8/systems/framework/input_port_descriptor.h#L37)):*\n<details><summary><i>Previously, SeanCurtis-TRI (Sean Curtis) wrote…</i></summary><blockquote>\n\nBTW Why not cast it to `SystemBase*`? \n</blockquote></details>\n\nOK We can't, because `System<T>` is forward declared, we don't know (here) thta SystemBase and System are related be inheritance.  (Do you think this needs a comment here saying so?)\n\n---\n\n*[systems/framework/system.h, line 1862 at r9](https://reviewable.io/reviews/robotlocomotion/drake/8760#-LC5EsX01k7kWwEY7RxR:-LC6DP_p7s0ov7YojV3I:b-9j0rd2) ([raw file](https://github.com/robotlocomotion/drake/blob/e3ec7ae0f046e8202429221dc15126326eae42b8/systems/framework/system.h#L1862)):*\n<details><summary><i>Previously, SeanCurtis-TRI (Sean Curtis) wrote…</i></summary><blockquote>\n\nFYI I'd prefer either a single line or curly braces. But this is GSG-compliant.\n</blockquote></details>\n\nFYI In the prior PR, I nagged sherm to do things this way, so that kcov output is more immediately readable to understand branch coverage.  Personally I would always add curlies, but we've decided not to force it one way or another.\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io/reviews/robotlocomotion/drake/8760)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"jwnimmer-tri"}}