Repository: sympy/sympy
Pull request: recreatable Dummy, simplified
Fixes #11847.

This is a simplification of #12144, only the last commit is new.  This version has slightly weaker guarantees but is much simpler and in particular doesn't keep an extra dict around to track the ordering of Dummys.

Summary of issues to consider [added by cps from below, updated again by cbm]
----
There are two types of collisions we might worry about:

Type 1: we automatically generate a Dummy that collides with a user-specified-dummy-index Dummy.
Type 2: user-specified-dummy-index unintentionally collides with an automatically generated one.

We need to be able to execute `eval(srepr(x))`, even if a new session is started between the `srepr` and the `eval` (which would mean that the `Dummy._count` and other internals may have changed).

It may be desirable for `eval(srepr(x))` to equal `x`, but perhaps not essential.  (The most important issue to fix here is #11847: if `f` is an expression with one Dummy appearing twice, then `eval(srepr(f))` should also have one Dummy appearing twice, not two Dummys'.)

Problem statement:
Dummy fails when is re-evaluated in S and srepr
Hi all!

Well, i found this little problem, Dummy is a nice function but in the way is implemented exist this:
```
>>> alpha = Dummy("alpha")
>>> i = Integral(1/sqrt(1 - sin(alpha)**2), (alpha, 0, pi/2))
>>> N(i)
0.e+2
>>> i = S(srepr(i))
>>> N(i)
Integral(1/sqrt(-sin(_alpha)**2 + 1), (_alpha, 0, pi/2))
```
Basically, if you works with Dummy, and you get the expression with srepr or similar, when you eval it every Dummy will be interpreted as a new Dummy, so it fails, in the example you can see we can't eval the second expression because the 2 Dummy expression are interpreted as differents, other ex:
```
>>> m=Dummy("m")
>>> a=Matrix([m, m])
>>> a[0]==a[1]
True
>>> b=S(srepr(a))
>>> b[0]==b[1]
False
```

So thinking a solution can be use a hash or similar function, like:
```
>>> m=Dummy("m")
>>> srepr(m)
"Dummy('m', hash=987654321)"
>>> m=Dummy("m")
>>> srepr(m)
"Dummy('m', hash=754619474)"
```
Always will exist the hash collision, but at least to cover all possible can be sympy check the existents Dummy expressions and check the hash never by equal to other, the probability of the hash collision, to be equal, in the same context, with the same name, and in the same expression, i think is very low.

Maybe instead use a hash can be a random expression, with chars like ```8waerfn23jb89a```, that can help to avoid hash collision.

Thx. Cya.

Hints from issue discussion:
I can confirm this.  Quoting `printing/repr.py`

> srepr returns a string so that the relation eval(srepr(expr))=expr holds in an appropriate environment.

Here's my minimal example:

```
>>> d = Dummy('d')
>>> A = Add(d, d, evaluate=False)

>>> srepr(A)                    # can see what the problem will be
"Add(Dummy('d'), Dummy('d'))"

>>> B = S(srepr(A))
>>> B
_d + _d

>>> A.doit()
2*_d

>>> B.doit()     # cannot, has two different Dummys
_d + _d
```

Note that Dummy does seem to maintain its identity across pickling:

```
>>> import cPickle
>>> d1 = Dummy()
>>> s1 = cPickle.dumps(d1)
>>> d2 = cPickle.loads(s1)
>>> d1 == d2
True
>>> d1 is d2
False
```

You have to set it to have the same dummy_index attribute. Right now there isn't a way to do that, but a (private) keyword argument could be added.  Not sure I agree that srepr should maintain this, though. 

mm, not support this i think is a vary bad idea, because this means all expressions with Dummy, used internally or externally will break all data export.......... (like the examples posted here)
And not all expressions can not have a Dummy, is used a lot in subs, and similars, and other common expression are integrals without elementary functions... (ex, all elliptic functions), we will be unable to represent it in the right way............
In my perspective i can see 2 ways, one, error when we send a dummy to srepr, second fix dummy in srepr, i actually have a fix for this, but i don't know if you will like it....

Well the fix is easy. Allow `Dummy('x', _dummy_index=10)` to override the `dummy_index` that is set from the class variable, and make srepr print that. 

But the whole point of dummy is that they shouldn't be equal to other instances of the same name. So the question is, should objects created with srepr be considered new or not? 

Also, if you do recreate an object in a new session, bets are off, because the same dummy_index could represent a completely different Dummy created somewhere else. This would only apply if you serialize and deserialize an expression in the same session. But I think srepr's primary use-case is for separate sessions (or any serialization really. Why would you serialize something if you're in the same session? Just reuse it).

