tvm-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <>
Subject [GitHub] [incubator-tvm] u99127 commented on pull request #5510: [FRONTEND][TFLite] Fully connected op conversion made in sync with TFLite
Date Tue, 05 May 2020 17:42:18 GMT

u99127 commented on pull request #5510:

   > > Minor nits, A better commit message would be to refer to handling batch matmul
in fully connected op rather than what it stands.
   > > Isn't there a need for Quantized inputs testing here ?
   > @u99127 : Thanks for your comments! Just for your info, TFLite converter has multiple
limitations in terms of various input types supported, so it is not possible always to add
all the cases.
   > I had already tried for it, but converter has errors. 
   Thanks for trying it and clarifying the situation with qnn ops.
   > Also it does not support N-dim.
   > About the commit, actually the changes are quite unrelated to batch matmul, it is
just that open up the issue in implementation. So IMO the commit message looks ok to me. Please
let me know if you think otherwise. Thanks!
   In my experience a good git commit message, explains what was changed and why. It's ok
if the body of the commit message is empty as long as the subject line can give a quick and
obvious summary to the reader. 
   The subject line and the commit message on a quick perusal last night suggested that this
commit synchronizes the implementation of fully connected with tflite. Dropping an assert
always makes me query why it was dropped which is why I queried it.

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:

View raw message