Add optimization for replacing identities of binary operators.

[sbacchio]

Hi,

I've casually find out pythran and it's a very interesting project. Thank you for all the effort you have put in it. I was experimenting a bit and trying to understand if it can be useful for the project I have in mind.
The structure is nice and easy to develop with it.

I've tried to implement an optimization which replace identities of the operators (BinOp at the moment). I tried to give a generic structure such that many identities can be implemented easily. Similarly in the optimization Square you have the function expand_pow which replace x**0 and x**1 This can be moved here for instance.

The pull request is not complete yet, but I wanted first to ask if it's something needed and the implementation is up-to-standard.

Cheers,
Simone

[serge-sans-paille]

Hi sabbachio and thanks for the on-going PR.
The idea is legitimate, but a few stuff could be improved / changed.

First your proposal is a ntural extension of

pythran.optimizations.PatternTransform

So you' be better register tour patterns here.

Then considering the validity of the transformations :

x * 0 != 0

think of x == [1,2] or x == 'er'.

but you could workaround that using
pythran/pythonic/include/utils/neutral.hpp and the netural element ofr the addition.

remember pythran optimizations are type agnostic, as the generated program must keep polymorphism.

[serge-sans-paille]

Multiplication by 1 looks always foldable to me, and it does not interfere with type inference

In [2]: numpy.array([1,2], dtype=numpy.uint8) * 1
Out[2]: array([1, 2], dtype=uint8)

[sbacchio]

Ok, I missed PatternTransform.
Shall I simply add the cases one by one explicitely, as it is done now, e.g.

    # X * 1 => X 
    (ast.BinOp(left=Placeholder(0), op=ast.Mult(), right=ast.Num(n=1)),
     lambda: Placeholder(0)),

or try construct a more compact method as I did, but based on Placeholder?
For instance something which uses:

    known_patterns_BinOp[ ast.Mult() ] = [ ( Placeholder(0), ast.Num(n=1), Placeholder(0) ),
                                                                  ( ast.Num(n=1), Placeholder(0), Placeholder(0) ),
                                                                  etc ]

[sbacchio]

For clarity sake, the implementation I would like to carry out is an optimization of matrix products with a given sparse matrix (e.g transformation).

For instance the code:

A = [[0, 0, 1, 0],
     [0, 0, 0, 1],
     [1, 0, 0, 0],
     [0, 1, 0, 0]]

#pythran export apply_A(float[ :, 4])
def apply_A ( vector ):
    return vector[:].dot(A)

Should be optimized in something like

def apply_A ( vector ):
    tmp = numpy.empty_like(vector)
    for i in range(len(vector)):
       tmp[i,0] = vector[i,2]
       tmp[i,1] = vector[i,3]
       tmp[i,2] = vector[i,0]
       tmp[i,3] = vector[i,1]

    return tmp

which then the compiler optimize easily.
Now.. there is quite a work to do, but I think the result is feasible with your code.

[serge-sans-paille]

Yes for the palceholders!

Your test case is very interesting :-) part of the job should feasible, you alreay have constant forlding, dead code elimination and loop unrolling.

We don't have support for constant array shape (in the pythran export) but that's something I should be able to do.

[serge-sans-paille]

Unfortunately, I relaized that even this simple transformation is not always correct:

x = [1,2]
y = x * 1
y[0] = 2
print(x, y)

[sbacchio]

Arg, damn python.. I need to find a way to make it safe...
For my test case I need somehow the replacements

y += x * 0 => y += 0 => pass

[serge-sans-paille]

This case is already handled by pythran.optimization.square

[serge-sans-paille]

same copy issue as above, when one multiplies by one.

[serge-sans-paille]

This one looks correct :-)

[serge-sans-paille]

Same copy issue here.

[serge-sans-paille]

why do you need that specialization?

[sbacchio]

I thought would be good for speeding-up the optimization.
As soon as I find a way to implement safely the patterns I have in mind, we may have order of 100 patterns to check for replacement.
Then, as it was, pattern transform would check for any node any given pattern careless of the structure.
In this way, instead, it goes trough BinOp just when needed and select from the dictionary the patterns relative to the used operator.

[serge-sans-paille]

y += x * 0 => y += 0 => pass

I'll think about it but capturing the first pattern as a whole looks safe to me. ``` y += x * 0 => pass ```

[serge-sans-paille]

ok, your identity is false for floating point numbers:

float('nan') * 0

evaluates to nan.

My guess is that you should give up with the pattern matching stuff and implement a new pattern in optimizations/inline_builtin that inlinies the np.dot call when one of the parameter is a constant. The remaining optimizations will be taken care of by the backend compiler, if you activate -ffast-math.

I don't know if I'm clear...

[sbacchio]

Sure sure, it's clear. I will look at that.