Repository: sympy/sympy
Pull request: Added test for sin(x + a) - sin(x) (for a)
References to other Issues or PRs
Fixes #10426

Brief description of what is fixed or changed
Here we added tests to address the above issue by comparing the given equation with the specified answer for a. In the solution, we found out a can be dependent on x also, so we covered all the possible solutions in the test. 

Verified on Sympy Live
![Screenshot (38)](https://user-images.githubusercontent.com/45384915/101804676-df44cc00-3b37-11eb-91c1-cc2693860b39.png)

Release Notes
<!-- BEGIN RELEASE NOTES -->
NO ENTRY
<!-- END RELEASE NOTES -->

Problem statement:
Can't solve sin(x + a) - sin(x) (for a)
```
In [9]: solveset(sin(x + a) - sin(x), a)
Out[9]: {a | a ∊ ℂ ∧ -sin(x) + sin(a + x) = 0}

In [10]: solveset(sin(x + a) - sin(x), a, domain=S.Reals)
Out[10]:
⎧              ⎛  ⎛   ⅈ⋅(-a - x)    ⅈ⋅(a + x)⎞  ⅈ⋅x    2⋅ⅈ⋅x    ⎞  -ⅈ⋅x    ⎫
⎪            ⅈ⋅⎝- ⎝- ℯ           + ℯ         ⎠⋅ℯ    + ℯ      - 1⎠⋅ℯ        ⎪
⎨a | a ∊ ℝ ∧ ────────────────────────────────────────────────────────── = 0⎬
⎪                                        2                                 ⎪
⎩                                                                          ⎭

In [11]: solve(sin(x + a) - sin(x), a)
Out[11]: [-x + asin(sin(x)), -x - asin(sin(x)) + π]

```

The solution should be `a=2*pi*n`. I believe the first solution from solve is equivalent to that.

Hints from issue discussion:
For a start, `a = 2*pi*n` is not the complete solution to the equation. Since `sin(x+a) - sin(x) = 2*cos(x+a/2)*sin(a/2)`, `a = 2*pi*n` corresponds only to the part of solution where `sin(a/2) = 0`, whereas `cos(x + a/2) = 0` gives `a = (2*n + 1) * pi - 2*x`.
SymPy seems to give the correct answer if we assume `x` to be real, but fails if it is a general complex variable. Perhaps this is due to the different implementation of `solveset_real` and `solveset_complex`?

``` Python
>>> x = symbols('x', real=True)
... z = symbols('z')
... a = Dummy('a')
...
>>> solveset(sin(x + a) - sin(x), a, domain=S.Reals)
                  ⎧           ⎛  -2⋅ⅈ⋅x⎞        ⎫
{2⋅n⋅π | n ∊ ℤ} ∪ ⎨2⋅n⋅π + arg⎝-ℯ      ⎠ | n ∊ ℤ⎬
                  ⎩                             ⎭
>>> solveset(sin(z + a) - sin(z), a, domain=S.Reals)
⎧              ⎛  ⎛   ⅈ⋅(-a - z)    ⅈ⋅(a + z)⎞  ⅈ⋅z    2⋅ⅈ⋅z    ⎞  -ⅈ⋅z    ⎫
⎪            ⅈ⋅⎝- ⎝- ℯ           + ℯ         ⎠⋅ℯ    + ℯ      - 1⎠⋅ℯ        ⎪
⎨a | a ∊ ℝ ∧ ────────────────────────────────────────────────────────── = 0⎬
⎪                                        2                                 ⎪
⎩                                                                          ⎭
```

Having said that, I believe `solveset` does needs improvement regarding trigonometric equations. For instance it doesn't know how to solve `sin(a/2) = 0`:

``` Python
>>> solveset(sin(a/2), a, domain=S.Reals)
⎧                           -a⋅ⅈ      ⎫
⎪                           ─────     ⎪
⎪               ⎛ a⋅ⅈ    ⎞    2       ⎪
⎨            -ⅈ⋅⎝ℯ    - 1⎠⋅ℯ          ⎬
⎪a | a ∊ ℝ ∧ ───────────────────── = 0⎪
⎪                      2              ⎪
⎩                                     ⎭
```

I think you are right. My goal here is specifically to find a solution that does not depend on x, so as to show that sine is periodic. Hence, although the solutions from solve are probably correct (plus `2*pi*n`) the second one is not useful as written because it is not easy to see that it doesn't depend on x. 

solution should be : 
`a = 4πn, a = 4πn + 2π, a = 4πn + π − 2x, a = 4πn + 3π − 2x`
because using this formula : 
`sin s − sin t = 2cos((s+t)/2)sin((s-t)/2)`
we get this `2cos((2x+a)/2)sin(a/2) = 0`

Getting the solution to reduce to just `2*pi*n` would also be helpful (since that would be the period of sine). This is just a matter of getting `solveset(sin(x))` to give `pi*n` instead of `2*pi*n` and `2*pi*n + pi`.

Currently solveset gives
```
In [62]: solveset(sin(x + a) - sin(x), a)                                                                                                                     
Out[62]: 
                  ⎧   ⎛  ⎛           ⎛  -2⋅ⅈ⋅x⎞⎞          ⎞        ⎫
{2⋅n⋅π | n ∊ ℤ} ∪ ⎨-ⅈ⋅⎝ⅈ⋅⎝2⋅n⋅π + arg⎝-ℯ      ⎠⎠ + 2⋅im(x)⎠ | n ∊ ℤ⎬
                  ⎩                                                
```
The first part of that is that the `2*n*pi` as requested. I guess that the other part shows that the solution depends on x in some way. For example if x is zero then we should have:
```
In [63]: solveset(sin(0 + a) - sin(0), a)                                                                                                                     
Out[63]: {2⋅n⋅π | n ∊ ℤ} ∪ {2⋅n⋅π + π | n ∊ ℤ}
```
which gives `a = n*pi`. The solutions from solveset matches up with this for zero x
```
In [64]: solveset(sin(x + a) - sin(x), a).subs(x, 0)                                                                                                          
Out[64]: {2⋅n⋅π | n ∊ ℤ} ∪ {2⋅n⋅π + π | n ∊ ℤ}
```
It also works for some other values:
```
In [67]: solveset(sin(1 + a) - sin(1), a).simplify()                                                                                                          
Out[67]: {2⋅n⋅π | n ∊ ℤ} ∪ {2⋅n⋅π - 2 + π | n ∊ ℤ}

In [68]: solveset(sin(x + a) - sin(x), a).subs(x, 1).simplify()                                                                                               
Out[68]: {2⋅n⋅π | n ∊ ℤ} ∪ {2⋅n⋅π - 2 + π | n ∊ ℤ}
```
I think this is correct so closing.

This can be closed if a test is added for solveset to prevent future regressions.
A test should also be added for solve
The original motivation for this was for the `periodicity` function. It looks like `periodicity` currently works without making use of solve. 
Hi @asmeurer @oscarbenjamin, I would like to know if this issue is resolved or I can add test cases for solveset. Kindly guide me through the process if it's still open.
I think that the solution is correct although potentially it could be simplified. This issue could be closed if the tests were added.

Commit message: Added test for sin(x + a) - sin(x) (for a)

Patch to review:
diff --git a/sympy/solvers/tests/test_solveset.py b/sympy/solvers/tests/test_solveset.py
index cb91957b79bd..9335f0596773 100644
--- a/sympy/solvers/tests/test_solveset.py
+++ b/sympy/solvers/tests/test_solveset.py
@@ -2472,6 +2472,14 @@ def test_issue_17276():
      FiniteSet((5**(S(1)/5), 25*5**(S(3)/10)))
 
 
+def test_issue_10426():
+    x=Dummy('x')
+    a=Symbol('a')
+    n=Symbol('n')
+    assert solveset(sin(x + a) - sin(x), a) == \
+    Union(ImageSet(Lambda(n, 2*pi*n), S.Integers),Intersection(S.Complexes,ImageSet(Lambda(n, -I*(I*(2*n*pi + arg(-exp(-2*I*x))) + 2*im(x))), S.Integers)))
+
+
 @XFAIL
 def test_substitution_with_infeasible_solution():
     a00, a01, a10, a11, l0, l1, l2, l3, m0, m1, m2, m3, m4, m5, m6, m7, c00, c01, c10, c11, p00, p01, p10, p11 = symbols(


Code review comments:
diff_hunk: @@ -2472,6 +2472,14 @@ def test_issue_17276():
      FiniteSet((5**(S(1)/5), 25*5**(S(3)/10)))
 
 
+def test_issue_10426():
+    x=Dummy('x')
+    a=Symbol('a')
+    n=Symbol('n')
+    assert solveset(sin(x + a) - sin(x), a) == \
+    Union(ImageSet(Lambda(n, 2*pi*n), S.Integers),Intersection(S.Complexes,ImageSet(Lambda(n, -I*(I*(2*n*pi + arg(-exp(-2*I*x))) + 2*im(x))), S.Integers)))
path: sympy/solvers/tests/test_solveset.py
text: @user1:
Would be more readable as
```suggestion
    x = Dummy('x')
    a = Symbol('a')
    n = Symbol('n')
    assert solveset(sin(x + a) - sin(x), a) == Union(
        ImageSet(Lambda(n, 2*pi*n), S.Integers),
        Intersection(S.Complexes, ImageSet(Lambda(n, -I*(I*(2*n*pi + arg(-exp(-2*I*x))) + 2*im(x))), S.Integers)))
```

@author:
Thank you for the suggestion, I tried it running locally, it shows an assertion error. I guess it is coming because of the order of n and pi in both are different. Can you provide me a confirmation on that.