sympy / sympy

A computer algebra system written in pure Python
https://sympy.org/
Other
12.93k stars 4.42k forks source link

BooleanFalse object has no attribute 'is_constant' in nonlinsolve #19144

Closed oscarbenjamin closed 3 years ago

oscarbenjamin commented 4 years ago

From stackoverflow: https://stackoverflow.com/questions/61280934/nonlinsolve-in-sympy-giving-imaginary-solutions-when-symbols-are-real

The code

# Create real symbols x_1,x_2,...,x_18
syms = [Symbol('x_{}'.format(i), real=True) for i in range(1,19)]
for i,s in enumerate(syms):
    exec('x_{} = syms[{}]'.format(i+1, i))

# This code respects the real attribute
test = [Eq(eq) for eq in [x_1*x_1+1]]
print(nonlinsolve(test, x_1))

# This code violates the real attribute and gives imaginary solutions
test = [Eq(eq) for eq in [36*x_1 + 15*x_18**3 - 50*x_18**2 + 6*x_18 - 4, 15*x_18**3 - 50*x_18**2 + 114*x_18 + 36*x_2 - 40, -15*x_18**3 + 50*x_18**2 - 54*x_18 + 12*x_3 + 28, x_4 + 1, -x_18 + x_5, x_6 - 1, -15*x_18**3 + 50*x_18**2 - 42*x_18 + 36*x_7 + 4, 15*x_18**3 - 50*x_18**2 + 6*x_18 + 36*x_8 - 4, 15*x_18**3 - 50*x_18**2 + 54*x_18 + 12*x_9 - 16, x_10 - 3*x_18 + 2, x_11 - 1, x_12 + x_18 - 1, 36*x_13 + 15*x_18**3 - 50*x_18**2 + 114*x_18 - 76, x_14 + 1, 18*x_15 - 15*x_18**3 + 50*x_18**2 - 24*x_18 + 4, 6*x_16 + 15*x_18**3 - 50*x_18**2 + 54*x_18 - 22, x_17 + 2*x_18 - 1, 15*x_18**4 - 20*x_18**3 + 14*x_18**2 + 8*x_18 - 8]]
print(nonlinsolve(test, syms))

On stackoverflow the reported problem is that real solutions are shown even though the symbols are declared as real. On master I find that an exception is raised:

   ...: print(nonlinsolve(test, syms))                                                                                            
/Users/enojb/current/sympy/sympy/sympy/core/relational.py:486: SymPyDeprecationWarning: 

