lhutton1 commented on a change in pull request #9521:
URL: https://github.com/apache/tvm/pull/9521#discussion_r752149533



##########
File path: python/tvm/relay/op/contrib/ethosu.py
##########
@@ -537,9 +538,14 @@ def is_valid(self):
         """
         if np.dtype(self.ofm) == np.int32 and self.activation is not None:

Review comment:
       Good point. Ln541 should really be checking the ofm rather than the ifm

##########
File path: tests/python/contrib/test_ethosu/test_codegen.py
##########
@@ -349,6 +349,7 @@ def representative_dataset():
         ([1, 2, 3, 4], [1, 2, 3, 4]),
         ([1, 2, 3, 4], [1, 1, 1, 1]),
         ([1, 1, 1, 1], [1, 2, 3, 4]),
+        ([1, 4, 4], [4, 1]),

Review comment:
       I was being a bit wary here about adding too many tests as adding all 3 
cases would lead to 120 additional tests rather than just 40 :) I wonder if its 
worth testing this separately for just `ADD` for example as the underlying 
operation doesn't matter for this change?




-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to