[Bug] Weird behaviors for relay.sum and relay.cast op

[Kevin-XiongC]

import tvm
from tvm import relay
from tvm.contrib import graph_executor
import numpy as np
x=relay.var("x",shape=[2,2],dtype="int32")
t1=relay.cast(x,"bool")
y=relay.sum(t1)
f = relay.Function([x],relay.Tuple([t1,y]))
mod = tvm.IRModule({"main":f})
target = "llvm"
with tvm.transform.PassContext(opt_level=3 ):
    lib = relay.build(mod, target=target)
m = graph_executor.GraphModule(lib['default'](tvm.cpu()))
x = np.ones((2,2)).astype("int32")
m.set_input("x",x)
print(m.get_output(0))
print(m.get_output(1))

The code snippets above should be equivalent to below:

x = np.ones((2,2)).astype("int32").astype(bool)
print(x)
print(np.sum(x,dtype=bool))

Expected behavior

The relay program should always produce t1 all true and y true just like the numpy-based code outputs below. In common sense, it is expected to view non-zero elements as True and sum should perform reduction by OR operation for bool data type.

[[ True  True]
 [ True  True]]
True

Actual behavior

However, the relay-based code produce rather random results.

[[ True False]
 [False False]]
True

Or

[[False False]
 [False False]]
False

Even

[[False False]
 [False False]]
True

Environment

commit id: 1258863
OS: ubuntu 20.04 or macos
LLVM 14

Steps to reproduce

Run the code snippets above.

[quic-sanirudh]

This seems to return all True for me consistently with the same commit ID, but I'm also using the latest LLVM and Ubuntu 18.04, but that should not cause changes as far as I can understand.

[Kevin-XiongC]

Yeah, I believe the LLVM version should by no means cause that since these are quite basic computations. That's why I am pretty confused here.

[quic-sanirudh]

Okay, when I ran it 30 times in a loop, I got a 2-3 of the outputs as false. There seems to be something inconsistent going on here. I'll look into cast and see what's going on if possible, and sorry for the earlier reply. Looks like there is some unexpected/undefined behavior.

[masahi]

@Kevin-XiongC Interesting finding, does this come up in a real model?

[Kevin-XiongC]

@masahi It was found when I was adapting OPs from a training frontend as an alternative for relay.any().

[vinx13]

@Kevin-XiongC it seems you didn't call m.run() before getting output and as a result it returned uninitialized tensor

[Kevin-XiongC]

@Kevin-XiongC it seems you didn't call m.run() before getting output and as a result it returned uninitialized tensor

import tvm
from tvm import relay
from tvm.contrib import graph_executor
import numpy as np
x=relay.var("x",shape=[2,2],dtype="float32")
t1=relay.cast(x,"bool")
y=relay.sum(t1)
f = relay.Function([x],relay.Tuple([t1,y]))
mod = tvm.IRModule({"main":f})
target = "llvm"
with tvm.transform.PassContext(opt_level=3):
    lib = relay.build(mod, target=target)
m = graph_executor.GraphModule(lib['default'](tvm.cpu()))
x = np.ones((2,2)).astype("float32")
m.set_input("x",x)
m.run()
print(m.get_output(0))
print(m.get_output(1))
print(np.sum(x,dtype=bool))

Apologize for my carelessness. Now it should be the issue of relay.sum() whose behavior is different from np.sum().

[[ True  True]
 [ True  True]]
False
True

[padreofthegame]

I came up on the similar problem working with relay.sum operator. It looks like whenever there is even number of True values in corresponding summing axis result will always be False. Taking this observation into account, it really looks to me that the summing operation works fine, but since the expected output is supposed to be boolean, state of the return value is derived based on the LS bit. That may be the explanation for function returning False for even number of True values. I solved this by just modifying the relay.sum operator in relay using cast operation. Similar thing can be done in topi.sum, but it requires 2 cast operations to be done, which may be unnecessary.

But looking deeper into code, I realized that both relay.sum and topi.sum realizations, depends on the same sum function, which means that both problems may be solved by delegating the job inside that function to tvm::any function instead of tvm::sum, in the case of boolean tensors. Luckily, tvm::sum and tvm::any has the same calling arguments, so probably the solution for this may be changing the line:

return CommReduce(data, axis, tvm::sum, keepdims, atleast1d);

in the corresponding sum function with:

if(data->dtype.is_bool())
return CommReduce(data, axis, tvm::any, keepdims, atleast1d);
else
return CommReduce(data, axis, tvm::sum, keepdims, atleast1d);

@vinx13 @masahi

[masahi]

@padreofthegame that seems good to me! I thought that a "more correct" binary op of sum reduction for boolean would be xor, but any (or or) might be closer to what we want in practice. Feel free to send a PR for such change.

[padreofthegame]

Yeah @masahi, you are absolutely right. In my opinion xor is is also considerable as the some kind of sum operator in boolean algebra.

But one more observation I came up today working on PR is that numpy actually doesn't care about the fact that the input data are of type boolean. It looks like it consider them as zeroes and ones, and it just accumulate corresponding values for True and False. For example, it even this

np.sum(np.array([[True, True], [True, True]]), axis = ())

evaluates as

array([[1, 1],
[1, 1]])

So looking at the example reported in this issue (print(np.sum(x,dtype=bool))), it may be reasonable to think that in this case numpy do additional cast operation after summation, and maybe better option here, from the aspect of topi.sum or relay.sum, would be to interpret all boolean data as integer values 0 or 1, and keep the current realization of sum function?

[masahi]

Didn't get what you meant, are you saying that we might want to cast bool to int before summation? If the result is > 1, we need to convert that to 1.

[padreofthegame]

Yes and no. Why don't return integer result (as numpy does), which will represent number of True values in corresponding axis. For example:

data = np.array([[True, True, True], [False, True, True], [False, False, True]])
result = np.sum(data, (0,))

print(result)
print(result.dtype)

======================
array([1, 2, 3])
dtype('int64')

Difference with the example above is that there we explicitly requested output to be boolean in the call of np.sum(x,dtype=bool), providing the dtype parameter. I also suggest maybe to keep it simple and in the case of boolean inputs to cast them to int and provide them to sum operator. It should result also with integer output. But if someone wants output to be of other type, it should say that explicitly in the relay or topi definition (by calling cast on obtained result).

[masahi]

I prefer not to do that. If user wants to sum a boolean array for the purpose of counting, cast should be explicitly done at the Relay level.