Eq(expr) with rhs default to 0 has been deprecated since SymPy 1.5.
Use Eq(expr, 0) instead. See
https://github.com/sympy/sympy/issues/16587 for more info.

  SymPyDeprecationWarning(
FiniteSet((EmptySet,))
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-2-007c56be3ec5> in <module>
     10 # This code violates the real attribute and gives imaginary solutions
     11 test = [Eq(eq) for eq in [36*x_1 + 15*x_18**3 - 50*x_18**2 + 6*x_18 - 4, 15*x_18**3 - 50*x_18**2 + 114*x_18 + 36*x_2 - 40, -15*x_18**3 + 50*x_18**2 - 54*x_18 + 12*x_3 + 28, x_4 + 1, -x_18 + x_5, x_6 - 1, -15*x_18**3 + 50*x_18**2 - 42*x_18 + 36*x_7 + 4, 15*x_18**3 - 50*x_18**2 + 6*x_18 + 36*x_8 - 4, 15*x_18**3 - 50*x_18**2 + 54*x_18 + 12*x_9 - 16, x_10 - 3*x_18 + 2, x_11 - 1, x_12 + x_18 - 1, 36*x_13 + 15*x_18**3 - 50*x_18**2 + 114*x_18 - 76, x_14 + 1, 18*x_15 - 15*x_18**3 + 50*x_18**2 - 24*x_18 + 4, 6*x_16 + 15*x_18**3 - 50*x_18**2 + 54*x_18 - 22, x_17 + 2*x_18 - 1, 15*x_18**4 - 20*x_18**3 + 14*x_18**2 + 8*x_18 - 8]]
---> 12 print(nonlinsolve(test, syms))

~/current/sympy/sympy/sympy/solvers/solveset.py in nonlinsolve(system, *symbols)
   3420         # If all the equations are not polynomial.
   3421         # Use `substitution` method for the system
-> 3422         result = substitution(
   3423             polys_expr + nonpolys, symbols, exclude=denominators)
   3424         return result

~/current/sympy/sympy/sympy/solvers/solveset.py in substitution(system, symbols, result, known_symbols, exclude, all_symbols)
   3085     # end def _solve_using_know_values()
   3086 
-> 3087     new_result_real, solve_call1, cnd_call1 = _solve_using_known_values(
   3088         old_result, solveset_real)
   3089     new_result_complex, solve_call2, cnd_call2 = _solve_using_known_values(

~/current/sympy/sympy/sympy/solvers/solveset.py in _solve_using_known_values(result, solver)
   2982                 if not unsolved_syms:
   2983                     if res:
-> 2984                         newresult, delete_res = _append_new_soln(
   2985                             res, None, None, imgset_yes, soln_imageset,
   2986                             original_imageset, newresult, eq2)

~/current/sympy/sympy/sympy/solvers/solveset.py in _append_new_soln(rnew, sym, sol, imgset_yes, soln_imageset, original_imageset, newresult, eq)
   2917                         eq, newresult, rnew, delete_soln, local_n)
   2918             elif eq is not None:
-> 2919                 newresult, rnew, delete_soln = _append_eq(
   2920                     eq, newresult, rnew, delete_soln)
   2921             elif soln_imageset:

~/current/sympy/sympy/sympy/solvers/solveset.py in _append_eq(eq, result, res, delete_soln, n)
   2872         if n:
   2873             eq = eq.subs(n, 0)
-> 2874         satisfy = checksol(u, u, eq, minimal=True)
   2875         if satisfy is False:
   2876             delete_soln = True

~/current/sympy/sympy/sympy/solvers/solvers.py in checksol(f, symbol, sol, **flags)
    300         elif attempt == 1:
    301             if not val.is_number:
--> 302                 if not val.is_constant(*list(sol.keys()), simplify=not minimal):
    303                     return False
    304                 # there are free symbols -- simple expansion might work

AttributeError: 'BooleanFalse' object has no attribute 'is_constant'
smichr commented 4 years ago

At the end of checksol is the val-handling:

        if val == was:
            continue
        elif val.is_Rational:
            return val == 0
        if numerical and val.is_number:
            return (abs(val.n(18).n(12, chop=True)) < 1e-9) is S.true
        was = val

A test for a boolean can be made there. if val is S.false, return False, I think.

oscarbenjamin commented 4 years ago

My first impression is that the problem is in the caller of checksol rather than checksol itself.

In this example checksol is called as

In [1]: u = Dummy()                                                                                                               

In [2]: checksol(u, u, S.false, minimal=True)                                                                                     
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-2-0fdbce3c8e92> in <module>
----> 1 checksol(u, u, S.false, minimal=True)

~/current/sympy/sympy/sympy/solvers/solvers.py in checksol(f, symbol, sol, **flags)
    300         elif attempt == 1:
    301             if not val.is_number:
--> 302                 if not val.is_constant(*list(sol.keys()), simplify=not minimal):
    303                     return False
    304                 # there are free symbols -- simple expansion might work

AttributeError: 'BooleanFalse' object has no attribute 'is_constant'

My interpretation of that is we want to check whether the equation u = 0 is solved by u taking the value False. Is that really meaningful? As far as I can tell the sol argument is expected to be an Expr or otherwise the symbol argument should be a dict mapping symbols to Expr.

I'm guessing this has showed up because of #19054 so this is another example where being cleaner about types shows up bugs.

smichr commented 3 years ago

here is a possible solution:

diff --git a/sympy/solvers/solvers.py b/sympy/solvers/solvers.py
index cdc7a72462..1daa180889 100644
--- a/sympy/solvers/solvers.py
+++ b/sympy/solvers/solvers.py
@@ -274,6 +274,8 @@ def checksol(f, symbol, sol=None, **flags):

     if isinstance(f, BooleanAtom):
         return bool(f)
+    if f == symbol and isinstance(sol[symbol], BooleanAtom):
+        return bool(f)
     elif not f.is_Relational and not f:
         return True
J-Montgomery commented 3 years ago

I meant to pick this one up, but it's already been resolved by #21602. This issue can be closed.

oscarbenjamin commented 3 years ago

There is still a bug here because nonlinsolve returns different numbers of solutions depending on whether the inputs are given as Eq or as Expr:

from sympy import *

syms = symbols('x_1:19', real=True)

(x_1, x_2, x_3, x_4, x_5, x_6, x_7, x_8, x_9, x_10,
 x_11, x_12, x_13, x_14, x_15, x_16, x_17, x_18) = syms

eqs_expr = [
    36*x_1 + 15*x_18**3 - 50*x_18**2 + 6*x_18 - 4,
    15*x_18**3 - 50*x_18**2 + 114*x_18 + 36*x_2 - 40,
    -15*x_18**3 + 50*x_18**2 - 54*x_18 + 12*x_3 + 28,
    x_4 + 1,
    -x_18 + x_5,
    x_6 - 1,
    -15*x_18**3 + 50*x_18**2 - 42*x_18 + 36*x_7 + 4,
    15*x_18**3 - 50*x_18**2 + 6*x_18 + 36*x_8 - 4,
    15*x_18**3 - 50*x_18**2 + 54*x_18 + 12*x_9 - 16,
    x_10 - 3*x_18 + 2,
    x_11 - 1,
    x_12 + x_18 - 1,
    36*x_13 + 15*x_18**3 - 50*x_18**2 + 114*x_18 - 76,
    x_14 + 1,
    18*x_15 - 15*x_18**3 + 50*x_18**2 - 24*x_18 + 4,
    6*x_16 + 15*x_18**3 - 50*x_18**2 + 54*x_18 - 22,
    x_17 + 2*x_18 - 1,
    15*x_18**4 - 20*x_18**3 + 14*x_18**2 + 8*x_18 - 8
]

eqs_eq = [Eq(e, 0) for e in eqs_expr]

print(len(nonlinsolve(eqs_expr, syms)))  # <-- prints 4
print(len(nonlinsolve(eqs_eq, syms)))    # <-- prints 2
alijosephine commented 3 years ago

Looks like for Expr input, complex/imaginary solutions are included but for Eq input, they're not. Please see the results for the below code I tried to run.

x = Symbol('x')
y = Symbol('y')
syms = [x,y]
eqs_expr = [y**2 + 1, x + y - 1]
eqs_eq = [Eq(e, 0) for e in eqs_expr]

nonlinsolve(eqs_expr, syms) ---> {(1 - I, I), (1 + I, -I)}

nonlinsolve(eqs_eq, syms) ---> {(1 - y, y)}

Can I work on this issue? I'm a beginner, so would need some guidance. (Please let me know if in case this is not a beginner-friendly issue)

I have cloned the git repo, installed sympy + mpmath and verified that I am able to run sympy. I have also gone through the tutorials to get an overview. I am using PyCharm to navigate through the project. However, I am not sure how to run in debug mode in PyCharm for this project. (I am more comfortable debugging using an IDE rather than pdb, if possible).

Update -- able to run in debug mode now, and noticed that in case of Eq input, poly equations are categorized as nonpolys. Is this intended? Would it be correct to use Equality.as_poly() in _separate_poly_nonpoly() to convert the Eq to poly expr instead of categorizing it as nonpoly?

Update -- seems like the above approach cannot handle denominators without tweaking the as_poly() code. Hence, considering converting Eq to Expr as below before processing the eq in _separate_poly_nonpoly() :

# Convert equality to expression
if isinstance(eq, Equality):
      eq = eq.lhs - eq.rhs
alijosephine commented 3 years ago

Update to above change:

  1. Made the change, added tests cases, passed all unit tests and doc tests before committing.
  2. Had forked the repo, setup github remote and added SSH keys.
  3. But pushing the branch gives below error and not able to troubleshoot:
    
    git@github.com: Permission denied (publickey).
    fatal: Could not read from remote repository.

Please make sure you have the correct access rights and the repository exists.


I understand this error has nothing to do with the project but is caused by some authentication issue but any help if possible would be appreciated. Thank you. 
oscarbenjamin commented 3 years ago

What repo are you trying to push to?

alijosephine commented 3 years ago

@oscarbenjamin I was trying to push to the forked repo - https://github.com/alijosephine/sympy. Guess I had not configured SSH keys correctly. For now, I tried to commit the changes directly to github.

Could you please review the PR and comment in case the approach is wrong or needs further work on it? Also, would like to contribute more to SymPy in other active issues.

oscargus commented 3 years ago

This could be closed now, right?

alijosephine commented 3 years ago

@oscargus there is an unresolved conversation in the PR about using rewrite() instead of lhs - rhs. Think that can be marked resolved and then the issue can be closed?

oscarbenjamin commented 3 years ago

I don't think that discussion matters too much. If the issue is fixed this can be closed.