{"action":"created","issue":{"url":"https://api.github.com/repos/stan-dev/math/issues/135","labels_url":"https://api.github.com/repos/stan-dev/math/issues/135/labels{/name}","comments_url":"https://api.github.com/repos/stan-dev/math/issues/135/comments","events_url":"https://api.github.com/repos/stan-dev/math/issues/135/events","html_url":"https://github.com/stan-dev/math/pull/135","id":98282116,"number":135,"title":"Feature/issue 125 cholesky derivative","user":{"login":"rtrangucci","id":3925639,"avatar_url":"https://avatars.githubusercontent.com/u/3925639?v=3","gravatar_id":"","url":"https://api.github.com/users/rtrangucci","html_url":"https://github.com/rtrangucci","followers_url":"https://api.github.com/users/rtrangucci/followers","following_url":"https://api.github.com/users/rtrangucci/following{/other_user}","gists_url":"https://api.github.com/users/rtrangucci/gists{/gist_id}","starred_url":"https://api.github.com/users/rtrangucci/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rtrangucci/subscriptions","organizations_url":"https://api.github.com/users/rtrangucci/orgs","repos_url":"https://api.github.com/users/rtrangucci/repos","events_url":"https://api.github.com/users/rtrangucci/events{/privacy}","received_events_url":"https://api.github.com/users/rtrangucci/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":10,"created_at":"2015-07-30T22:58:04Z","updated_at":"2015-08-04T15:21:47Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/stan-dev/math/pulls/135","html_url":"https://github.com/stan-dev/math/pull/135","diff_url":"https://github.com/stan-dev/math/pull/135.diff","patch_url":"https://github.com/stan-dev/math/pull/135.patch"},"body":"#### Summary:\r\n\r\nIntroduces var specialization of cholesky_decompose\r\n\r\n#### Intended Effect:\r\n\r\nWall-time boost of about about 30% to 50% over currently implemented naive autodiff-through-Eigen version, memory use O(D^2) instead of O(D^3).\r\n\r\n#### How to Verify:\r\n\r\n./runTests.py test/unit/math/rev/mat/fun/cholesky_decompose_test.cpp\r\n\r\nruns two gradient tests comparing results to finite diff.\r\n\r\n#### Side Effects:\r\n\r\nNone.\r\n\r\n#### Documentation:\r\n\r\nIncluded in code.\r\n\r\n#### Reviewer Suggestions:\r\n\r\nAnyone"},"comment":{"url":"https://api.github.com/repos/stan-dev/math/issues/comments/127647509","html_url":"https://github.com/stan-dev/math/pull/135#issuecomment-127647509","issue_url":"https://api.github.com/repos/stan-dev/math/issues/135","id":127647509,"user":{"login":"mbrubake","id":3383416,"avatar_url":"https://avatars.githubusercontent.com/u/3383416?v=3","gravatar_id":"","url":"https://api.github.com/users/mbrubake","html_url":"https://github.com/mbrubake","followers_url":"https://api.github.com/users/mbrubake/followers","following_url":"https://api.github.com/users/mbrubake/following{/other_user}","gists_url":"https://api.github.com/users/mbrubake/gists{/gist_id}","starred_url":"https://api.github.com/users/mbrubake/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mbrubake/subscriptions","organizations_url":"https://api.github.com/users/mbrubake/orgs","repos_url":"https://api.github.com/users/mbrubake/repos","events_url":"https://api.github.com/users/mbrubake/events{/privacy}","received_events_url":"https://api.github.com/users/mbrubake/received_events","type":"User","site_admin":false},"created_at":"2015-08-04T15:21:47Z","updated_at":"2015-08-04T15:21:47Z","body":"I had separately given Rob my feedback and suggestions on the vari implementation so I don't really have much to add there. \r\n\r\nOne thing though which is missing is a gradient test which doesn't use the constrained transformation.  The tricky thing about getting the (autodiff) gradients of something like Cholesky correct is how to properly handle the symmetry when propagating the derivatives.  For instance, it's common for the gradient of the upper or lower triangular portion to be zero when it shouldn't be or to be off by a factor of 2.  @rtrangucci can you add tests which don't use ```cov_matrix_constrain``` and instead look at the FD and AD gradients wrt the full matrix?"}}