Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow sympy algorithm in solve #22322

Closed
kcrisman opened this issue Feb 7, 2017 · 56 comments
Closed

allow sympy algorithm in solve #22322

kcrisman opened this issue Feb 7, 2017 · 56 comments

Comments

@kcrisman
Copy link
Member

kcrisman commented Feb 7, 2017

What it says. See e.g. this post for why.

Depends on #23990
Depends on #24104
Depends on #24062

CC: @rwst @mforets @sagetrac-tmonteil @EmmanuelCharpentier

Component: symbolics

Author: Ralf Stephan

Branch/Commit: 8e1e240

Reviewer: Emmanuel Charpentier

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

@kcrisman kcrisman added this to the sage-7.6 milestone Feb 7, 2017
@mforets
Copy link
Mannequin

mforets mannequin commented May 20, 2017

comment:1

AFAIK, also useful for equations where the unkown appears as an exponent, cf. solve(4.94 * 1.062^x == 15, x)

@mforets
Copy link
Mannequin

mforets mannequin commented Jun 4, 2017

comment:3

solve (from expression.pyx) passes a relational expression to Maxima's solve, but this doesn't seem to be available in sympy:

sage: (x==1)._maxima_()
_SAGE_VAR_x=1
sage: (x==1)._sympy_()
Traceback (most recent call last)
...
NotImplementedError: relation

do you have some idea on how to tackle this? otherwise, what about passing ex.lhs()-ex.rhs() to sympy's solve.

@rwst
Copy link

rwst commented Jun 5, 2017

comment:4

Replying to @mforets:

solve (from expression.pyx) passes a relational expression to Maxima's solve, but this doesn't seem to be available in sympy:
...
do you have some idea on how to tackle this? otherwise, what about passing ex.lhs()-ex.rhs() to sympy's solve.