@asmeurer I think the point is that a single object could contain the same Dummy more than once (think of a variable of integration or my silly `evaluate=False` example).

Pre-serialization this is fine.

But upon `S(srepr(x))`, they become two distinct objects.  Bad news for the integral, whose meaning has just completely changed.

(There is a related concern about two distinct objects depending on the same Dummy, and serialized separately.  I agree that's a much greyer area).

Ah, I see. So in that case, I do think this is a good idea, but caveated that dummy objects are only guaranteed to be the same in a single srepr'd expression, not across expressions. 

Hi, can you give me an example of across expressions plis?

@latot, see my example above but consider:

```
A2 = 3*A
```

Now `A2` and `A` are two expressions which contain the same Dummy (the Dummy is shared "across expressions".

Now do `B = S(srepr(A))` and `B2 = S(srepr(A2))`: we want each of those to "DTRT".  But perhaps we do not guarantee that both `B` and `B2` contain the same Dummy (the Dummy might not be shared across B and B2).

mm, i think both dummy should be equals, only for one reason, if don't are equals will break the user env...... (ex, if is trying to export it)

Commit message: Dummy: remove untrue comment from test

[skip-ci]

Patch to review:
diff --git a/sympy/core/function.py b/sympy/core/function.py
index eec86a8540d5..c7688f5829bc 100644
--- a/sympy/core/function.py
+++ b/sympy/core/function.py
@@ -707,8 +707,7 @@ def fdiff(self, argindex=1):
             else:
                 return Derivative(self, self.args[argindex - 1], evaluate=False)
         # See issue 4624 and issue 4719 and issue 5600
-        arg_dummy = Dummy('xi_%i' % argindex)
-        arg_dummy.dummy_index = hash(self.args[argindex - 1])
+        arg_dummy = Dummy('xi_%i' % argindex, dummy_index=hash(self.args[argindex - 1]))
         new_args = [arg for arg in self.args]
         new_args[argindex-1] = arg_dummy
         return Subs(Derivative(self.func(*new_args), arg_dummy),
@@ -1178,8 +1177,7 @@ def __new__(cls, expr, *variables, **assumptions):
                 obj = None
             else:
                 if not is_symbol:
-                    new_v = Dummy('xi_%i' % i)
-                    new_v.dummy_index = hash(v)
+                    new_v = Dummy('xi_%i' % i, dummy_index=hash(v))
                     expr = expr.xreplace({v: new_v})
                     old_v = v
                     v = new_v
diff --git a/sympy/core/symbol.py b/sympy/core/symbol.py
index 75e749f2e1fd..61b012b62980 100644
--- a/sympy/core/symbol.py
+++ b/sympy/core/symbol.py
@@ -14,6 +14,7 @@
 
 import string
 import re as _re
+import random
 
 
 class Symbol(AtomicExpr, Boolean):
@@ -178,13 +179,13 @@ def free_symbols(self):
 
 
 class Dummy(Symbol):
-    """Dummy symbols are each unique, identified by an internal count index:
+    """Dummy symbols are each unique, even if they have the same name:
 
     >>> from sympy import Dummy
     >>> bool(Dummy("x") == Dummy("x")) == True
     False
 
-    If a name is not supplied then a string value of the count index will be
+    If a name is not supplied then a string value of an internal count will be
     used. This is useful when a temporary variable is needed and the name
     of the variable used in the expression is not important.
 
@@ -193,21 +194,41 @@ class Dummy(Symbol):
 
     """
 
+    # In the rare event that a Dummy object needs to be recreated, both the
+    # `name` and `dummy_index` should be passed.  This is used by `srepr` for
+    # example:
+    # >>> d1 = Dummy()
+    # >>> d2 = eval(srepr(d1))
+    # >>> d2 == d1
+    # True
+    #
+    # If a new session is started between `srepr` and `eval`, there is a very
+    # small chance that `d2` will be equal to a previously-created Dummy.
+
     _count = 0
+    _prng = random.Random()
+    _base_dummy_index = _prng.randint(10**6, 9*10**6)
 
     __slots__ = ['dummy_index']
 
     is_Dummy = True
 
-    def __new__(cls, name=None, **assumptions):
+    def __new__(cls, name=None, dummy_index=None, **assumptions):
+        if dummy_index is not None:
+            assert name is not None, "If you specify a dummy_index, you must also provide a name"
+
         if name is None:
             name = "Dummy_" + str(Dummy._count)
 
+        if dummy_index is None:
+            dummy_index = Dummy._base_dummy_index + Dummy._count
+
         cls._sanitize(assumptions, cls)
         obj = Symbol.__xnew__(cls, name, **assumptions)
 
+        obj.dummy_index = dummy_index
+
         Dummy._count += 1
-        obj.dummy_index = Dummy._count
         return obj
 
     def __getstate__(self):
diff --git a/sympy/core/tests/test_symbol.py b/sympy/core/tests/test_symbol.py
index 69288934664e..c60123582027 100644
--- a/sympy/core/tests/test_symbol.py
+++ b/sympy/core/tests/test_symbol.py
@@ -30,10 +30,17 @@ def test_Symbol():
 
 def test_Dummy():
     assert Dummy() != Dummy()
-    Dummy._count = 0
-    d1 = Dummy()
-    Dummy._count = 0
-    assert d1 == Dummy()
+
+
+def test_Dummy_force_dummy_index():
+    raises(AssertionError, lambda: Dummy(dummy_index=1))
+    assert Dummy('d', dummy_index=2) == Dummy('d', dummy_index=2)
+    assert Dummy('d1', dummy_index=2) != Dummy('d2', dummy_index=2)
+    d1 = Dummy('d', dummy_index=3)
+    d2 = Dummy('d')
+    assert d1 != d2
+    d3 = Dummy('d', dummy_index=3)
+    assert d1 == d3
 
 
 def test_as_dummy():
diff --git a/sympy/printing/repr.py b/sympy/printing/repr.py
index a3488630e2e3..d09ec71f8471 100644
--- a/sympy/printing/repr.py
+++ b/sympy/printing/repr.py
@@ -145,6 +145,10 @@ def _print_Sum2(self, expr):
 
     def _print_Symbol(self, expr):
         d = expr._assumptions.generator
+        # print the dummy_index like it was an assumption
+        if expr.is_Dummy:
+            d['dummy_index'] = expr.dummy_index
+
         if d == {}:
             return "%s(%s)" % (expr.__class__.__name__, self._print(expr.name))
         else:
diff --git a/sympy/printing/tests/test_repr.py b/sympy/printing/tests/test_repr.py
index e8fd9ff7569e..1f05385916cf 100644
--- a/sympy/printing/tests/test_repr.py
+++ b/sympy/printing/tests/test_repr.py
@@ -137,16 +137,25 @@ def test_Wild():
 
 
 def test_Dummy():
-    # cannot use sT here
+    d = Dummy('d')
+    sT(d, "Dummy('d', dummy_index=%s)" % str(d.dummy_index))
+
+
+def test_Dummy_assumption():
     d = Dummy('d', nonzero=True)
-    assert srepr(d) == "Dummy('d', nonzero=True)"
+    assert d == eval(srepr(d))
+    s1 = "Dummy('d', dummy_index=%s, nonzero=True)" % str(d.dummy_index)
+    s2 = "Dummy('d', nonzero=True, dummy_index=%s)" % str(d.dummy_index)
+    assert srepr(d) in (s1, s2)
 
 
 def test_Dummy_from_Symbol():
     # should not get the full dictionary of assumptions
     n = Symbol('n', integer=True)
     d = n.as_dummy()
-    assert srepr(d) == "Dummy('n', integer=True)"
+    s1 = "Dummy('n', dummy_index=%s, integer=True)" % str(d.dummy_index)
+    s2 = "Dummy('n', integer=True, dummy_index=%s)" % str(d.dummy_index)
+    assert srepr(d) in (s1, s2)
 
 
 def test_tuple():


Code review comments:
diff_hunk: @@ -178,13 +179,13 @@ def free_symbols(self):
 
 
 class Dummy(Symbol):
-    """Dummy symbols are each unique, identified by an internal count index:
+    """Dummy symbols are each unique, even if they have the same name:
 
     >>> from sympy import Dummy
     >>> bool(Dummy("x") == Dummy("x")) == True
path: sympy/core/symbol.py
text: @user1:
I wonder if we could just simplify that as 

```
>>> Dummy('x') == Dummy('x')
False
```

@user1:
I did so in the PR that I sent.

@author:
thanks, I agree and I'll take a look.