{"action":"created","issue":{"url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8227","repository_url":"https://api.github.com/repos/RobotLocomotion/drake","labels_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8227/labels{/name}","comments_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8227/comments","events_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8227/events","html_url":"https://github.com/RobotLocomotion/drake/pull/8227","id":301883664,"number":8227,"title":"Adds low-level Cache components to ContextBase","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":351649295,"url":"https://api.github.com/repos/RobotLocomotion/drake/labels/team:%20dynamics","name":"team: dynamics","color":"0052cc","default":false},{"id":366761608,"url":"https://api.github.com/repos/RobotLocomotion/drake/labels/team:%20software%20core","name":"team: software core","color":"0052cc","default":false},{"id":77797851,"url":"https://api.github.com/repos/RobotLocomotion/drake/labels/type:%20feature%20request","name":"type: feature request","color":"0e8a16","default":false}],"state":"open","locked":false,"assignee":{"login":"edrumwri","id":2224335,"avatar_url":"https://avatars0.githubusercontent.com/u/2224335?v=4","gravatar_id":"","url":"https://api.github.com/users/edrumwri","html_url":"https://github.com/edrumwri","followers_url":"https://api.github.com/users/edrumwri/followers","following_url":"https://api.github.com/users/edrumwri/following{/other_user}","gists_url":"https://api.github.com/users/edrumwri/gists{/gist_id}","starred_url":"https://api.github.com/users/edrumwri/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/edrumwri/subscriptions","organizations_url":"https://api.github.com/users/edrumwri/orgs","repos_url":"https://api.github.com/users/edrumwri/repos","events_url":"https://api.github.com/users/edrumwri/events{/privacy}","received_events_url":"https://api.github.com/users/edrumwri/received_events","type":"User","site_admin":false},"assignees":[{"login":"edrumwri","id":2224335,"avatar_url":"https://avatars0.githubusercontent.com/u/2224335?v=4","gravatar_id":"","url":"https://api.github.com/users/edrumwri","html_url":"https://github.com/edrumwri","followers_url":"https://api.github.com/users/edrumwri/followers","following_url":"https://api.github.com/users/edrumwri/following{/other_user}","gists_url":"https://api.github.com/users/edrumwri/gists{/gist_id}","starred_url":"https://api.github.com/users/edrumwri/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/edrumwri/subscriptions","organizations_url":"https://api.github.com/users/edrumwri/orgs","repos_url":"https://api.github.com/users/edrumwri/repos","events_url":"https://api.github.com/users/edrumwri/events{/privacy}","received_events_url":"https://api.github.com/users/edrumwri/received_events","type":"User","site_admin":false},{"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}],"milestone":null,"comments":2,"created_at":"2018-03-02T19:47:07Z","updated_at":"2018-03-05T15:10:16Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/RobotLocomotion/drake/pulls/8227","html_url":"https://github.com/RobotLocomotion/drake/pull/8227","diff_url":"https://github.com/RobotLocomotion/drake/pull/8227.diff","patch_url":"https://github.com/RobotLocomotion/drake/pull/8227.patch"},"body":"This is the next big chunk of caching code from #7668. This builds on #8050 (DependencyTrackers) to add the Context side of the Cache. This replaces the Cache and CacheEntryValue classes that were stubbed in #8050 with their complete implementations. These are properly wired up to DependencyTrackers in ContextBase, but are still disconnected from the operational System and Context classes. There is just enough here to allow independent testing of Cache and CacheEntryValue functionality, with much new code and documentation in cache.h,cc and tests in test/cache_test.cc. The rest of the changes (primarily to context_base.h,cc) are just to support the cache tests. However, all the code here is real code from #7668 and should be reviewed; I didn't add any mock infrastructure.\r\n\r\nThis is a large PR (650 loc + comments; about half unit tests) but I think it is a logical subunit of caching that makes sense to review in one PR.\r\n\r\n```\r\nCategory            added  modified  removed  \r\n----------------------------------------------\r\ncode                648    38        20       \r\ncomments            382    28        10       \r\nblank               134    0         1        \r\n----------------------------------------------\r\nTOTAL               1164   66        31       \r\n```\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/8227)\r\n<!-- Reviewable:end -->\r\n"},"comment":{"url":"https://api.github.com/repos/RobotLocomotion/drake/issues/comments/370449497","html_url":"https://github.com/RobotLocomotion/drake/pull/8227#issuecomment-370449497","issue_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8227","id":370449497,"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-03-05T15:10:16Z","updated_at":"2018-03-05T15:10:16Z","author_association":"COLLABORATOR","body":"\n\n\n\nReviewed 2 of 11 files at r1.\nReview status: 8 of 11 files reviewed at latest revision, 6 unresolved discussions.\n\n---\n\n*[systems/framework/context_base.cc, line 104 at r1](https://reviewable.io:443/reviews/robotlocomotion/drake/8227#-L6qefK7kgJDh2v_tsae:-L6qefK7kgJDh2v_tsaf:b-hpdnrz) ([raw file](https://github.com/robotlocomotion/drake/blob/6c933ff2a0e792a5e0a6c5efc3a0d46bb212f54d/systems/framework/context_base.cc#L104)):*\n> ```cpp\n> \n>   // Allocate the \"all sources\" tracker. The complete list of known sources\n>   // is t,a,x,p, and u. Note that cache entries are not included. Under normal\n> ```\n\nFYI I wouldn't mind `t,a,x,p,u` (without the ` and`).\n\n---\n\n*[systems/framework/context_base.cc, line 118 at r1](https://reviewable.io:443/reviews/robotlocomotion/drake/8227#-L6qgRlPI2kdyKkGLTOD:-L6qgRlPI2kdyKkGLTOE:b1eu8zp) ([raw file](https://github.com/robotlocomotion/drake/blob/6c933ff2a0e792a5e0a6c5efc3a0d46bb212f54d/systems/framework/context_base.cc#L118)):*\n> ```cpp\n> \n>   // TODO(sherm1) Add the rest of the built-in trackers here.\n> }\n> ```\n\nI'm not seeing any unit testing that these declarations (notably, the dependency summations) are correct.\n\n---\n\n*[systems/framework/context_base.cc, line 136 at r1](https://reviewable.io:443/reviews/robotlocomotion/drake/8227#-L6qesJ52pWa5kyHqX2L:-L6qesJ6flF7ghy119tT:b-7wpipn) ([raw file](https://github.com/robotlocomotion/drake/blob/6c933ff2a0e792a5e0a6c5efc3a0d46bb212f54d/systems/framework/context_base.cc#L136)):*\n> ```cpp\n>                                &cache_);\n>   // Cache and only needs its back pointers set to this.\n>   cache_.RepairCachePointers(this);\n> ```\n\nThis line is new in this PR (the rest of this method is just relocated code).  If I comment out this line, no unit tests fail.\n\n---\n\n*[systems/framework/context_base.h, line 16 at r1](https://reviewable.io:443/reviews/robotlocomotion/drake/8227#-L6qb0v4_H7X9b6KAsn_:-L6qb0v4_H7X9b6KAsna:bw24qy6) ([raw file](https://github.com/robotlocomotion/drake/blob/6c933ff2a0e792a5e0a6c5efc3a0d46bb212f54d/systems/framework/context_base.h#L16)):*\n> ```objc\n> #include \"drake/systems/framework/cache.h\"\n> #include \"drake/systems/framework/dependency_tracker.h\"\n> #include \"drake/systems/framework/value.h\"\n> ```\n\nNone of the above new includes are used by this header.\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/robotlocomotion/drake/8227)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"jwnimmer-tri"}}