jwfromm commented on a change in pull request #6160:
URL: https://github.com/apache/incubator-tvm/pull/6160#discussion_r461686793
##########
File path: tests/python/frontend/onnx/test_forward.py
##########
@@ -2374,17 +2374,11 @@ def test_pooling():
auto_pad='SAME_UPPER')
-def verify_mod(x_shape, y_shape, fmod, dtype='float32'):
+def verify_mod(x_shape, y_shape, fmod, out_shape, dtype='float32'):
x_np = np.random.uniform(size=x_shape).astype(dtype)
y_np = np.random.uniform(size=y_shape).astype(dtype)
Review comment:
These tests are only passing because `np.random.uniform` is always
positive. If you use negative numbers, the behavior of `mod` changes based on
the value of `fmod`. We should use a different random input generator than
uniform anyway since values between 0 and 1 dont make sense for mod testing
anyway.
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -530,10 +530,7 @@ class Mod(OnnxOpConverter):
@classmethod
def _impl_v1(cls, inputs, attr, params):
assert len(inputs) == 2, "Mod op take 2 inputs, {}
given".format(len(inputs))
- if attr['fmod'] == 1:
- op_name = "floor_mod"
- else:
- op_name = "mod"
+ op_name = "mod"
Review comment:
Some handling for `fmod=1` will need to be added here. From the onnx
operator definition, `fmod=1` implies that the sign of the dividend should be
kept in the output. When the dividend is negative, this differs from default
relay behavior. We'll probably have to multiply the output by
`relay.sign(inputs[0])` when `fmod=1` to make this work.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
