{"action":"created","issue":{"url":"https://api.github.com/repos/RobotLocomotion/drake/issues/4703","repository_url":"https://api.github.com/repos/RobotLocomotion/drake","labels_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/4703/labels{/name}","comments_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/4703/comments","events_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/4703/events","html_url":"https://github.com/RobotLocomotion/drake/pull/4703","id":199305219,"number":4703,"title":"Updated Analytical solution to allow for non-identity quaternion.","user":{"login":"mitiguy","id":21087628,"avatar_url":"https://avatars.githubusercontent.com/u/21087628?v=3","gravatar_id":"","url":"https://api.github.com/users/mitiguy","html_url":"https://github.com/mitiguy","followers_url":"https://api.github.com/users/mitiguy/followers","following_url":"https://api.github.com/users/mitiguy/following{/other_user}","gists_url":"https://api.github.com/users/mitiguy/gists{/gist_id}","starred_url":"https://api.github.com/users/mitiguy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mitiguy/subscriptions","organizations_url":"https://api.github.com/users/mitiguy/orgs","repos_url":"https://api.github.com/users/mitiguy/repos","events_url":"https://api.github.com/users/mitiguy/events{/privacy}","received_events_url":"https://api.github.com/users/mitiguy/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"edrumwri","id":2224335,"avatar_url":"https://avatars.githubusercontent.com/u/2224335?v=3","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://avatars.githubusercontent.com/u/2224335?v=3","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":"sherm1","id":4088016,"avatar_url":"https://avatars.githubusercontent.com/u/4088016?v=3","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}],"milestone":null,"comments":3,"created_at":"2017-01-06T22:21:11Z","updated_at":"2017-01-07T03:18:33Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/RobotLocomotion/drake/pulls/4703","html_url":"https://github.com/RobotLocomotion/drake/pull/4703","diff_url":"https://github.com/RobotLocomotion/drake/pull/4703.diff","patch_url":"https://github.com/RobotLocomotion/drake/pull/4703.patch"},"body":"Need to synchronize with Evan's PR #4604.\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/robotlocomotion/drake/4703)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/RobotLocomotion/drake/issues/comments/271059194","html_url":"https://github.com/RobotLocomotion/drake/pull/4703#issuecomment-271059194","issue_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/4703","id":271059194,"user":{"login":"mitiguy","id":21087628,"avatar_url":"https://avatars.githubusercontent.com/u/21087628?v=3","gravatar_id":"","url":"https://api.github.com/users/mitiguy","html_url":"https://github.com/mitiguy","followers_url":"https://api.github.com/users/mitiguy/followers","following_url":"https://api.github.com/users/mitiguy/following{/other_user}","gists_url":"https://api.github.com/users/mitiguy/gists{/gist_id}","starred_url":"https://api.github.com/users/mitiguy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mitiguy/subscriptions","organizations_url":"https://api.github.com/users/mitiguy/orgs","repos_url":"https://api.github.com/users/mitiguy/repos","events_url":"https://api.github.com/users/mitiguy/events{/privacy}","received_events_url":"https://api.github.com/users/mitiguy/received_events","type":"User","site_admin":false},"created_at":"2017-01-07T03:18:33Z","updated_at":"2017-01-07T03:18:33Z","body":"Much more to do to address current comments.\nSome of which may have to wait until Evan merges fixes to\nMapQDotToVelocity and MapVelocityToQDot.\n\n---\n\nReview status: 0 of 1 files reviewed at latest revision, 13 unresolved discussions.\n\n---\n\n*[drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc, line 81 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/4703#-K_q3cv7gDgsasSAjZvn:-K_qtdm32v4KOX9fd5Ni:b9ld5qy) ([raw file](https://github.com/robotlocomotion/drake/blob/c7489c91dd615327bff7f254283df1d7e366914c/drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc#L81)):*\n<details><summary><i>Previously, sherm1 (Michael Sherman) wrote…</i></summary><blockquote>\n\nBTW, does it matter here whether quatDt was differentiated in A or B?\n</blockquote></details>\n\nDone - removed _B from name quatDt_B\n\n---\n\n*[drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc, line 95 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/4703#-K_q4c6I6B7WgcaBsoWc:-K_qtitbiLzn0hsXkOsc:b-896fix) ([raw file](https://github.com/robotlocomotion/drake/blob/c7489c91dd615327bff7f254283df1d7e366914c/drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc#L95)):*\n<details><summary><i>Previously, sherm1 (Michael Sherman) wrote…</i></summary><blockquote>\n\nBTW, comment should say whether the quaternion must be normalized and if not what are the consequences?\n</blockquote></details>\n\nDone.\n\n---\n\n*[drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc, line 100 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/4703#-K_q4HpS7-Sk5J65m270:-K_qtkUenvXcuOEyQr74:b-896fix) ([raw file](https://github.com/robotlocomotion/drake/blob/c7489c91dd615327bff7f254283df1d7e366914c/drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc#L100)):*\n<details><summary><i>Previously, sherm1 (Michael Sherman) wrote…</i></summary><blockquote>\n\nReturn val not described correctly here (it is a bool).\n</blockquote></details>\n\nDone.\n\n---\n\n*[drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc, line 112 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/4703#-K_qACXE4LVIk9lOpCGD:-K_qtygy6cFAjG-qh8ST:bvngk3e) ([raw file](https://github.com/robotlocomotion/drake/blob/c7489c91dd615327bff7f254283df1d7e366914c/drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc#L112)):*\n<details><summary><i>Previously, sherm1 (Michael Sherman) wrote…</i></summary><blockquote>\n\nBTW, better to use `std::numeric_limits<double>::epsilon()` here since it is standard C++.\n</blockquote></details>\n\nWill update on next round.\n\n---\n\n*[drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc, line 144 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/4703#-K_q4xfBswhqaqDtR5J5:-K_qu4NxkYYIINNvZD8o:bsumaa2) ([raw file](https://github.com/robotlocomotion/drake/blob/c7489c91dd615327bff7f254283df1d7e366914c/drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc#L144)):*\n<details><summary><i>Previously, sherm1 (Michael Sherman) wrote…</i></summary><blockquote>\n\nBTW, says \"expressed in\" but I think the derivative is also \"taken in\" B. Maybe it should just say `InB` here to capture both?\n</blockquote></details>\n\nDone - renamed to Calculate.....FromQuaternionDt()\n\n---\n\n*[drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc, line 357 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/4703#-K_q6LMa75qU5JdNHshz:-K_quFyX0fSa1W70ZlVq:b1w2tkm) ([raw file](https://github.com/robotlocomotion/drake/blob/c7489c91dd615327bff7f254283df1d7e366914c/drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc#L357)):*\n<details><summary><i>Previously, sherm1 (Michael Sherman) wrote…</i></summary><blockquote>\n\nA rotation of pi about the x axis has a lot of zeroes so could mask problems. Can this be some lesser angle about a more-general axis?\n</blockquote></details>\n\nWill be done on next round.\n\n---\n\n*[drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc, line 460 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/4703#-K_qAZQ5zzyycJbtMh9Y:-K_quJKBguvU5QS-jzip:b-3jsnjo) ([raw file](https://github.com/robotlocomotion/drake/blob/c7489c91dd615327bff7f254283df1d7e366914c/drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc#L460)):*\n<details><summary><i>Previously, sherm1 (Michael Sherman) wrote…</i></summary><blockquote>\n\nBTW, better to use `std::numeric_limits<double>::epsilon()` here since it is standard C++.\n</blockquote></details>\n\nWill be done on next round of changes.\n\n---\n\n*[drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc, line 463 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/4703#-K_qAnOBO2pNHiLiegta:-K_quOBaG7ysA96jvYNq:b-xxrvbn) ([raw file](https://github.com/robotlocomotion/drake/blob/c7489c91dd615327bff7f254283df1d7e366914c/drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc#L463)):*\n<details><summary><i>Previously, sherm1 (Michael Sherman) wrote…</i></summary><blockquote>\n\nBTW, could be a tighter tolerance if the quaternions didn't have to be converted to rotation matrices first? Could canonicalize to make sure e0 has same sign in both before comparing. But I guess `1e-14` is still tight enough.\n</blockquote></details>\n\nWill change to compare quaternions directly - after canonicalization (whew cool word - for Popes and code).\n\n---\n\n*[drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc, line 475 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/4703#-K_qCSeaHAf0sQFefIyl:-K_qudiwgBYy5ksp_vhi:b-f5rzfb) ([raw file](https://github.com/robotlocomotion/drake/blob/c7489c91dd615327bff7f254283df1d7e366914c/drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc#L475)):*\n<details><summary><i>Previously, sherm1 (Michael Sherman) wrote…</i></summary><blockquote>\n\nCan you delete these now before they go into the master branch? If not, please at least turn them off.\n</blockquote></details>\n\nDone.  Turned off.  Will remove soon.\n\n---\n\n*[drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc, line 498 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/4703#-K_qCiclIfXe9pVx1l0i:-K_qugukYTofSzrmkN8c:bkd0f1c) ([raw file](https://github.com/robotlocomotion/drake/blob/c7489c91dd615327bff7f254283df1d7e366914c/drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc#L498)):*\n<details><summary><i>Previously, sherm1 (Michael Sherman) wrote…</i></summary><blockquote>\n\nBTW, is this a reasonable tolerance? (~4e-13)\n</blockquote></details>\n\nNo - I need to dig into this more.  We are losing too much precision.  Could be a long dig.\n\n---\n\n*[drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc, line 503 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/4703#-K_qDZcYjGDBm7o1AOA0:-K_quoqkvObQiQfRGy8G:b-j1c05a) ([raw file](https://github.com/robotlocomotion/drake/blob/c7489c91dd615327bff7f254283df1d7e366914c/drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc#L503)):*\n<details><summary><i>Previously, sherm1 (Michael Sherman) wrote…</i></summary><blockquote>\n\nBTW, rather than repeated `50*epsilon` please capture that with a named tolerance, like `significant_tol` or something like that.\n</blockquote></details>\n\nI need to readdress the whole epsilon thing.  For now, I want a test that can pass with the current code.  I'll tighten it as I can make sense of where Drake loses precision.\n\n---\n\n*[drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc, line 521 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/4703#-K_qFiyzn7omlmPVAnju:-K_qv5oyEjlZZr3dhyR8:bmyif0o) ([raw file](https://github.com/robotlocomotion/drake/blob/c7489c91dd615327bff7f254283df1d7e366914c/drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc#L521)):*\n<details><summary><i>Previously, sherm1 (Michael Sherman) wrote…</i></summary><blockquote>\n\nCan you get rid of BadFix now?\n</blockquote></details>\n\nNo.  I need to wait until Evan's merge is in.  The whole point was to create a test that could be used to Debug his code.\nBadFix will be removed and epsilon tightened everywhere.\n\n---\n\n*[drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc, line 525 at r2](https://reviewable.io:443/reviews/robotlocomotion/drake/4703#-K_qFQtV9kDhvu3CNbBv:-K_qvLGkIroG9tpN5pAw:b-xvzanj) ([raw file](https://github.com/robotlocomotion/drake/blob/c7489c91dd615327bff7f254283df1d7e366914c/drake/multibody/benchmarks/cylinder_torque_free_analytical_solution/test/cylinder_torque_free_analytical_solution.cc#L525)):*\n<details><summary><i>Previously, sherm1 (Michael Sherman) wrote…</i></summary><blockquote>\n\nShouldn't this be running both tests?\n</blockquote></details>\n\nYes.  I'll add a TODO.  This was helpful to Evan.\nFor now, the whole thing is commented out.\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/robotlocomotion/drake/4703#-:-K_qvVawihJLrdS1IqVs:bpo3r2c)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"mitiguy"}}