Repository: sympy/sympy
Pull request: Boolean objects ignores set flags in solve
<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->
Fixes #16138 
#### 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://github.com/blog/1506-closing-issues-via-pull-requests . Please also
write a comment on that issue linking back to this pull request once it is
open. -->

List of equations in which one or more have been evaluated to `True` was ignoring the `set flags`. For e.g.

```
In [1]: solve([Eq(x, 0)], [x], dict=True)                                                                                                                     
Out[1]: [{x: 0}]
In [2]: solve([True, Eq(x, 0)], [x], dict=True)                                                                                                               
Out[2]: x = 0
```

But now the output is, 

```
In [1]: solve([Eq(x, 0)], [x], dict=True)                                                                                                                     
Out[1]: [{x: 0}]
In [2]: solve([True, Eq(x, 0)], [x], dict=True)                                                                                                               
Out[2]: [{x: 0}]
```

#### Brief description of what is fixed or changed


#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. 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 -->
NO ENTRY
<!-- END RELEASE NOTES -->


Problem statement:
Solve ignores dict=True when bools are in the input equations
The docstring for solve says:
```
        'dict'=True (default is False)
            return list (perhaps empty) of solution mappings
```

However when given a list of Eqs where one of the Eqs has auto-evaluated to `True` it doesn't return a list of dicts:
```julia
In [1]: solve([True, Eq(x, 0)], [x])                                                                                                                          
Out[1]: x = 0

In [2]: solve([True, Eq(x, 0)], [x], dict=True)                                                                                                               
Out[2]: x = 0

In [3]: solve([Eq(x, 0)], [x], dict=True)                                                                                                                     
Out[3]: [{x: 0}]
```

Hints from issue discussion:
https://github.com/sympy/sympy/blob/0d93891268729373339dddc759f7320c85dc4851/sympy/solvers/solvers.py#L981
I have gone through the code and I find that when bool is present it returns directly without checking other flags so in such case all flag is useless.
I wonder why it does that. That line was add in #13204.

I would think that solve should return an empty list of any equation is `False` and ignore any equation that is `True` but maybe I'm misunderstanding...
Is this related to #13534? (which happens to be when relational operators are involved)
They do look related and both are coming from the same line of code so both issues here can be closed by a single PR I guess.

The difference in this issue is that it shouldn't be calling the inequality solver here. These bools are coming from evaluated Eqs e.g.:
```julia
In [5]: eqs = [Eq(x, x), Eq(x, 1)]                                                                                                                            

In [6]: eqs                                                                                                                                                   
Out[6]: [True, x = 1]

In [7]: solve(eqs)                                                                                                                                            
Out[7]: x = 1

In [8]: solve(eqs, dict=True)                                                                                                                                 
Out[8]: x = 1
```
So #13534 is solved by propagating dict=True through to `reduce_inequalities` whereas this issue is solved by not assuming that a bool represents an inequality.

@oscarbenjamin Why can't we just ignore the `Boolean Values` in solve e.g: 

```
solve([True, Eq(x, 0)], [x], dict=True)
```
is equivalent to 

```
solve([Eq(x, 0)], [x], dict=True) 
```
And also for the issue #13534, propagating only `dict=True` will lead to only checking for dict flag and leaving other flags like 'set` and many more unchecked.
We should ignore Trues in the input. If there is a False then we should return an empty list.

We have to imagine that these bools have come from evaluated Eqs. True means an equation that is trivially true and therefore tells us nothing about any of the variables we are interested in. Those can be ignored. False means that the equation was unsatisfiable: that means it cannot have any solutions.
```julia
In [29]: Eq(x, x+1)                                                                                                                                           
Out[29]: False

In [30]: Eq(x, x)                                                                                                                                             
Out[30]: True
```

Commit message: Added test cases and added check for flags in reduce_inequalities

Patch to review:
diff --git a/sympy/solvers/inequalities.py b/sympy/solvers/inequalities.py
index 5e5d9fb6008c..f1a3fafa8e86 100644
--- a/sympy/solvers/inequalities.py
+++ b/sympy/solvers/inequalities.py
@@ -878,7 +878,7 @@ def classify(ie, s, i):
     conds.append(rv)
     return And(*conds)
 
-def _reduce_inequalities(inequalities, symbols):
+def _reduce_inequalities(inequalities, symbols, **flags):
     # helper for reduce_inequalities
 
     poly_part, abs_part = {}, {}
@@ -929,7 +929,7 @@ def _reduce_inequalities(inequalities, symbols):
     return And(*(poly_reduced + abs_reduced + other))
 
 
