sympy / sympy

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

remove old assumptions #4983

Open vks opened 14 years ago

vks commented 14 years ago
On sympy-patches I said:

"Global assumptions are a mess, you actually never want them, I think
they should be removed from sympy. It works only for very simple and
small scripts, but not for complex stuff. If some part of your code
uses

x = Symbol('x', positive=True)

and another part

x = Symbol('x', negative=True)

you got a problem. Even if you overwrite the global assumption, it
does not work if you call a subroutine that changes them and breaks
the assumptions of the routine calling the subroutine.

Really, you don't want global assumptions, you want local assumptions.

Using Python's introspection, it should be easy to implement
assumptions that are only valid for the local scope and automatically
garbage-collected if no longer necessary. It would be maybe somewhat
hackish, but it is clearly defined behavior.

It would be a solution to clean your global assumptions explicitly,
but this is not backward-backward compatible and unnecessarily
cumbersome. And it's a huge amount of useless code added to sympy."

Well, local assumptions are much harder to implement that I thought.
Actually I need the 'nonlocal' statement from Python 3. :)

The problem is that I need to define a variable in the outer scope, which
is apparently not possible in Python 2. 'exec' allows to define a locals
and globals dict that gets respected, but it does not seem to affect the
"real" globals and locals correctly

My not-working-as-wanted code can be pulled from

    git@github.com:vks/sympy.git local_assump

Original issue for #4983: http://code.google.com/p/sympy/issues/detail?id=1884 Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/ Referenced issues: #5007 Original owner: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

asmeurer commented 14 years ago
So I guess we need to either wait for Python 3, or else make everything global and somehow mark it as to which 
scope it is supposed to be in (which would be pretty hackish).  

What exactly do you need to change in the outer scope?

Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c1 Original author: https://code.google.com/u/asmeurer@gmail.com/

vks commented 14 years ago
The basic idea is to inject a "hidden" variable holding the local assumptions into
the local scope. To do this, I need to write a function set_local_assumptions(),
which injects the hidden variable into the outer scope, which happens to be my
current local scope to which I want to tie my assumptions. If I do not use
set_local_assumptions(), I get the assumptions of the next outer scope that has some.

How could we mark what belongs to which scope? I guess we will have to reimplement
Python's scoping manually. Maybe we could have a look at the stack trace code.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c2 Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

asmeurer commented 14 years ago
Sounds like something that would be easy to do in C with pointers :)

PEP 3104 (the one about nonlocal) suggests "It has been argued that that such a feature isn't necessary, because 
a rebindable outer variable can be simulated by wrapping it in a mutable object:"  So maybe we could try 
something like that.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c3 Original author: https://code.google.com/u/asmeurer@gmail.com/

vks commented 14 years ago
Quite often I wished for C-style pointers in Python. :) Sometimes it would make
things so much easier and cleaner.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c4 Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

vks commented 14 years ago
The correct git url is

    git://github.com/vks/sympy.git local_assump

sorry.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c5 Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

vks commented 14 years ago
I finally managed to implement as wanted (import inspect is the key), please review.
The next step is to replace the old system used for all the is_* stuff. For this we
probably need to improve the performance of ask() using caching for trivial queries.

**Labels:** NeedsReview  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c6 Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

mattpap commented 14 years ago
> need to improve the performance of ask() using caching for trivial queries

That's one thing. First we need to improve ask() itself, e.g. by removing get_class()
from it, using mro attribute directly instead of using inspect, improving speed of
And (!, or substitute it by some low-level mechanism), etc.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c7 Original author: https://code.google.com/u/101069955704897915480/

vks commented 14 years ago
**Summary:** remove old assumptions  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c8 Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

vks commented 14 years ago
Issue 1615 has been merged into this issue.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c9 Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

vks commented 14 years ago
Mateusz, I addressed the concerns you raised (except the slow And) in my new_assump
branch. Is this what you meant? Do you think these changes are necessary for performance?

**Cc:** mattpap  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c10 Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

vks commented 14 years ago
Actually, the speed difference is not as big as I expected:

In [6]: %timeit x.is_positive
1000000 loops, best of 3: 393 ns per loop

In [7]: %timeit ask(x, Q.positive)
1000 loops, best of 3: 1.65 ms per loop

So it is only 4.2 times slower. This will be worse if there are many assumptions on
x, but it will be much more powerful and caching should allow a similar speed for
trivial queries. I think we can remove the old system and improve the performance later.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c11 Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

rlamy commented 14 years ago
I don't quite understand the point of this inspect magic. What is the difference with
explicitly creating an AssumptionsContext() instance? (which, BTW, is just a
stripped-down version of And, except that it's mutable)

Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c12 Original author: https://code.google.com/u/101272611947379421629/

vks commented 14 years ago
The point is to stay backward compatible. Without inspect magic you would have to write

a = AssumptionsContext()
x = Symbol('x', real=True, ctx=a)

instead of

x = Symbol('x', real=True)

Which is possible, it would need changing the whole library, but this can be done
automatically. I think it is more convenient to bind your assumptions to the local
scope where you defined your symbols, this is what you want basically to do all the
time. The first approach is of course cleaner and more flexible.

In my new_assump branch I just tried to hook local assumptions into the creation of
symbols. It worked, but at some point I got a hanging tests, and I could not yet
debug why. Rebasing too early is evil. :)

Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c13 Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

