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

Bug with Dummy #9326

Closed
joshuisken opened this Issue Apr 22, 2015 · 8 comments

Comments

Projects
None yet
4 participants
@joshuisken
Copy link

joshuisken commented Apr 22, 2015

a = Dummy("q")
b = Dummy("q")
a == b gives False as it should.

But when substituting different values for both dummies things go wrong... as shown in this example
So: if a Dummy is given a name they must be globally unique: that is unwanted in my opinion.

@moorepants

This comment has been minimized.

Copy link
Member

moorepants commented Apr 22, 2015

Confirmed on master. Here is a little more info:

In [1]: import sympy as sm

In [2]: sm.Dummy._count
Out[2]: 14

In [3]: x = sm.Dummy('a')

In [4]: x._count
Out[4]: 15

In [5]: y = sm.Dummy('a')

In [6]: y._count
Out[6]: 16

In [7]: x._count
Out[7]: 16

Seems like the _count on the dummy symbols should be a copy of Dummy._count.

@moorepants

This comment has been minimized.

Copy link
Member

moorepants commented Apr 22, 2015

The bug is in evalf and how it uses the subs kwarg:

In [1]: import sympy as sm

In [2]: sm.Dummy._count
Out[2]: 14

In [3]: x0 = sm.Dummy('x')

In [4]: sm.Dummy._count
Out[4]: 15

In [5]: x0._count
Out[5]: 15

In [6]: x1 = sm.Dummy('x')

In [7]: sm.Dummy._count
Out[7]: 16

In [8]: x0._count
Out[8]: 16

In [9]: x1._count
Out[9]: 16

In [10]: x2 = sm.Dummy('y')

In [11]: sm.Dummy._count
Out[11]: 17

In [12]: x0._count
Out[12]: 17

In [13]: x1._count
Out[13]: 17

In [14]: x2._count
Out[14]: 17

In [15]: x0 == x1
Out[15]: False

In [16]: x1 == x2
Out[16]: False

In [17]: x0
Out[17]: _x

In [18]: x1
Out[18]: _x

In [19]: x2
Out[19]: _y

In [20]: x0
Out[20]: _x

In [21]: x1
Out[21]: _x

In [22]: (x0 + x1).subs({x0:5, x1:6})
Out[22]: 11

In [23]: (x0 + x1).evalf(subs={x0:5, x1:6})
Out[23]: 10.0000000000000
@toolforger

This comment has been minimized.

Copy link
Contributor

toolforger commented Apr 22, 2015

x._count is not a copy of Dummy._count, it is an alias:

>>> from sympy import Dummy
>>> Dummy._count
14
>>> x = Dummy('a')
>>> x._count
15
>>> Dummy._count
15
>>> Dummy._count = 16
>>> x._count
16

(This is normal Python behaviour: as long as x._count is not assigned to, read accesses to it will be served from Dummy._count. More abstractly, if an instance variable does not exist, read accesses retrieve the value of the class variable, while write accesses will create the instance variable. It's horrible but that's how Python does it.)

@toolforger

This comment has been minimized.

Copy link
Contributor

toolforger commented Apr 22, 2015

@moorepants To get the index of a Dummy instance, you need to look at its dummy_index.

@toolforger

This comment has been minimized.

Copy link
Contributor

toolforger commented Apr 22, 2015

Interestingly, subs gives the correct result but evalf does not:

>>> q2 = Dummy('q')
>>> q3 = Dummy('q')
>>> q2.dummy_index
15
>>> q3.dummy_index
16
>>> # So they're really different, despite printing the same
>>> f2 = (q2 + q3)*(q2 + q3)
>>> f2
       2
(q + q) 
>>> f2.subs({q2: 2, q3:1})
9
>>> # Evaluation is a side effect of flattening I think
>>> f2.evalf(subs = {q2: 2, q3: 1})
16.0000000000000
@joshuisken

This comment has been minimized.

Copy link

joshuisken commented Apr 22, 2015

