Repository: sympy/sympy

Issue:
signsimp() create invalid tree when matrices are involved
I have an expression involving symbolic matrices that raises an error when given to `simplify()`. I reduced it to the following code which also fails:
```
from sympy import *
A = MatrixSymbol('A', 2, 2)
e = signsimp(-A * A - A)
print(e)
```
Stack trace:
```
Traceback (most recent call last):
  File "test.py", line 4, in <module>
    e = signsimp(-A * A - A)
  File "/path/sympy/sympy/simplify/simplify.py", line 385, in signsimp
    e = sub_post(sub_pre(expr))
  File "/path/sympy/sympy/simplify/cse_opts.py", line 29, in sub_pre
    e = e.xreplace(negs)
  File "/path/sympy/sympy/core/basic.py", line 1148, in xreplace
    value, _ = self._xreplace(rule)
  File "/path/sympy/sympy/core/basic.py", line 1163, in _xreplace
    a_xr = _xreplace(rule)
  File "/path/sympy/sympy/core/basic.py", line 1170, in _xreplace
    return self.func(*args), True
  File "/path/sympy/sympy/matrices/expressions/matmul.py", line 48, in __new__
    factor, matrices = obj.as_coeff_matrices()
  File "/path/sympy/sympy/matrices/expressions/matmul.py", line 114, in as_coeff_matrices
    raise NotImplementedError("noncommutative scalars in MatMul are not supported.")
NotImplementedError: noncommutative scalars in MatMul are not supported.
```
Obviously no noncommutative scalars are involved here. The reason seems to be a `Mul` object that has matrix arguments, which to my understanding is an invalid tree. The following diff fixes the problem, but I'm unsure whether that is the correct fix:
```
diff --git a/sympy/simplify/cse_opts.py b/sympy/simplify/cse_opts.py
index 754394d43f..16daefc0ba 100644
--- a/sympy/simplify/cse_opts.py
+++ b/sympy/simplify/cse_opts.py
@@ -14,7 +14,7 @@ def sub_pre(e):
     """
     # replacing Add, A, from which -1 can be extracted with -1*-A
     adds = [a for a in e.atoms(Add) if a.could_extract_minus_sign()]
-    reps = dict((a, Mul._from_args([S.NegativeOne, -a])) for a in adds)
+    reps = dict((a, Mul(S.NegativeOne, -a)) for a in adds)
     e = e.xreplace(reps)
 
     # repeat again for persisting Adds but mark these with a leading 1, -1
```
`Mul._from_args()` always seems to create a `Mul` object, whereas `Mul(...)` checks its arguments and returns a `MatMul` if the arguments are matrices.

Would the above be an acceptable fix? `Mul._from_args()` is probably being used for performance reasons, which is lost when using `Mul()`. So perhaps `Mul._from_args()` should be fixed instead... (Note: there are more calls to `Mul._from_args()` in that file.)

Discussion:
sympy-bot: :white_check_mark:

