Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect broadcasting logic in local_elemwise_alloc #1094

Closed
brandonwillard opened this issue Aug 3, 2022 · 0 comments · Fixed by #1102
Closed

Incorrect broadcasting logic in local_elemwise_alloc #1094

brandonwillard opened this issue Aug 3, 2022 · 0 comments · Fixed by #1102
Labels
bug Something isn't working graph rewriting help wanted Extra attention is needed important

Comments

@brandonwillard
Copy link
Member

Some broken broadcasting-related logic is causing local_elemwise_alloc to produce graphs that fail to perform simple multiplications:

import numpy as np
import aesara
import aesara.tensor as at
from aesara.compile.mode import get_default_mode


# We're interested in evaluating expressions like the following:
# np.ones((3, 2)) * np.ones((1, 2))

y = at.matrix("y")
x = at.matrix("x")

z = at.ones(y.shape) * x

with aesara.config.change_flags(optimizer_verbose=True):
    z_fn = aesara.function([x, y], z)
# rewriting: rewrite local_shape_to_shape_i replaces Shape.0 of Shape(y) with MakeVector{dtype='int64'}.0 of MakeVector{dtype='int64'}(Shape_i{0}.0, Shape_i{1}.0)
# rewriting: rewrite local_subtensor_make_vector replaces Subtensor{int64}.0 of Subtensor{int64}(MakeVector{dtype='int64'}.0, ScalarConstant{0}) with Shape_i{0}.0 of Shape_i{0}(y)
# rewriting: rewrite local_subtensor_make_vector replaces Subtensor{int64}.0 of Subtensor{int64}(MakeVector{dtype='int64'}.0, ScalarConstant{1}) with Shape_i{1}.0 of Shape_i{1}(y)
# rewriting: rewrite local_elemwise_alloc replaces Elemwise{mul,no_inplace}.0 of Elemwise{mul,no_inplace}(Alloc.0, x) with Elemwise{mul,no_inplace}.0 of Elemwise{mul,no_inplace}(InplaceDimShuffle{x,x}.0, Assert{msg=Aesara Assert failed!}.0)
# rewriting: rewrite constant_folding replaces InplaceDimShuffle{x,x}.0 of InplaceDimShuffle{x,x}(TensorConstant{1.0}) with TensorConstant{(1, 1) of 1.0} of None
# rewriting: rewrite local_mul_specialize replaces Elemwise{mul,no_inplace}.0 of Elemwise{mul,no_inplace}(TensorConstant{(1, 1) of 1.0}, Assert{msg=Aesara Assert failed!}.0) with Assert{msg=Aesara Assert failed!}.0 of Assert{msg=Aesara Assert failed!}(x, Elemwise{eq,no_inplace}.0, Elemwise{eq,no_inplace}.0)

aesara.dprint(z_fn)
# DeepCopyOp [id A] 7
#  |Assert{msg=Aesara Assert failed!} [id B] 6
#    |x [id C]
#    |Elemwise{eq,no_inplace} [id D] 5
#    | |Shape_i{0} [id E] 4
#    | | |y [id F]
#    | |Shape_i{0} [id G] 3
#    |   |x [id C]
#    |Elemwise{eq,no_inplace} [id H] 2
#      |Shape_i{1} [id I] 1
#      | |y [id F]
#      |Shape_i{1} [id J] 0
#        |x [id C]

z_fn(np.ones((1, 2)), np.ones((3, 2)))
# AssertionError: Aesara Assert failed!

mode = get_default_mode().excluding("local_elemwise_alloc")
z_fn_2 = aesara.function([x, y], z, mode=mode)

aesara.dprint(z_fn_2)
# Elemwise{Mul}[(0, 0)] [id A] 3
#  |Alloc [id B] 2
#  | |TensorConstant{1.0} [id C]
#  | |Shape_i{0} [id D] 1
#  | | |y [id E]
#  | |Shape_i{1} [id F] 0
#  |   |y [id E]
#  |x [id G]

z_fn(np.ones((1, 2)), np.ones((3, 2)))
# array([[1., 1.],
#        [1., 1.],
#        [1., 1.]])

I believe this is at least somewhat related to the problematic use of static shape/broadcast information discussed in #1089.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working graph rewriting help wanted Extra attention is needed important
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant