EwBinaryMat kernel mishandles broadcasting where lhs
is broadcast (and not scalar)
#924
Labels
bug
A mistake in the code.
good first issue
Non-urgent, simple task with limited scope, suitable for getting started as a contributor.
The current implementation of
EwBinaryMat.h
forDenseMatrix
assumes that the operand that is broadcast is therhs
operand. However, if both operands are matrices there is no canonicalizer or logic elsewhere to guarantee this. Thus, the following script mishandles the broadcasting and returns a wrong result:Since
rhs
is assumed to be broadcast and the kernel only checks whether broadcasting is generally allowed, the inferred dimension for the result is wrong and the kernel broadcasts the wrong matrix (see lines 72-97).Dimensions are not always known at compile time, so instead of/complementary to a canonicalizer pass that shifts the broadcast matrix to be the rhs operand, a new utility function to handle broadcasting in general would be very helpful and could be reused by other kernels that support similar broadcasting I believe. E.g. it could be run at the beginning of the kernel call and verify whether the dimensions are valid, then swap
lhs
andrhs
if necessary.The text was updated successfully, but these errors were encountered: