Repository: sympy/sympy
Pull request: diophantine: make trivial solution satisfy assumptions
Currently, trivial solutions will always be returned regardless of the assumptions of positiveness. An additional check is now added after a trivial solution is appended.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->
Fixes #26783 

#### Brief description of what is fixed or changed
* An assumption check for the trivial solution is added.
* 3 more test cases in the test function.
* Changes have been made to `diop_solve()` to warn users that the function will not filter out infeasible solutions.

#### Other comments
I'm not sure if the additional warning is enough for users who care about the variable assumptions. Maybe we should deprecate the `diop_solve()`, which forces the users to directly use `diophantine()` instead?

#### Release Notes

<!-- Write the release notes for this release below between the BEGIN and END
statements. The basic format is a bulleted list with the name of the subpackage
and the release note for this PR. For example:

* solvers
  * Added a new solver for logarithmic equations.

* functions
  * Fixed a bug with log of integers. Formerly, `log(-x)` incorrectly gave `-log(x)`.

* physics.units
  * Corrected a semantical error in the conversion between volt and statvolt which
    reported the volt as being larger than the statvolt.

or if no release note(s) should be included use:

NO ENTRY

See https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more
information on how to write release notes. The bot will check your release
notes automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* solvers
  * Now `diophantine()` only returns a trivial solution under assumptions.
<!-- END RELEASE NOTES -->


Problem statement:
diophantine and diop_solve ignore assumptions
Using `solvers.diophantine` `diophantine` and `diop_solve` functions with integer and positive assumed symbols, also `{(0, 0)}` and negative solutions are returned.
For example:
```python
from sympy import Symbol
from sympy.solvers.diophantine import diophantine, diop_solve

x = Symbol("x", integer=True, positive=True)
y = Symbol("y", integer=True, positive=True)
eq = 10*x**2 + 5*x*y - 3*y
print(diophantine(eq))
print(diop_solve(eq))
```
prints
```
{(0, 0)}
{(1, -5), (-3, 5), (0, 0)}
```
while it should print at least an empty solution set.

Hints from issue discussion:
It seems currently `diop_quadratic()` will always return a trivial solution if applied when no other solutions are found.
https://github.com/sympy/sympy/blob/3a812204052176854bdfc1ea9d12097c3f9307ca/sympy/solvers/diophantine/diophantine.py#L1479-L1485
Maybe we should rearrange the logic? By the way, you should use `diophantine()` rather than `diop_solve()` since the latter won't filter out the negative solutions. 
I'm not inside `sympy` logic, but if a symbol is subjected to certain assumptions, intuitively I expect that functions I call on that symbols are going to adhere to such assumptions. So yes, probably editing `diophantine()` to make it returns an empty set if the trivial solution is not compatible with assumptions will be a better choice. Regarding `diop_solve()`, as it is an internal use function only, maybe adding in the documentation the warning that it does not necessarily respect assumptions (except the obvious `integer=True`) is the way.

Commit message: diophantine: trivial solution under assumptions

Currently trivial solution will always be returned regardless of
the assumptions of positiveness. An additional check is now added
after a trivial solution is appended.

Patch to review:
diff --git a/sympy/solvers/diophantine/diophantine.py b/sympy/solvers/diophantine/diophantine.py
index d02d0689d814..e2cbb57509e5 100644
--- a/sympy/solvers/diophantine/diophantine.py
+++ b/sympy/solvers/diophantine/diophantine.py
@@ -1482,7 +1482,14 @@ def diophantine(eq, param=symbols("t", integer=True), syms=None,
     null = tuple([0]*len(var))
     # if there is no solution, return trivial solution
     if not sols and eq.subs(zip(var, null)).is_zero:
-        sols.add(null)
+        null_feasible = True
+        for val, symb in zip(null, var):
+            if check_assumptions(val, **symb.assumptions0) is False:
+                null_feasible = False
+                break
+        if null_feasible:
+            sols.add(null)
+
     final_soln = set()
     for sol in sols:
         if all(int_valued(s) for s in sol):
@@ -1558,7 +1565,8 @@ def diop_solve(eq, param=symbols("t", integer=True)):
 
     Use of ``diophantine()`` is recommended over other helper functions.
     ``diop_solve()`` can return either a set or a tuple depending on the
-    nature of the equation.
+    nature of the equation. Solutions which DO NOT satisfy the assumptions
+    such as `positive=True` are returned as well.
 
     Usage
     =====
diff --git a/sympy/solvers/diophantine/tests/test_diophantine.py b/sympy/solvers/diophantine/tests/test_diophantine.py
index a500903fc06f..e36c4c9ad165 100644
--- a/sympy/solvers/diophantine/tests/test_diophantine.py
+++ b/sympy/solvers/diophantine/tests/test_diophantine.py
@@ -752,6 +752,18 @@ def test_assumptions():
     diof = diophantine(a*b + 2*a + 3*b - 6)
     assert diof == {(-15, -3), (-9, -4), (-7, -5), (-6, -6), (-5, -8), (-4, -14)}
 
+    x, y = symbols('x y', integer=True)
+    diof = diophantine(10*x**2 + 5*x*y - 3*y)
+    assert diof == {(1, -5), (-3, 5), (0, 0)}
+
+    x, y = symbols('x y', integer=True, positive=True)
+    diof = diophantine(10*x**2 + 5*x*y - 3*y)
+    assert diof == set()
+
+    x, y = symbols('x y', integer=True, negative=False)
+    diof = diophantine(10*x**2 + 5*x*y - 3*y)
+    assert diof == {(0, 0)}
+
 
 def check_solutions(eq):
     """


Code review comments:
diff_hunk: @@ -1482,7 +1482,14 @@ def diophantine(eq, param=symbols("t", integer=True), syms=None,
     null = tuple([0]*len(var))
     # if there is no solution, return trivial solution
     if not sols and eq.subs(zip(var, null)).is_zero:
-        sols.add(null)
+        null_feasible = True
+        for val, symb in zip(null, var):
+            if check_assumptions(val, **symb.assumptions0) is False:
+                null_feasible = False
+                break
+        if null_feasible:
path: sympy/solvers/diophantine/diophantine.py
text: @user1:
```suggestion
        if all(check_assumptions(v, **s.assumptions0) is not False for v, s in zip(mull, var)):
```