Hi, I am the [SymPy bot](https://github.com/sympy/sympy-bot) (v158). I'm here to help you write a release notes entry. Please read the [guide on how to write release notes](https://github.com/sympy/sympy/wiki/Writing-Release-Notes).



* No release notes entry will be added for this pull request.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it. <details><summary>Click here to see the pull request description that was parsed.</summary>

    Without executing Mul's post-processor, it might pack matrices inside.

    <!-- Your title above should be a short description of what
    was changed. Do not include the issue number in the title. -->

    #### References to other Issues or PRs
    Fixes #18991

    #### Other comments
    This raises the question whether maybe all `_from_args()` methods should themselves invoke the post processors instead, to avoid having to do that from all call sites. Existing code seems to call it after `_from_args()` though.

    #### Release Notes
    <!-- BEGIN RELEASE NOTES -->
    NO ENTRY
    <!-- END RELEASE NOTES -->

</details><p>


Patch:
diff --git a/doc/ext/sympylive.py b/doc/ext/sympylive.py
index 3c37290f20..eaed7af97e 100644
--- a/doc/ext/sympylive.py
+++ b/doc/ext/sympylive.py
@@ -15,17 +15,18 @@ def builder_inited(app):
         raise ExtensionError('sympylive_url config value must be set'
                              ' for the sympylive extension to work')
 
-    app.add_js_file(app.config.sympylive_url + '/static/utilities.js')
-    app.add_js_file(app.config.sympylive_url + '/static/external/classy.js')
-
-    app.add_css_file(app.config.sympylive_url + '/static/live-core.css')
-    app.add_css_file(app.config.sympylive_url +
-                     '/static/live-autocomplete.css')
-    app.add_css_file(app.config.sympylive_url + '/static/live-sphinx.css')
-
-    app.add_js_file(app.config.sympylive_url + '/static/live-core.js')
-    app.add_js_file(app.config.sympylive_url + '/static/live-autocomplete.js')
-    app.add_js_file(app.config.sympylive_url + '/static/live-sphinx.js')
+    app.add_javascript(app.config.sympylive_url + '/static/utilities.js')
+    app.add_javascript(app.config.sympylive_url + '/static/external/classy.js')
+
+    app.add_stylesheet(app.config.sympylive_url + '/static/live-core.css')
+    app.add_stylesheet(app.config.sympylive_url +
+                       '/static/live-autocomplete.css')
+    app.add_stylesheet(app.config.sympylive_url + '/static/live-sphinx.css')
+
+    app.add_javascript(app.config.sympylive_url + '/static/live-core.js')
+    app.add_javascript(app.config.sympylive_url +
+                       '/static/live-autocomplete.js')
+    app.add_javascript(app.config.sympylive_url + '/static/live-sphinx.js')
 
 
 def setup(app):
diff --git a/doc/src/modules/crypto.rst b/doc/src/modules/crypto.rst
index 901b3d1e5e..43a916b647 100644
--- a/doc/src/modules/crypto.rst
+++ b/doc/src/modules/crypto.rst
@@ -83,6 +83,8 @@ substitutions at different times in the message.
 
 .. autofunction:: decipher_bifid5
 
+.. autofunction:: bifid5_square
+
 .. autofunction:: encipher_bifid6
 
 .. autofunction:: decipher_bifid6
@@ -129,6 +131,10 @@ substitutions at different times in the message.
 
 .. autofunction:: dh_shared_key
 
+.. autofunction:: encipher_elgamal
+
+.. autofunction:: decipher_elgamal
+
 .. autofunction:: gm_public_key
 
 .. autofunction:: gm_private_key
diff --git a/doc/src/modules/polys/reference.rst b/doc/src/modules/polys/reference.rst
index 426b16683e..635a3072ef 100644
--- a/doc/src/modules/polys/reference.rst
+++ b/doc/src/modules/polys/reference.rst
@@ -33,6 +33,8 @@ Basic polynomial manipulation functions
 .. autofunction:: subresultants
 .. autofunction:: resultant
 .. autofunction:: discriminant
+.. autofunction:: sympy.polys.dispersion.dispersion
+.. autofunction:: sympy.polys.dispersion.dispersionset
 .. autofunction:: terms_gcd
 .. autofunction:: cofactors
 .. autofunction:: gcd
diff --git a/doc/src/modules/sets.rst b/doc/src/modules/sets.rst
index 0c88eb818f..bc7add54e6 100644
--- a/doc/src/modules/sets.rst
+++ b/doc/src/modules/sets.rst
@@ -27,6 +27,14 @@ FiniteSet
 .. autoclass:: FiniteSet
    :members:
 
+ConditionSet
+^^^^^^^^^^^^
+.. module:: sympy.sets.conditionset
+    :noindex:
+
+.. autoclass:: ConditionSet
+    :members:
+
 Compound Sets
 -------------
 
@@ -166,5 +174,4 @@ Condition Sets
 ConditionSet
 ^^^^^^^^^^^^
 
-.. autoclass:: ConditionSet
-    :members:
+.. autofunction:: ConditionSet
diff --git a/sympy/core/containers.py b/sympy/core/containers.py
index f5c7a0f607..b9ddeac3a9 100644
--- a/sympy/core/containers.py
+++ b/sympy/core/containers.py
@@ -122,7 +122,8 @@ def tuple_count(self, value):
         return self.args.count(value)
 
     def index(self, value, start=None, stop=None):
-        """Searches and returns the first index of the value."""
+        """T.index(value, [start, [stop]]) -> integer -- return first index of value.
+           Raises ValueError if the value is not present."""
         # XXX: One would expect:
         #
         # return self.args.index(value, start, stop)
@@ -258,16 +259,15 @@ def args(self):
         return tuple(self.elements)
 
     def items(self):
-        '''Returns a set-like object providing a view on dict's items.
-        '''
+        '''D.items() -> list of D's (key, value) pairs, as 2-tuples'''
         return self._dict.items()
 
     def keys(self):
-        '''Returns the list of the dict's keys.'''
+        '''D.keys() -> list of D's keys'''
         return self._dict.keys()
 
     def values(self):
-        '''Returns the list of the dict's values.'''
+        '''D.values() -> list of D's values'''
         return self._dict.values()
 
     def __iter__(self):
@@ -279,7 +279,7 @@ def __len__(self):
         return self._dict.__len__()
 
     def get(self, key, default=None):
-        '''Returns the value for key if the key is in the dictionary.'''
+        '''D.get(k[,d]) -> D[k] if k in D, else d.  d defaults to None.'''
         return self._dict.get(sympify(key), default)
 
     def __contains__(self, key):
diff --git a/sympy/simplify/cse_opts.py b/sympy/simplify/cse_opts.py
index 754394d43f..f02b6e61a7 100644
--- a/sympy/simplify/cse_opts.py
+++ b/sympy/simplify/cse_opts.py
@@ -14,7 +14,8 @@ def sub_pre(e):
     """
     # replacing Add, A, from which -1 can be extracted with -1*-A
     adds = [a for a in e.atoms(Add) if a.could_extract_minus_sign()]
-    reps = dict((a, Mul._from_args([S.NegativeOne, -a])) for a in adds)
+    reps = dict((a, Mul(S.NegativeOne, -a, evaluate=False)) for a in adds)
+
     e = e.xreplace(reps)
 
     # repeat again for persisting Adds but mark these with a leading 1, -1
@@ -25,7 +26,7 @@ def sub_pre(e):
             if a in reps:
                 negs[a] = reps[a]
             elif a.could_extract_minus_sign():
-                negs[a] = Mul._from_args([S.One, S.NegativeOne, -a])
+                negs[a] = Mul(S.One, S.NegativeOne, -a, evaluate=False)
         e = e.xreplace(negs)
     return e
 
@@ -37,7 +38,7 @@ def sub_post(e):
     for node in preorder_traversal(e):
         if isinstance(node, Mul) and \
             node.args[0] is S.One and node.args[1] is S.NegativeOne:
-            replacements.append((node, -Mul._from_args(node.args[2:])))
+            replacements.append((node, -Mul(*node.args[2:], evaluate=False)))
     for node, replacement in replacements:
         e = e.xreplace({node: replacement})
 


Test patch:
diff --git a/sympy/simplify/tests/test_cse.py b/sympy/simplify/tests/test_cse.py
index 4e46adafe0..f076d8f533 100644
--- a/sympy/simplify/tests/test_cse.py
+++ b/sympy/simplify/tests/test_cse.py
@@ -5,7 +5,7 @@
 from sympy import (
     Add, Mul, Pow, Symbol, exp, sqrt, symbols, sympify, cse,
     Matrix, S, cos, sin, Eq, Function, Tuple, CRootOf,
-    IndexedBase, Idx, Piecewise, O
+    IndexedBase, Idx, Piecewise, O, signsimp
 )
 from sympy.core.function import count_ops
 from sympy.simplify.cse_opts import sub_pre, sub_post
@@ -545,3 +545,7 @@ def test_issue_18203():
 def test_unevaluated_mul():
     eq = Mul(x + y, x + y, evaluate=False)
     assert cse(eq) == ([(x0, x + y)], [x0**2])
+
+def test_issue_18991():
+    A = MatrixSymbol('A', 2, 2)
+    assert signsimp(-A * A - A) == -A * A - A