.as_terms and sort key broken for unevaluated terms

[Duane Nykamp]

The sort key seems insufficient for sorting unevaluated expressions.

In [37]: Add(x, Mul(x,x,x,evaluate=False), x, evaluate=False)
Out[37]: x + x⋅x⋅x + x

With a slight change, it works as expected:

In [38]: Add(x, Mul(x,y,x,evaluate=False), x, evaluate=False)
Out[38]: x⋅x⋅y + x + x

It seems that the sort key is the same for all three terms in the case that doesn't work.
In [43]: poly=Add(x, Mul(x,x,x,evaluate=False), x, x, evaluate=False)

In [49]: terms, gens = poly.as_terms()

In [56]: key, reverse = poly._parse_order(None)

In [57]: key(terms[0])
Out[57]: ((-1,), (), ((False, 0.0), (1.0, 0.0)))

In [58]: key(terms[1])
Out[58]: ((-1,), (), ((False, 0.0), (1.0, 0.0)))

In [59]: key(terms[2])
Out[59]: ((-1,), (), ((False, 0.0), (1.0, 0.0)))

Since the key function completely ignores the first component of each term, I'm guessing that the remaining components of the term data structure all supposed to encode all the useful information.  Except for the first component, each term is identical, which I believe to be the source of the problem.

In [68]: terms[0]
Out[68]: (x, ((1.0, 0.0), (1, 0), ()))

In [69]: terms[1]
Out[69]: (x⋅x⋅x, ((1.0, 0.0), (1, 0), ()))

In [70]: terms[2]
Out[70]: (x, ((1.0, 0.0), (1, 0), ()))

I can't say I understand all the ways in which terms are used, so I don't know what would be an appropriate fix.  But, clearly, it's broken for the terms one gets with unevaluated functions.

Any idea on how to fix it?

Duane

[Duane Nykamp]

I found a solution that works for me, but since I don't understand how the sorting work or all the places where it fits in, I'm unsure of the merits of the solution.

I just made a couple changes to .as_terms from expr.py, as shown in the diff below.  The idea is that
1) since a given base can show up multiple times in a term, we have to add up the exponents from all instances, and
2) since the exponent alone doesn't distinguish x*x from x**2, we also keep track of how many times the base appeared.

I stored this information in a tuple so that x*x is represented by (2,2) and x**2 is represented by (2,1). 

With the change, an unevaluated polynomial sorts its terms in a consistent way.

In [4]: Add(x, x**3, x, Mul(x,x,x,evaluate=False), x, Mul(x**2,x, evaluate=False), x**3, evaluate=False)
Out[4]:
           2    3    3           
x⋅x⋅x + x⋅x  + x  + x  + x + x + x

If this seems reasonable, I could make a pull request.

Duane


diff --git a/sympy/core/expr.py b/sympy/core/expr.py
index 0de02ff..c9093f0 100644
--- a/sympy/core/expr.py
+++ b/sympy/core/expr.py
@@ -928,7 +928,8 @@ def as_terms(self):
                     if factor.is_commutative:
                         base, exp = decompose_power(factor)
 
-                        cpart[base] = exp
+                        prev = cpart.get(base,(0,0))
+                        cpart[base] = (prev[0]+exp, prev[1]+1)
                         gens.add(base)
                     else:
                         ncpart.append(factor)

[Aaron Meurer]

I would make a pull request. Then Travis CI will run the tests and if
anything fails you can see if it has unpredicted implications on some
other part of the codebase. And if not, it is probably fine.

Aaron Meurer

> --
> You received this message because you are subscribed to the Google Groups
> "sympy" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to sympy+un...@googlegroups.com.
> To post to this group, send email to sy...@googlegroups.com.
> Visit this group at http://groups.google.com/group/sympy.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/sympy/8b727451-a1b3-4c5e-820d-7f06b93db6cc%40googlegroups.com.
>
> For more options, visit https://groups.google.com/d/optout.