Maybe a superfluous remark: when constructing Dummy() with a unique name as argument it works correctly.
So I guess it could be related with a name lookup. I.e. Dummy('q') is named -non uniquely- '_q', whereas Dummy() is created with names like 'Dummy%d'.

BTW: my complements for the prompt reaction!

toolforger added a commit to toolforger/sympy that referenced this issue Apr 22, 2015

toolforger added a commit to toolforger/sympy that referenced this issue Apr 22, 2015

Experimental fix for sympy#9326
This fix is just a quick shot in the dark.
Somebody with a far better understanding of the details of evalf needs
to check this for ramifications and unintended side effects.
@toolforger

This comment has been minimized.

Copy link
Contributor

toolforger commented Apr 22, 2015

Line 1172 in evalf looks fishy:

        cached, cached_prec = cache.get(x.name, (None, MINUS_INF))

x is the Dummy symbol.

I added a test and an experimental fix as PR #9328 .

toolforger added a commit to toolforger/sympy that referenced this issue Jun 5, 2015

Added test for issue sympy#9326
This commit demonstrates the problem, the subsequent one fixes it.

toolforger added a commit to toolforger/sympy that referenced this issue Jun 5, 2015

Fix sympy#9326: use full symbol in evalf cache, not just symbol name
Old status: evalf_symbol used just the symbol name in its cache, this
would break if two symbols happened to use the same name.
This fix uses the full symbol instead.

toolforger added a commit to toolforger/sympy that referenced this issue Jun 5, 2015

Added test for issue sympy#9326
This commit demonstrates the problem, the subsequent one fixes it.

toolforger added a commit to toolforger/sympy that referenced this issue Jun 5, 2015

Fix sympy#9326: use full symbol in evalf cache, not just symbol name
Old status: evalf_symbol used just the symbol name in its cache, this
would break if two symbols happened to use the same name.
This fix uses the full symbol instead.
@bjodah

This comment has been minimized.

Copy link
Member

bjodah commented Jul 23, 2015

Fixed in #9328

@bjodah bjodah closed this Jul 23, 2015

bjodah added a commit that referenced this issue Jul 23, 2015

skirpichev added a commit to diofant/diofant that referenced this issue Sep 13, 2015

Fix sympy/sympy#9326: use full symbol in evalf cache, not just symbol…
… name

Old status: evalf_symbol used just the symbol name in its cache, this
would break if two symbols happened to use the same name.
This fix uses the full symbol instead.

// edited by skirpichev

skirpichev added a commit to diofant/diofant that referenced this issue Sep 13, 2015

Fix sympy/sympy#9326: use full symbol in evalf cache, not just symbol…
… name

Old status: evalf_symbol used just the symbol name in its cache, this
would break if two symbols happened to use the same name.
This fix uses the full symbol instead.

// edited by skirpichev

skirpichev added a commit to diofant/diofant that referenced this issue Sep 16, 2015

Fix sympy/sympy#9326: use full symbol in evalf cache, not just symbol…
… name

Old status: evalf_symbol used just the symbol name in its cache, this
would break if two symbols happened to use the same name.
This fix uses the full symbol instead.

// edited by skirpichev

skirpichev added a commit to diofant/diofant that referenced this issue Sep 28, 2015

Fix sympy/sympy#9326: use full symbol in evalf cache, not just symbol…
… name

Old status: evalf_symbol used just the symbol name in its cache, this
would break if two symbols happened to use the same name.
This fix uses the full symbol instead.

// edited by skirpichev

skirpichev added a commit to diofant/diofant that referenced this issue Sep 30, 2015

Fix sympy/sympy#9326: use full symbol in evalf cache, not just symbol…
… name

Old status: evalf_symbol used just the symbol name in its cache, this
would break if two symbols happened to use the same name.
This fix uses the full symbol instead.

// edited by skirpichev

skirpichev added a commit to diofant/diofant that referenced this issue Sep 30, 2015

Fix sympy/sympy#9326: use full symbol in evalf cache, not just symbol…
… name

Old status: evalf_symbol used just the symbol name in its cache, this
would break if two symbols happened to use the same name.
This fix uses the full symbol instead.

// edited by skirpichev
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment