Slight change to Add gives infinite recursion on evalf

[Duane Nykamp]

I created an AddUnsort class that doesn't change the order of its argument.  Now, I discovered I get an infinite recursion when I evalf.  But, I'm having trouble figuring out how to prevent this infinite recursion.

The still get the infinite recursion even with this stripped down class that does very little to Add.  The only change is that it doesn't evaluate the arguments (it acts as long evaluate=False) and it doesn't order the arguments.

from sympy import Add
from sympy.core.cache import cacheit
from sympy.core.sympify import _sympify

class AddUnsortPart(Add):
    @cacheit
    def __new__(cls, *args, **options):
        # __new__ from AssocOp, with differences
        # - we don't throw out the identity
        # - we never evaluate

        args = list(map(_sympify, args))

        if len(args) == 0:
            return cls.identity
        if len(args) == 1:
            return args[0]

        return cls._from_args(args)


But, executing, for example,

AddUnsortPart(2,5).evalf()

gets an infinite loop between the _eval_evalf() function of AssocOp and ._evalf() of EvalfMixin.  I'm obviously missing something here, as I don't see why one usually doesn't get this infinite loop, so what I should do to prevent this problem.

Any pointers would be helpful.  The above stripped down class hardly modifies Add at all, so it's puzzling why this change creates the problem.  (Of course, in the real class, I'm making a lot more changes to make it useful AddUnsort class.)

Thanks,
Duane

[Duane Nykamp]

OK, I'm pretty sure this is a bug.  I don't have to change Add at all to get this infinite recursion, just create any subclass

In [7]: Add(3,3,evaluate=False).evalf()
Out[7]: 6.00000000000000

In [8]: class Add2(Add):
   ...:     pass
   ...:

In [9]: Add2(3,3,evaluate=False).evalf()
RuntimeError: maximum recursion depth exceeded

I can't figure out where the code is treating Add and its identical subclass Add2 differently.

Duane

[Aaron Meurer]

That's definitely a bug. Subclassing any class without changing
anything shouldn't break anything. There are unfortunately several
instances of this issue (see
https://github.com/sympy/sympy/issues/6751). Probably somewhere in
that code path something is referencing Add directly instead of using
self.func, or doing a type comparison without using isinstance.

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/60272dc0-8bf7-47db-83a9-01a840180943%40googlegroups.com.
>
> For more options, visit https://groups.google.com/d/optout.

[Francesco Bonazzi]

By the way, is there a real difference between type(self)(*args) and self.func(*args) ?

[Aaron Meurer]

For most classes there isn't, but there can be. Using func instead of
type() allows objects to have a function that isn't its literal Python
class. This is necessary, for instance, if you want to allow the
function to also be usable as an expression, since in that case it
would also have to be a Python object (not a class).

An example of this are the predicate objects, like Q.positive. We want
to be able to use both Q.positive and Q.positive(x) in logic
expressions. If Q.positive were the type() of Q.positive(x), this
would be impossible, because then Q.positive couldn't be an instance
of Basic.

So instead, it has its own type, and the .func is set appropriately:

In [20]: type(Q.positive(x))
Out[20]: sympy.assumptions.assume.AppliedPredicate

In [21]: Q.positive(x).func
Out[21]: Q.positive

Ideally we would like to do the same thing with functions as well
(https://github.com/sympy/sympy/issues/4787) so that it will be
possible to write things like cos + sin, and other "operator" like
expressions. sin + cos may end up not being a good idea, but as the
issue notes, something like D(f), where D is some kind of differential
operator, would be possible in this case (right now it isn't really
because f isn't an instance of Basic, so you can't just have D(f).args
== (f,)).

func has also been suggested as the way around issues with classes
that want to put non-Basic things in their args (particularly strings,
like with MatrixSymbol). Instead of MatrixSymbol('x', n, n).args ==
('x', n, n) as it is now, which is bad and breaks a lot of things
(because 'x' is not an instance of Basic), we would have
MatrixSymbol('x', n, n).args == (n, n) and MatrixSymbol('x', n,
n).func would be a special object that contains the 'x', which can
recreate the original MatrixSymbol('x', n, n).

Aaron Meurer

> https://groups.google.com/d/msgid/sympy/fe2324dd-75b8-4f14-a21b-f9fcb192b2c5%40googlegroups.com.

[Duane Nykamp]

There are two places where I see a hard coded Add that might be a problem.

One in _create_evalf_table() in evalf.py, but commenting out the Add line doesn't create an infinite recursion.

The more likely place is in Expr.as_independent().  Clearly, that gives a different result for Add and Add2.

In [9]: Add(3,2, evaluate=False).as_independent(Symbol, AppliedUndef)
Out[9]: (5, 0)

In [10]: Add2(3,2, evaluate=False).as_independent(Symbol, AppliedUndef)
Out[10]: (2 + 3, 1)

OK, just tried to write a fix, but as I don't really understand what as_independent does, I'm not confident of the fix.  Here's a pull request that at least gives the right answer for the one case of as_independent that I checked and removes the infinite loop for Add2.

In [3]: Add2(3,2, evaluate=False).as_independent(Symbol, AppliedUndef)
Out[3]: (5, 0)

In [4]: Add2(3,2, evaluate=False).evalf()
Out[4]: 5.00000000000000

https://github.com/sympy/sympy/pull/10126

Sorry, haven't written any tests, yet.  I'm out of time that I can spend on this for now. :)

Duane