{"action":"created","issue":{"url":"https://api.github.com/repos/RobotLocomotion/drake/issues/3669","repository_url":"https://api.github.com/repos/RobotLocomotion/drake","labels_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/3669/labels{/name}","comments_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/3669/comments","events_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/3669/events","html_url":"https://github.com/RobotLocomotion/drake/pull/3669","id":180968734,"number":3669,"title":"Add symbolic expression/formula and related classes","user":{"login":"soonho-tri","id":20779767,"avatar_url":"https://avatars.githubusercontent.com/u/20779767?v=3","gravatar_id":"","url":"https://api.github.com/users/soonho-tri","html_url":"https://github.com/soonho-tri","followers_url":"https://api.github.com/users/soonho-tri/followers","following_url":"https://api.github.com/users/soonho-tri/following{/other_user}","gists_url":"https://api.github.com/users/soonho-tri/gists{/gist_id}","starred_url":"https://api.github.com/users/soonho-tri/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/soonho-tri/subscriptions","organizations_url":"https://api.github.com/users/soonho-tri/orgs","repos_url":"https://api.github.com/users/soonho-tri/repos","events_url":"https://api.github.com/users/soonho-tri/events{/privacy}","received_events_url":"https://api.github.com/users/soonho-tri/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"david-german-tri","id":17437069,"avatar_url":"https://avatars.githubusercontent.com/u/17437069?v=3","gravatar_id":"","url":"https://api.github.com/users/david-german-tri","html_url":"https://github.com/david-german-tri","followers_url":"https://api.github.com/users/david-german-tri/followers","following_url":"https://api.github.com/users/david-german-tri/following{/other_user}","gists_url":"https://api.github.com/users/david-german-tri/gists{/gist_id}","starred_url":"https://api.github.com/users/david-german-tri/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/david-german-tri/subscriptions","organizations_url":"https://api.github.com/users/david-german-tri/orgs","repos_url":"https://api.github.com/users/david-german-tri/repos","events_url":"https://api.github.com/users/david-german-tri/events{/privacy}","received_events_url":"https://api.github.com/users/david-german-tri/received_events","type":"User","site_admin":false},"assignees":[{"login":"david-german-tri","id":17437069,"avatar_url":"https://avatars.githubusercontent.com/u/17437069?v=3","gravatar_id":"","url":"https://api.github.com/users/david-german-tri","html_url":"https://github.com/david-german-tri","followers_url":"https://api.github.com/users/david-german-tri/followers","following_url":"https://api.github.com/users/david-german-tri/following{/other_user}","gists_url":"https://api.github.com/users/david-german-tri/gists{/gist_id}","starred_url":"https://api.github.com/users/david-german-tri/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/david-german-tri/subscriptions","organizations_url":"https://api.github.com/users/david-german-tri/orgs","repos_url":"https://api.github.com/users/david-german-tri/repos","events_url":"https://api.github.com/users/david-german-tri/events{/privacy}","received_events_url":"https://api.github.com/users/david-german-tri/received_events","type":"User","site_admin":false},{"login":"jwnimmer-tri","id":17596505,"avatar_url":"https://avatars.githubusercontent.com/u/17596505?v=3","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":33,"created_at":"2016-10-04T18:27:18Z","updated_at":"2016-10-08T02:06:53Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/RobotLocomotion/drake/pulls/3669","html_url":"https://github.com/RobotLocomotion/drake/pull/3669","diff_url":"https://github.com/RobotLocomotion/drake/pull/3669.diff","patch_url":"https://github.com/RobotLocomotion/drake/pull/3669.patch"},"body":"\n\n<!-- Reviewable:start -->\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/robotlocomotion/drake/3669)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/RobotLocomotion/drake/issues/comments/252396058","html_url":"https://github.com/RobotLocomotion/drake/pull/3669#issuecomment-252396058","issue_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/3669","id":252396058,"user":{"login":"jwnimmer-tri","id":17596505,"avatar_url":"https://avatars.githubusercontent.com/u/17596505?v=3","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":"2016-10-08T02:06:53Z","updated_at":"2016-10-08T02:06:53Z","body":"\n\n\n\nReviewed 1 of 6 files at r5.\nReview status: 17 of 18 files reviewed at latest revision, 59 unresolved discussions, some commit checks failed.\n\n---\n\n*[drake/common/test/symbolic_formula_test.cc, line 30 at r5](https://reviewable.io:443/reviews/robotlocomotion/drake/3669#-KTWzm0ivtWuzkla2Jzj:-KTWzm0jJ0DJJtRdL6xP:b-32ife5) ([raw file](https://github.com/robotlocomotion/drake/blob/b542f24bfc43d34e31e7e4d8a7a7fe00db5156b0/drake/common/test/symbolic_formula_test.cc#L30)):*\n> ```cpp\n> \n>   Formula const f1 = (e1 == e1);\n>   EXPECT_TRUE(f1.EqualTo(Formula::True()));\n> ```\n\nBTW For thoroughness (less reliance on glass-box testing) and readability, consider adding `EXPECT_FALSE(f#.EqualTo(Formula::False()))` throughout this test -- and analogously in the below tests.  We are simplifying certain expressions to True or False; testing both options here would have made the concept under test slightly easier for me to understand.\n\n---\n\n*[drake/common/test/symbolic_formula_test.cc, line 41 at r5](https://reviewable.io:443/reviews/robotlocomotion/drake/3669#-KTX0TpfXMulplpijNGi:-KTX0TpgzCMhT5Jhq74a:b-y69aat) ([raw file](https://github.com/robotlocomotion/drake/blob/b542f24bfc43d34e31e7e4d8a7a7fe00db5156b0/drake/common/test/symbolic_formula_test.cc#L41)):*\n> ```cpp\n>   EXPECT_EQ(f3.Evaluate(env), (2 + 3) == (2 + 3));\n>   EXPECT_EQ(f4.Evaluate(env), (2 - 3) == (2 + 3));\n> }\n> ```\n\nFor Eq and all of the other non-constant (non-True, non-False) formula terms, I would like to see more test coverage of structural equality.  At first read, it appears to me that True and False equality is well-covered, but not structural equality of boolean expressions.  This also includes confirming that `f1.EqualTo(f2)` is false for disparate cell types.\n\n---\n\n*[drake/common/test/symbolic_formula_test.cc, line 171 at r5](https://reviewable.io:443/reviews/robotlocomotion/drake/3669#-KTX-bLOSMQ32wLKSxN5:-KTX-bLOSMQ32wLKSxN6:b89wfq) ([raw file](https://github.com/robotlocomotion/drake/blob/b542f24bfc43d34e31e7e4d8a7a7fe00db5156b0/drake/common/test/symbolic_formula_test.cc#L171)):*\n> ```cpp\n>   EXPECT_EQ(f3.Evaluate(env), (2 + 3) >= (2 + 3));\n>   EXPECT_EQ(f4.Evaluate(env), (2 - 3) >= (2 + 3));\n> }\n> ```\n\nGiven the repetitiveness of this test case (and its friends above), I wonder if there is a way to compress the text of the unit tests.  Perhaps at least a base test fixture with the prologue variables and expressions established (as well as the sample environment)?\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/robotlocomotion/drake/3669)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"jwnimmer-tri"}}