{"action":"created","issue":{"url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8036","repository_url":"https://api.github.com/repos/RobotLocomotion/drake","labels_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8036/labels{/name}","comments_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8036/comments","events_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8036/events","html_url":"https://github.com/RobotLocomotion/drake/pull/8036","id":296234653,"number":8036,"title":"implements fitted value iteration on a barycentric mesh","user":{"login":"RussTedrake","id":6442292,"avatar_url":"https://avatars2.githubusercontent.com/u/6442292?v=4","gravatar_id":"","url":"https://api.github.com/users/RussTedrake","html_url":"https://github.com/RussTedrake","followers_url":"https://api.github.com/users/RussTedrake/followers","following_url":"https://api.github.com/users/RussTedrake/following{/other_user}","gists_url":"https://api.github.com/users/RussTedrake/gists{/gist_id}","starred_url":"https://api.github.com/users/RussTedrake/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/RussTedrake/subscriptions","organizations_url":"https://api.github.com/users/RussTedrake/orgs","repos_url":"https://api.github.com/users/RussTedrake/repos","events_url":"https://api.github.com/users/RussTedrake/events{/privacy}","received_events_url":"https://api.github.com/users/RussTedrake/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"frankpermenter","id":3444097,"avatar_url":"https://avatars2.githubusercontent.com/u/3444097?v=4","gravatar_id":"","url":"https://api.github.com/users/frankpermenter","html_url":"https://github.com/frankpermenter","followers_url":"https://api.github.com/users/frankpermenter/followers","following_url":"https://api.github.com/users/frankpermenter/following{/other_user}","gists_url":"https://api.github.com/users/frankpermenter/gists{/gist_id}","starred_url":"https://api.github.com/users/frankpermenter/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/frankpermenter/subscriptions","organizations_url":"https://api.github.com/users/frankpermenter/orgs","repos_url":"https://api.github.com/users/frankpermenter/repos","events_url":"https://api.github.com/users/frankpermenter/events{/privacy}","received_events_url":"https://api.github.com/users/frankpermenter/received_events","type":"User","site_admin":false},"assignees":[{"login":"frankpermenter","id":3444097,"avatar_url":"https://avatars2.githubusercontent.com/u/3444097?v=4","gravatar_id":"","url":"https://api.github.com/users/frankpermenter","html_url":"https://github.com/frankpermenter","followers_url":"https://api.github.com/users/frankpermenter/followers","following_url":"https://api.github.com/users/frankpermenter/following{/other_user}","gists_url":"https://api.github.com/users/frankpermenter/gists{/gist_id}","starred_url":"https://api.github.com/users/frankpermenter/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/frankpermenter/subscriptions","organizations_url":"https://api.github.com/users/frankpermenter/orgs","repos_url":"https://api.github.com/users/frankpermenter/repos","events_url":"https://api.github.com/users/frankpermenter/events{/privacy}","received_events_url":"https://api.github.com/users/frankpermenter/received_events","type":"User","site_admin":false},{"login":"hongkai-dai","id":6314000,"avatar_url":"https://avatars2.githubusercontent.com/u/6314000?v=4","gravatar_id":"","url":"https://api.github.com/users/hongkai-dai","html_url":"https://github.com/hongkai-dai","followers_url":"https://api.github.com/users/hongkai-dai/followers","following_url":"https://api.github.com/users/hongkai-dai/following{/other_user}","gists_url":"https://api.github.com/users/hongkai-dai/gists{/gist_id}","starred_url":"https://api.github.com/users/hongkai-dai/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hongkai-dai/subscriptions","organizations_url":"https://api.github.com/users/hongkai-dai/orgs","repos_url":"https://api.github.com/users/hongkai-dai/repos","events_url":"https://api.github.com/users/hongkai-dai/events{/privacy}","received_events_url":"https://api.github.com/users/hongkai-dai/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":7,"created_at":"2018-02-11T22:53:57Z","updated_at":"2018-02-12T14:22:57Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/RobotLocomotion/drake/pulls/8036","html_url":"https://github.com/RobotLocomotion/drake/pull/8036","diff_url":"https://github.com/RobotLocomotion/drake/pull/8036.diff","patch_url":"https://github.com/RobotLocomotion/drake/pull/8036.patch"},"body":"![fitted_value_iteration](https://user-images.githubusercontent.com/6442292/36079753-8439e7b2-0f54-11e8-809c-9061dfc17d89.png)\r\n\r\nThe plot on the left is the value iteration solution of the cost-to-go for the LQR problem for the double integrator.  The plot on the right is the LQR solution.  The remaining differences are primarily due to the boundary conditions (in the positive and negative orthants) and the increased cost due to the discretized action space.  \r\n\r\nThe value iteration code uses the full simulation stack, so should be accurate and easily generalizable to more complex systems. \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/8036)\r\n<!-- Reviewable:end -->\r\n"},"comment":{"url":"https://api.github.com/repos/RobotLocomotion/drake/issues/comments/364936443","html_url":"https://github.com/RobotLocomotion/drake/pull/8036#issuecomment-364936443","issue_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8036","id":364936443,"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-02-12T14:22:57Z","updated_at":"2018-02-12T14:22:57Z","author_association":"COLLABORATOR","body":"Checkpoint.\n\n---\n\nReviewed 6 of 9 files at r1, 1 of 2 files at r2.\nReview status: all files reviewed at latest revision, 7 unresolved discussions.\n\n---\n\n*[systems/controllers/dynamic_programming.cc, line 48 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/8036#-L59NGeugp_e_eYdX7xY:-L59NGevRGmbrpOhMu7n:bo3lfwj) ([raw file](https://github.com/robotlocomotion/drake/blob/3f15952f989b18b7c9e796166274887b74d618c3/systems/controllers/dynamic_programming.cc#L48)):*\n> ```cpp\n>   math::BarycentricMesh<double> input_mesh(input_grid);\n> \n>   const int kNumStates = state_mesh.get_num_mesh_points();\n> ```\n\n_META_ Not allowed to use kFoo unless the value is constant for all time (within a single process).  Since these are user inputs and won't be that, we want `const int num_states` etc.\n\n---\n\n*[systems/controllers/dynamic_programming.cc, line 61 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/8036#-L59NWzOzJ54oskUAJe5:-L59NWzOzJ54oskUAJe6:bnjhhp2) ([raw file](https://github.com/robotlocomotion/drake/blob/3f15952f989b18b7c9e796166274887b74d618c3/systems/controllers/dynamic_programming.cc#L61)):*\n> ```cpp\n> \n>   {  // Build transition matrices.\n>     std::cout << \"Computing transition and cost matrices\";\n> ```\n\nBTW Use Drake's logging APIs, or else remove.  Ditto for other couts below.  (The defect is that with cout, the caller can't easily redirect or throttle the output.)\n\n---\n\n*[systems/controllers/dynamic_programming.cc, line 104 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/8036#-L59NmIEDurE32WOF00V:-L59NmIEDurE32WOF00W:bgrzhq2) ([raw file](https://github.com/robotlocomotion/drake/blob/3f15952f989b18b7c9e796166274887b74d618c3/systems/controllers/dynamic_programming.cc#L104)):*\n> ```cpp\n>   }\n> \n>   // Perform value iteration loop\n> ```\n\nBTW punc\n\n---\n\n*[systems/controllers/dynamic_programming.h, line 17 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/8036#-L59ItqZWwW9XTidDIfE:-L59ItqZWwW9XTidDIfF:b-7a9l24) ([raw file](https://github.com/robotlocomotion/drake/blob/3f15952f989b18b7c9e796166274887b74d618c3/systems/controllers/dynamic_programming.h#L17)):*\n> ```objc\n> \n> /// Consolidates the many possible options to be passed to the dynamic\n> /// programming algorithms.\n> ```\n\nI don't see anywhere (either here, or in the FittedValueIteration) that explains the semantics of these three options.  They should be documented, under the \"all non-obvious function arguments require documentation\" rule.\n\nAt least to include:\n- discount_factor in (0, 1].\n\n---\n\n*[systems/controllers/dynamic_programming.h, line 39 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/8036#-L59JeECFCRNB3rfwn-h:-L59JeEDoEDsK1Z3unBY:bixddk7) ([raw file](https://github.com/robotlocomotion/drake/blob/3f15952f989b18b7c9e796166274887b74d618c3/systems/controllers/dynamic_programming.h#L39)):*\n> ```objc\n> /// a Context for that system, which may contain non-default Parameters, etc.\n> /// The @p simulator is run for @p timestep seconds from every point on the mesh\n> /// in order to approximate the dynamics.. all of the simulation parameters\n> ```\n\nBTW Typo `..`?  Seems like `.` or `...` is wanted.\n\n---\n\n*[systems/controllers/dynamic_programming.h, line 48 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/8036#-L59MLtILsjGnaxggauw:-L59MLtILsjGnaxggaux:b-l8rqsz) ([raw file](https://github.com/robotlocomotion/drake/blob/3f15952f989b18b7c9e796166274887b74d618c3/systems/controllers/dynamic_programming.h#L48)):*\n> ```objc\n> /// the cost-to-go function and the resulting policy.\n> /// @param input_grid defines the discrete action space used in the value\n> /// iteraiton update.\n> ```\n\nBTW `iteration` typo.\n\n---\n\n*[systems/controllers/dynamic_programming.h, line 66 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/8036#-L59LtSf2YupCrL25Mvt:-L59LtSf2YupCrL25Mvu:b-g7qde6) ([raw file](https://github.com/robotlocomotion/drake/blob/3f15952f989b18b7c9e796166274887b74d618c3/systems/controllers/dynamic_programming.h#L66)):*\n> ```objc\n>     const math::BarycentricMesh<double>::MeshGrid& state_grid,\n>     const math::BarycentricMesh<double>::MeshGrid& input_grid,\n>     const double timestep,\n> ```\n\n`const` here (in the _declaration_) not allowed:\nhttp://drake.mit.edu/styleguide/cppguide.html#Use_of_const\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/robotlocomotion/drake/8036#-:-L59NpfQxIpq_KaqMQae:by96gdd)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"jwnimmer-tri"}}