{"issue":{"created_at":"2011-11-30T19:19:39Z","state":"open","updated_at":"2011-12-07T15:46:14Z","user":{"gravatar_id":"06e8f668ca62cbe8a49b53bede84dec1","id":759243,"url":"https://api.github.com/users/pernici","login":"pernici","avatar_url":"https://secure.gravatar.com/avatar/06e8f668ca62cbe8a49b53bede84dec1?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-140.png"},"comments":28,"body":"`sqrtdenest` does not denest this example (appearing with XFAIL in test_sqrtdenest.py)\r\n\r\n```\r\n>>> sqrtdenest(sqrt(16-2*sqrt(29)+2*sqrt(55-10*sqrt(29))))\r\nsqrt(-2*sqrt(29) + 2*sqrt(-10*sqrt(29) + 55) + 16)\r\n```\r\n\r\nThe reason is that `expr.match(sqrt(a + b * sqrt(r)))` does\r\nnot guarantee that `sqrt(r)` contains the deepest sqrt; on my computer\r\n\r\n```\r\n>>> a, b, r = Wild('a'), Wild('b'), Wild('r')\r\n>>> expr = sqrt(-2*sqrt(29) + 2*sqrt(-10*sqrt(29) + 55) + 16)\r\n>>> expr.match(sqrt(a + b * sqrt(r)))\r\n{r_: 29, b_: -2, a_: 2*sqrt(-10*sqrt(29) + 55) + 16}\r\n```\r\n\r\nI fixed this problem using `sqrt_depth` to find the depth of `sqrt` in an expression.\r\nAdded a simplified radsimp to simplify the result; e.g. in master\r\nthe following example is denested\r\n\r\n```\r\n>>> z=sqrt(-2*sqrt(5) + 2*sqrt(2)*sqrt(-2*sqrt(5) + 11) + 13)\r\n>>> a = sqrtdenest(z); a\r\nsqrt(-16*sqrt(5)/(-8*sqrt(5) + 44) + 88/(-8*sqrt(5) + 44)) + sqrt(-2*sqrt(5) + 11)\r\n```\r\n\r\nbut here one gets `sqrt(2) + sqrt(-2*sqrt(5) + 11)`.\r\n\r\nI also changed `subsets` to use inversed lexicographic ordering, to\r\nguarantee that the case of the fourth root is treated last.\r\nIn the examples I tried this gives sometimes a result more denested, e.g.\r\nin this branch\r\n\r\n```\r\n>>> from sympy import *\r\n>>> z=sqrt(-2*sqrt(10) + 2*sqrt(2)*sqrt(-2*sqrt(10) + 11) + 13)\r\n>>> sqrtdenest(z)\r\n-1 + sqrt(2) + sqrt(10)\r\n```\r\n\r\nReplacing `subsets` with the one used in master one gets\r\n\r\n```\r\nsqrt(-2*sqrt(10) - 2*sqrt(2) + 4*sqrt(5) + 13)\r\n```\r\n\r\n\r\n","assignee":null,"id":2405936,"number":792,"pull_request":{"patch_url":"https://github.com/sympy/sympy/pull/792.patch","html_url":"https://github.com/sympy/sympy/pull/792","diff_url":"https://github.com/sympy/sympy/pull/792.diff"},"labels":[],"title":"sqrt matching in sqrtdenest","html_url":"https://github.com/sympy/sympy/issues/792","closed_at":null,"url":"https://api.github.com/repos/sympy/sympy/issues/792","milestone":null},"action":"created","comment":{"created_at":"2011-12-07T15:46:13Z","updated_at":"2011-12-07T15:46:13Z","user":{"gravatar_id":"06e8f668ca62cbe8a49b53bede84dec1","id":759243,"url":"https://api.github.com/users/pernici","login":"pernici","avatar_url":"https://secure.gravatar.com/avatar/06e8f668ca62cbe8a49b53bede84dec1?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-140.png"},"body":"> btw, this branch needs rebasing -- not sure if you've been checking back here or not.\r\n\r\nWhat should I do to rebase?\r\n\r\n","id":3048690,"url":"https://api.github.com/repos/sympy/sympy/issues/comments/3048690"}}