vks commented 14 years ago
I fixed this and some bugs, however currently symbols() and probably also var() are
not correctly creating local assumptions.

Just to clarify: The new_assump branch is unstable and breaks tests, there is a more
stable new_assump_stable branch. The local_assump branch is currently under review.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c15 Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

vks commented 14 years ago
Rebased the new_assump branch on master.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c16 Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

vks commented 14 years ago
Current problems are some random failures for unknown reasons. Also I could not yet debug why symbols() does not get to the correct scope (see the go_back kwarg and the test_newstyle_assumptions test).

Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c17 Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

asmeurer commented 14 years ago
**Labels:** Vinzent.Steinberg  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c18 Original author: https://code.google.com/u/asmeurer@gmail.com/

asmeurer commented 13 years ago
**Blocking:** 5040  

Referenced issues: #5040 Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c19 Original author: https://code.google.com/u/asmeurer@gmail.com/

asmeurer commented 12 years ago
**Blocking:** 5040  

Referenced issues: #5040 Original comment: http://code.google.com/p/sympy/issues/detail?id=1884#c20 Original author: https://code.google.com/u/asmeurer@gmail.com/

schymans commented 7 years ago

I just wanted to know the behaviour in the current master branch and did:

>>> from sympy import Symbol
>>> y = Symbol('y', positive=True)
>>> print y.assumptions0
{'nonzero': True, 'real': True, 'commutative': True, 'zero': False, 'nonpositive': False, 'positive': True, 'negative': False, 'nonnegative': True, 'hermitian': True, 'complex': True, 'imaginary': False}
>>> y = Symbol('y', negative=True)
>>> print y.assumptions0
{'nonzero': True, 'real': True, 'commutative': True, 'zero': False, 'nonpositive': True, 'composite': False, 'prime': False, 'negative': True, 'nonnegative': False, 'hermitian': True, 'complex': True, 'positive': False, 'imaginary': False}
>>> y = Symbol('y')
>>> print y.assumptions0
{'commutative': True}

Note that the assumptions are over-written with every call of Symbol(), i.e. x = Symbol('x', positive=True) followed by x = Symbol('x', negative=True) as mentioned in the OP should not lead to problems. However, there is a problem if one updates the assumptions for a symbol in the following way and then re-defines the symbol without assumptions:

>>> y._assumptions.update({'prime': True})
>>> print y.assumptions0
{'prime': True, 'commutative': True}
>>> y = Symbol('y')
>>> print y.assumptions0
{'prime': True, 'commutative': True}

As you see above, the assumption added before is transferred to the new symbol. Defining the symbol with an assumption avoids the problem:

>>> y = Symbol('y', real=True)
>>> print y.assumptions0
{'real': True, 'imaginary': False, 'complex': True, 'hermitian': True, 'commutative': True}
asmeurer commented 7 years ago

@schymans you shouldn't modify SymPy objects in-place. SymPy objects should be considered immutable. In your example, simply setting prime does not propagate the assumptions as Symbol('x', prime=True) would. I'm not quite sure what is happening there when you modified _assumptions, but suffice it say you should not modify that dictionary in-place, as it clearly messes up some global state on the Symbol class.

The assumptions aren't overwritten. Every Symbol with different assumptions is a distinct object. So Symbol('y', real=True), Symbol('y'), and Symbol('y', positive=True) creates three different symbols, as you can see here:

>>> y1 = Symbol('y')
>>> y2 = Symbol('y', real=True)
>>> y3 = Symbol('y', positive=True)
>>> y1 == y2
False
>>> y2 == y3
False
>>> y1.assumptions0
{'commutative': True}
>>> y2.assumptions0
{'hermitian': True, 'real': True, 'complex': True, 'imaginary': False, 'commutative': True}
>>> y3.assumptions0
{'negative': False, 'real': True, 'nonpositive': False, 'zero': False, 'hermitian': True, 'nonzero': True, 'commutative': True, 'complex': True, 'nonnegative': True, 'positive': True, 'imaginary': False}
schymans commented 7 years ago

@asmeurer, thanks for the explanation! Interestingly, y.assumptions0 is immutable, whereas y._assumptions is not. Should it be made immutable, then? Also, what is the correct way of modifying assumptions related to a symbol? Would I have to re-define the symbol, as in the below example, or is there another way? I could not find it in the docs.

>>> assumptions_y = y.assumptions0.copy()
>>> assumptions_y.update({'prime': True})
>>> y = Symbol('y', **assumptions_y)