sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.47k stars 487 forks source link

Remove vacuous solutions from solve #14229

Open ppurka opened 11 years ago

ppurka commented 11 years ago

Sometimes solve and its variants tend to report vacuous solutions. A recent one from ask.sagemath is the following:

sage: x,y = var('x,y')
sage: solve([x^2*y^2 <= x^2*y, x^2*y^2 > x^2*y],[x,y])
[[x == 0, 1 < y, 0 != 0], [x == 0, y < 0, 0 != 0]]

Shouldn't we remove these meaningless solutions? The attached patch contains a potential solution. If it seems reasonable, then similar changes could be introduced in solve_ineq.

The output of the above command after this patch is as expected:

sage: x,y = var('x,y')
sage: solve([x^2*y^2 <= x^2*y, x^2*y^2 > x^2*y],[x,y])
[]

If you can translate this to maxima proper, please feel free to do so and submit a patch upstream.


Workaround: call maxima_calculus("domain: real") before solve (see comment 12).

CC: @kcrisman @jondo

Component: symbolics

Issue created by migration from https://trac.sagemath.org/ticket/14229

ppurka commented 11 years ago

Apply to devel/sage

ppurka commented 11 years ago
comment:1

Attachment: trac_14229-remove_vacuous_solutions.patch.gz

ppurka commented 11 years ago

Description changed:

--- 
+++ 
@@ -6,4 +6,4 @@
 [[x == 0, 1 < y, 0 != 0], [x == 0, y < 0, 0 != 0]]

-Shouldn't we remove these meaningless solutions? +Shouldn't we remove these meaningless solutions? The attached patch contains a potential solution. If it seems reasonable, then similar changes could be introduced in solve_ineq.

ppurka commented 11 years ago

Description changed:

--- 
+++ 
@@ -7,3 +7,11 @@

Shouldn't we remove these meaningless solutions? The attached patch contains a potential solution. If it seems reasonable, then similar changes could be introduced in solve_ineq. + +The output of the above command after this patch is as expected: + + +sage: x,y = var('x,y') +sage: solve([x^2*y^2 <= x^2*y, x^2*y^2 > x^2*y],[x,y]) +[] +

nbruin commented 11 years ago
comment:5

This kind of change should really be made to maxima, not to sage's postprocessing of what maxima returns.

ppurka commented 11 years ago

Work Issues: port to upstream

ppurka commented 11 years ago
comment:6

If someone knows how to translate this to maxima, please feel free to put this upstream. Thanks! [ I can not spend time in learning maxima right now. :( ]

ppurka commented 11 years ago

Description changed:

--- 
+++ 
@@ -15,3 +15,7 @@
 sage: solve([x^2*y^2 <= x^2*y, x^2*y^2 > x^2*y],[x,y])
 []

+ +--- + +If you can translate this to maxima proper, please feel free to do so and submit a patch upstream.

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 11 years ago
comment:7

Helloooooooo !!

You remove equations that do not contain a variable, but couldn't 0=0 appear from time to time ? Secondly, can't the expression "bool(solution_list)" be costly from time to time ?

You could also write, later in the patch

sol_list = filter(_test_solution, string_to_list_of_solutions(repr(s)))

Nathann

ppurka commented 11 years ago
comment:8

0==0 can appear, but bool(0==0) should evaluate to True. Which is fine by me. The point is to remove solutions which are clearly false. Anyway, this patch is going nowhere since nbruin suggests putting this upstream (I can not do that now).

EDIT: bool(solution_list) is done only when a symbolic expression (i.e., not a list of symbolic expressions) is input and there are no variables in the symbolic expression. This shouldn't introduce a significant slowdown. The solve function itself will probably take way more time than what these boolean checks will take.

nbruin commented 10 years ago
comment:12

These vacuous solutions seem to arise from the fact that solving inequalities with domain: complex gives some problems, which is to be expected. So rather than trying to clean up the mess afterwards, we should find a way of setting domain: real in maxima before solving inequalities:

sage: sage: x,y = var('x,y')
sage: maxima_calculus("domain: real")
real
sage: sage: solve([x^2*y^2 <= x^2*y, x^2*y^2 > x^2*y],[x,y])
[]

See sage-devel too.

nbruin commented 10 years ago
comment:14

One may be concerned with performance when we have to set these flags all the time. On the expect interface that's probably silly anyway, and on the library interface we can reach a little deeper and save some time:

sage: maxima_calculus(1) # make sure maxima_lib is initialized
sage: from sage.libs.ecl import EclObject,ecl_eval
sage: set_complex=ecl_eval("(defun sc () (setf $DOMAIN '$COMPLEX))")
sage: set_real=ecl_eval("(defun sc () (setf $DOMAIN '$REAL))")
sage: %timeit set_complex()
100000 loops, best of 3: 2.14 µs per loop
sage: %timeit maxima_calculus("domain: complex")
10000 loops, best of 3: 146 µs per loop

For comparison:

sage: %timeit solve([x<y],[x,y])
100 loops, best of 3: 4.6 ms per loop

so I think we can afford a little set_real.eval(); ...solve iquality; set_complex.eval();. We could keep a flag on what we set last and be lazy with flipping, but I expect that solving inequalities is relatively rare, and always slow, so we shouldn't slow down other operations with testing for a flag. To give you an idea, the following does call into maxima:

sage: %timeit integrate(x,x)
1000 loops, best of 3: 693 us per loop
jondo commented 9 years ago
comment:16

Replying to @nbruin:

so I think we can afford a little set_real.eval(); ...solve iquality; set_complex.eval();.

So do you think this should be done in the Sage solve function when it detects inequalities?

jondo commented 9 years ago

Changed work issues from port to upstream to none

jondo commented 9 years ago

Description changed:

--- 
+++ 
@@ -19,3 +19,6 @@
 ---

 If you can translate this to maxima proper, please feel free to do so and submit a patch upstream.
+
+---
+**Workaround**: call `maxima_calculus("domain: real")` before `solve` (see comment 12).