-def reduce_inequalities(inequalities, symbols=[]):
+def reduce_inequalities(inequalities, symbols=[], **flags):
     """Reduce a system of inequalities with rational coefficients.
 
     Examples
@@ -985,6 +985,18 @@ def reduce_inequalities(inequalities, symbols=[]):
 
     # solve system
     rv = _reduce_inequalities(inequalities, symbols)
+    as_dict = flags.get('dict', False)
 
-    # restore original symbols and return
-    return rv.xreplace({v: k for k, v in recast.items()})
+    # restore original symbols
+    solution = rv.xreplace({v: k for k, v in recast.items()})
+
+    if not as_dict:
+        return solution
+
+    if isinstance(solution, list):
+        solution = [{s.lhs: s.rhs} for s in solution]
+    else:
+        solution = [{solution.lhs: solution.rhs}]
+
+    if as_dict:
+        return solution
diff --git a/sympy/solvers/solvers.py b/sympy/solvers/solvers.py
index 97b2e3c7075e..967706cc5b2d 100644
--- a/sympy/solvers/solvers.py
+++ b/sympy/solvers/solvers.py
@@ -919,7 +919,8 @@ def _sympified_list(w):
                        )
                       )
     f, symbols = (_sympified_list(w) for w in [f, symbols])
-
+    if isinstance(f, list):
+        f = [s for s in f if s is not S.true and s is not True]
     implicit = flags.get('implicit', False)
 
     # preprocess symbol(s)
@@ -980,7 +981,7 @@ def _sympified_list(w):
             f[i] = fi
 
         if isinstance(fi, (bool, BooleanAtom)) or fi.is_Relational:
-            return reduce_inequalities(f, symbols=symbols)
+            return reduce_inequalities(f, symbols=symbols, **flags)
 
         if isinstance(fi, Poly):
             f[i] = fi.as_expr()
diff --git a/sympy/solvers/tests/test_solvers.py b/sympy/solvers/tests/test_solvers.py
index b71a91aeb8e0..e7e96fbc2ff1 100644
--- a/sympy/solvers/tests/test_solvers.py
+++ b/sympy/solvers/tests/test_solvers.py
@@ -164,6 +164,9 @@ def test_solve_args():
     assert solve([(x + y)**2 - 4, x + y - 2]) == [{x: -y + 2}]
     # - linear
     assert solve((x + y - 2, 2*x + 2*y - 4)) == {x: -y + 2}
+    # When one or more args are Boolean
+    assert solve([True, Eq(x, 0)], [x], dict=True) == [{x: 0}]
+    assert solve([Eq(x, x), Eq(x, 0), Eq(x, x+1)], [x], dict=True) == False
 
 
 def test_solve_polynomial1():


Code review comments:
diff_hunk: @@ -985,6 +985,18 @@ def reduce_inequalities(inequalities, symbols=[]):
 
     # solve system
     rv = _reduce_inequalities(inequalities, symbols)
+    as_dict = flags.get('dict', False)
 
-    # restore original symbols and return
-    return rv.xreplace({v: k for k, v in recast.items()})
+    # restore original symbols
+    solution = rv.xreplace({v: k for k, v in recast.items()})
+
+    if not as_dict:
+        return solution
+
+    if isinstance(solution, list):
+        solution = [{s.lhs: s.rhs} for s in solution]
+    else:
+        solution = [{solution.lhs: solution.rhs}]
+
+    if as_dict:
+        return solution
path: sympy/solvers/inequalities.py
text: @user1:
We don't need the if here

diff_hunk: @@ -878,7 +878,7 @@ def classify(ie, s, i):
     conds.append(rv)
     return And(*conds)
 
-def _reduce_inequalities(inequalities, symbols):
+def _reduce_inequalities(inequalities, symbols, **flags):
path: sympy/solvers/inequalities.py
text: @user1:
`**flags` is not used here. 

@author:
Yes, I'll just remove it in the next commit

diff_hunk: @@ -985,6 +985,18 @@ def reduce_inequalities(inequalities, symbols=[]):
 
     # solve system
     rv = _reduce_inequalities(inequalities, symbols)
+    as_dict = flags.get('dict', False)
path: sympy/solvers/inequalities.py
text: @user1:
Add this as an explicit keyword argument, rather than `**flags`. 

@author:
Well I used `**flags` because maybe there'd be more flags in future which may need to be checked for. So should I change it to `dict` or leave it as **flags

@user2:
Can you change it to an explicit argument? The problem with using `**flags` is that it allows other keyword arguments to be passed without raising an error (I realise that other functions in this module do that but we shouldn't make it worse).

Also can you describe the dict parameter in the doc-string and perhaps add a doctest showing what it does?