jiangjiajun commented on pull request #9126: URL: https://github.com/apache/tvm/pull/9126#issuecomment-933273751
> Coming together well. I think I am ok with merging this in the current state as part of a more experimental frontend, but would like another pair of eyes on this. > > Still a few comments which will make this better: > > 1. Can you add more substantial test cases to your tests? E.g. different input shapes and those of different ranks at least for some of the relevant ops. I feel that some issues might come to light from this. Do not worry about PR size anymore > 2. Please add type annotations to functions for this and future PRs. e.g. > > def myFunc(a: int, b: List[string]) -> None: ... - More cases have been added in tests, but only for the new operators in this pull request. I will send another pull request for the previous operators. - Type annotation is a good code habit, but for the function like `convert_dot(g : GraphProto, op : paddle.fluid.framework.operators, block: paddle.fluid.framework.Block)`, the type annotation will bring dependency of paddlepaddle for TVM, I noticed that all the frontends putting framework importing in `from_xxx` function to avoid strong dependency for TVM. -- 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]
