My biggest concern with this design is the prioritization system. How does it handle subclasses? Do subclasses always need to increase their op_priority to give themselves higher priority when they override the preprocessor?
Another question is if we want to support a multipledispatch like model where you can override the behavior independent of defining it on a class. We should also look at how multipledispatch system handle priority. I think it's harder here because we have an arbitrary number of arguments, but perhaps we can use the same principles. Aaron Meurer On Fri, May 15, 2020 at 12:36 AM mcpl snu <[email protected]> wrote: > > Discussion for introducing the hooks to core classes > > This thread is related to issue #5040 and PR #18769. > > > 1. What is the problem? > > Currently, SymPy's Add and Mul are very complicated. There exist various type > checkings in their flatten method to make them able to handle various > objects. However, hard-coding like this can be troublesome when new classes > are added. Not only these methods need to be added with more type checkings, > it will make the code more and more jumbled. > > To relieve this, 'postprocessor' is introduced to Add, Mul, Pow. When the > instance of these classes is constructed, this 'preprocessor' hook is applied > to it, transforming the result. (Currently, only matrix uses this hook.) > Although this can be used to reduce the complexity in the classes, still it > has many problems. First, it is not applied when 'evaluate=False' option is > passed. Second, it cannot take any keyword argument option. Third, the > instance always needs to go through flatten method first, and only after then > it is transformed. This is a waste if completely different flatten algorithm > is to be applied by postprocessor. Finally, because of the same reason with > third problem, the arguments must be made sure to pass flatten methods > without raising any error, adding unnecessary restriction to class behaviors. > > > 2. How can we solve it? > > I believe we can solve it by introducing a hook, which is applied before any > job is done, and also able to take keyword arguments. > > > 3. What is my proposal? > > My proposal in #18769 is to introduce a hook which is, unlike postprocessor, > applied to the arguments before anything is done. It has these features: > > 1) Keyword arguments can be passed, allowing more flexibility. > 2) Averts the arguments to be unnecessarily processed by flatten method. > 3) By checking `_op_priority` of the arguments, determines which hook > will be used. > > The third feature is important. In current SymPy, `_op_priority` determines > which operator methods will be used when operator (+, *, etc) is applied > between two objects. This means, if A's _op_priority is 10 and B's is 11, > then B.__radd__(A), instead of A.__add__(b), is called when A+B is executed. > So I thought; "If `_op_priority` determines the behavior of 'A+B', surely it > could determine `Add(A, B)` as well!" > > This is especially useful when argument of highest priority defines special > classes, such as MatAdd and MatMul for matrix classes. Suppose we have three > arguments A,B and C, with their priority A<B<C. When these three are added, > `C_Add(C, Add(A, B) )` is returned. > Resolving this is simple. Just make a hook function for C: > > >>> C_type_args = [i for i in args if isinstance(i, C)] > >>> other_args = [i for i in args if not isinstance(i, C.func)] > >>> other_args_added = Add(*other_args) > >>> return C_Add(C, other_args_added) > > See? No type checking is needed. > With this, let's say someone want to add a new class D. `Add(A, B, C, D)` > will return `C_Add(C, D_Add(A, B, D) )`. Doing this is simple indeed. Set D's > `_op_priority` so that A<B<D<C, and make a hook function for D: > > >>> D_type_args = [i for i in args if isinstance(i, D)] > >>> other_args = [i for i in args if not isinstance(i, D.func)] > >>> other_args_added = Add(*other_args) > >>> return D_Add(D, other_args_added) > > Then, running `Add(A, B, C, D)` will call C's hook. Inside it, > `Add(*other_args)` will call D's hook because it has the highest priority > among A,B and D, assigning D_add(A, B, D) to `other_args_added`. Add.flatten > is not called here. > This all works with full expandability, with minimum complexity and bypassing > unnecessary steps. > > > 4. Please give your opinion > > My proposal will be one of the many ways to resolve current issue. I'd be > grateful if you suggest any improvement to my work, or come up with better > idea. > > -- > 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 [email protected]. > To view this discussion on the web visit > https://groups.google.com/d/msgid/sympy/fee15557-d98b-4512-9d26-92203c207cc3%40googlegroups.com. -- 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 [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CAKgW%3D6KYaFN%3DZx_P7O8uBfnL911%2BFaCxkuFnGy36qWBPvo0riA%40mail.gmail.com.