Yes, that would be it for the equalities. For inequalities SympyConverter.relation() is needed (atm the superclass's relation is called).

@rwst
Copy link

rwst commented Jun 5, 2017

comment:5

Correction, doing ex.lhs()-ex.rhs() is not needed if the SympyConverter is fixed.

@rwst
Copy link

rwst commented Oct 8, 2017

Dependencies: #23990

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Oct 10, 2017

comment:8

Hmmm...

sage: var("a,b")
(a, b)
sage: sympy.sympify(a==b)._sage_()
a == b
sage: sympy.sympify(a==b).sage()
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-14-25de6a4c8f0a> in <module>()
----> 1 sympy.sympify(a==b).sage()

AttributeError: 'Equality' object has no attribute 'sage'

Is that the expected behaviour ? Or do you plan to enhance the .sage() method for various objects ?

It is not obvious to me why (e. g.) mathematica objets can (sometimes...) be converted back to sage via the .sage() (exposed) method, whereas Sympy objects have to use the ._sage_() (unexposed method.

@rwst
Copy link

rwst commented Oct 10, 2017

comment:9

As I explained in #23990 that is the SymPy convention.

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Oct 10, 2017

comment:10

Replying to @rwst:

As I explained in #23990 that is the SymPy convention.

Wups, bad wrong ticket. I thought that my comment was cancelled ; it was not.

Apologies...

@rwst
Copy link

rwst commented Oct 20, 2017

Changed dependencies from #23990 to #23990, #24078

@rwst
Copy link

rwst commented Oct 20, 2017

comment:11

Now that (with #23990) relations are translated from/to SymPy there are still things needed. For example we need to translate any assumptions on variables (and maybe anon. functions?) that were made using assume and var('x', domain=...). This is not only relevant for this ticket, any SymPy operation may access the SymPy knowledge base any time. So I think this should be handled the same way as with Maxima, i.e. at the time the assume/var calls are made. This is #24078.

@rwst rwst modified the milestones: sage-7.6, sage-8.2 Oct 20, 2017
@rwst
Copy link

rwst commented Oct 20, 2017

comment:12

Note that SymPy's solve does not respect the global assumptions database:

In [12]: global_assumptions
Out[12]: AssumptionsContext([Q.is_true(x > 2), Q.positive(x)])

In [13]: solve(x<3)
Out[13]: -∞ < x ∧ x < 3

In [15]: solve((x>2,x<3))
Out[15]: 2 < x ∧ x < 3

So the dependency is irrelevant (at the moment).

@rwst
Copy link

rwst commented Oct 20, 2017

Changed dependencies from #23990, #24078 to #23990

@rwst
Copy link

rwst commented Oct 20, 2017

comment:13

Our solve doesn't either, so it all is localized in the solve arguments:

sage: assume(x>2)
sage: solve(x<3,x)
[[x < 3]]
sage: solve((x>2,x<3),x)
[[2 < x, x < 3]]

@rwst
Copy link

rwst commented Oct 20, 2017

comment:14

Our solve's output is quite opaque IMHO:

sage: solve((x^2<1),x)
[[x > -1, x < 1]]
sage: solve((x^2>1),x)
[[x < -1], [x > 1]]

From a glance, which is AND, which is OR? So I would like to use And(...) and Or(...) as in SymPy but:

sage: Or = function('Or')
sage: ex = Or(x<0, x>1)
sage: ex
Or(x < 0, x > 1)
sage: ex.args()
(x,)
sage: ex.operands()
[x < 0, x > 1]

sage: ex[0]
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-8-c8fc37b12ad9> in <module>()
----> 1 ex[Integer(0)]

TypeError: 'sage.symbolic.expression.Expression' object does not support indexing

So people would need to know that operands() gets the solutions. Or we support indexing as alias to operands().

@rwst
Copy link

rwst commented Oct 20, 2017

Changed dependencies from #23990 to #23990, #24080

@rwst
Copy link

rwst commented Oct 24, 2017

comment:16

Replying to @rwst:

So people would need to know that operands() gets the solutions. Or we support indexing as alias to operands().

Supporting expression indexing seems tricky. Also this is an interface change. Better move it to a future ticket.

@rwst
Copy link

rwst commented Oct 24, 2017

Changed dependencies from #23990, #24080 to #23990

@rwst
Copy link

rwst commented Oct 25, 2017

comment:17

I cannot believe devs maintained two different versions of solve in symbolic/expression.pyx and symbolic/relation.py. expression.pyx should only be an interface to a function elsewhere, preferably in solve.py.

@mforets
Copy link
Mannequin

mforets mannequin commented Oct 25, 2017

comment:18

Note that SymPy's solve does not respect the global assumptions database:

shall we consider interfacing with SymPy's solveset? it is said to supersede solve sooner or later.

i find that the interface (equation, variable, domain) is very clear. isn't it preferable than relying on global assumptions? (or to fallback into the global assumptions if the domain is not provided.)

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Oct 25, 2017

comment:19

Replying to @mforets:

Note that SymPy's solve does not respect the global assumptions database:

shall we consider interfacing with SymPy's solveset? it is said to supersede solve sooner or later.

Fly in the ointment : solve can solve systems of equations, whereas solveset (which is, indeed, cleaner on more than one aspect), solves only equations.

In order to solve system, we would have to map solveset to all the equations in the system (each for one variable (which one ?)), and return the intersection of the results, which is currently problematic.

i find that the interface (equation, variable, domain) is very clear. isn't it preferable than relying on global assumptions? (or to fallback into the global assumptions if the domain is not provided.)

The domain is only a part of the possible assumptions : e. g., maxima often asks if some expression is positive, if such variable is integer, etc... We need to maintain a "knowledge base" about outr variables as least as rich as our current (= Maxima's) facts base.

A few remarks :

  1. I note that Sympy's knowledge base is richer than ours, BTW.

  2. We might use profitably something analogous to Mathematica's Assuming statement, which does something (roughly )equivalent to :

def assuming(condset, statement):
    try:
        OldAssumptions=assumptions()
        Assumptions=OldAssumptions.copy()
        Assumptions.extend(condset)
        forget(assumptions())
        assume(Assumptions)
        result=<do statement, somehow>
    finally:
        forget(assumptions())
        assume(OldAssumptions)

The wasp in the ointment is of course that statement is evaluated before being passed to Assuming : Python has no macroes...

I think that assuming could be implemented somehow as a decorator, but I'm still to understand that.

@rwst
Copy link

rwst commented Oct 25, 2017

comment:20

Replying to @mforets:

i find that the interface (equation, variable, domain) is very clear. isn't it preferable than relying on global assumptions?

I have abandoned the idea of respecting assumptions after I saw that SymPy doesn't either. We don't have to reproduce Maxima results so we're good.

solve can solve systems of equations, whereas solveset (which is, indeed, cleaner on more than one aspect), solves only equations.

If they replace it will be unified. And I think Sage's ex.solve() and solve(ex) should be unified too, if for no other reason than for better maintenance.

In order to solve system, we would have to map solveset to all the equations in the system (each for one variable (which one ?)), and return the intersection of the results, which is currently problematic.

It's always problematic because you need to decide ordering of expressions without variables, and that can take time because of the possible need to increase precision. The question is, should we support SymPy's solve or immediately use solveset exclusively?

We might use profitably something analogous to Mathematica's Assuming

Please review #10035.

We need to maintain a "knowledge base" about outr variables as least as rich as our current (= Maxima's) facts base.

Isn't that the same as a local context?

@rwst
Copy link

rwst commented Oct 25, 2017

Changed dependencies from #23990 to #23990, #24104

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Oct 25, 2017

comment:22

Replying to @rwst:

Replying to @mforets:

And me, it seems...

i find that the interface (equation, variable, domain) is very clear. isn't it preferable than relying on global assumptions?

I have abandoned the idea of respecting assumptions after I saw that SymPy doesn't either. We don't have to reproduce Maxima results so we're good.

That's pushing the can down the road : in order to get the solutions, we're bound to solve a system comprising the solutions obtained without the assumptions and the assumptions themselves, which might be unsolvable (e. g. cubic equation with a solution in casus irreducibilis + constraints showing that the roots are real)...

solve can solve systems of equations, whereas solveset (which is, indeed, cleaner on more than one aspect), solves only equations.

If they replace it will be unified. And I think Sage's ex.solve() and solve(ex) should be unified too, if for no other reason than for better maintenance.

Indeed. If...

In order to solve system, we would have to map solveset to all the equations in the system (each for one variable (which one ?)), and return the intersection of the results, which is currently problematic.

It's always problematic because you need to decide ordering of expressions without variables, and that can take time because of the possible need to increase precision. The question is, should we support SymPy's solve or immediately use solveset exclusively?

IMHO, both. But that might be a lot of work.. Depends on your prognosis on the time necessary for the Sympy team to coax solveset in solving systems...

We might use profitably something analogous to Mathematica's Assuming

Please review #10035.

Done (positively). But that is a possible solution only for function calls supporting hold. solve isn't one of them.

We need to maintain a "knowledge base" about outr variables as least as rich as our current (= Maxima's) facts base.

Isn't that the same as a local context?

I do not understand that (yet). Care to amplify ?

And, BTW, thank you for the huge work you are doing on Sympy, which might turn out very important for Sage.

@rwst
Copy link

rwst commented Oct 31, 2017

comment:41

We'll need to work on #22024 next because solve(x^5 + 3*x^3 + 7, x, algorithm='sympy') will fail. But not for this ticket.

@rwst
Copy link

rwst commented Oct 31, 2017

Changed branch from u/rws/22322 to u/rws/22322-1

@rwst
Copy link

rwst commented Oct 31, 2017

Changed commit from eac5f8b to 963cc0e

@rwst
Copy link

rwst commented Oct 31, 2017

comment:43

This is a new branch because of differences in the dependencies. I also added the missing translation of LambertW. Another problem is that SymPy's solveset is not always better than its solve esp. with nonlinear equations, compare e.g. x+2**x==0. This needs more work later.

Please review, these should be the last big changes.


New commits:

269c2deMerge branch 'develop' into 22322-1
86e9c2024104: move doctests
91cb91f24104: solve no longer calls itself
0f6e826Move the single expression solver into stand-alone function for granularity plus some mild cleanup.
4048e25Merge branch 'public/symbolcs/merge_solves-24104' of git://trac.sagemath.org/sage into 22322-1
963cc0e22322: allow sympy algorithm in solve

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Nov 1, 2017

comment:44

On top of 8.1.beta9 + #23980+2#4026+#24119, passes ptestlong with no errors whatsoever.

@tscrim
Copy link
Collaborator

tscrim commented Nov 1, 2017

comment:45

Reviewer name.

@rwst
Copy link

rwst commented Nov 2, 2017

Reviewer: Emmanuel Charpentier

@rwst
Copy link

rwst commented Nov 2, 2017

comment:47

@EmmanuelCharpentier I hope you don't mind me adding your name.

@vbraun
Copy link
Member

vbraun commented Dec 10, 2017

comment:49

On OSX:

**********************************************************************
File "src/sage/symbolic/relation.py", line 894, in sage.symbolic.relation.solve
Failed example:
    solve([x^2 - y^2/exp(x), y-1], x, y, algorithm='sympy')
Expected:
    [{y: 1, x: 2*lambert_w(1/2)}]
Got:
    [{x: 2*lambert_w(1/2), y: 1}]
**********************************************************************
File "src/sage/symbolic/relation.py", line 902, in sage.symbolic.relation.solve
Failed example:
    solve([x + y + z + t, -z - t], x, y, z, t, algorithm='sympy')
Expected:
    [{z: -t, x: -y}]
Got:
    [{x: -y, z: -t}]
**********************************************************************
File "src/sage/symbolic/relation.py", line 904, in sage.symbolic.relation.solve
Failed example:
    solve([x^2+y+z, y+x^2+z, x+y+z^2], x, y,z, algorithm='sympy')
Expected:
    [{y: -(z + 1)*z, x: z}, {y: -z^2 + z - 1, x: -z + 1}]
Got:
    [{x: z, y: -(z + 1)*z}, {x: -z + 1, y: -z^2 + z - 1}]
**********************************************************************
1 item had failures:
   3 of 112 in sage.symbolic.relation.solve
    [377 tests, 3 failures, 185.81 s]
----------------------------------------------------------------------
sage -t --long src/sage/symbolic/relation.py  # 3 doctests failed
----------------------------------------------------------------------

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 10, 2017

Changed commit from 963cc0e to 8e1e240

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 10, 2017

Branch pushed to git repo; I updated commit sha1. New commits:

8e1e24022322: dict order-independent doctests

@vbraun
Copy link
Member

vbraun commented Dec 13, 2017

Changed branch from u/rws/22322-1 to 8e1e240

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants