[DOCS] Update tutorial for exporting and loading back Relax executables#18412
[DOCS] Update tutorial for exporting and loading back Relax executables#18412tqchen merged 3 commits intoapache:mainfrom
Conversation
Summary of ChangesHello @tlopex, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request updates the tutorial for exporting and loading Relax executables. The primary goal is to reflect recent API changes by migrating the compilation step from Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request updates the tutorial for exporting and loading Relax executables by replacing the deprecated relax.build with tvm.compile. While this is a good update, the associated changes to handle model outputs are buggy. The logic for unpacking the output tensor is not robust and can lead to incorrect behavior for single-tensor outputs. Additionally, the example script in the documentation contains incorrect variable names that would cause it to fail. I have provided suggestions to fix these issues.
| if hasattr(tvm_output, "__len__") and len(tvm_output) > 0: | ||
| result_tensor = tvm_output[0] | ||
| else: | ||
| result_tensor = tvm_output |
There was a problem hiding this comment.
The logic to extract the result tensor is not robust. The check hasattr(tvm_output, "__len__") is too broad and can lead to incorrect behavior for single tensor outputs. For example, a tvm.runtime.NDArray has a __len__ attribute, and this check would cause it to be sliced, which is likely not the intended behavior for a single tensor output.
A more precise way to handle this is to check if the output is a sequence type that is not a tensor, like tvm.runtime.Array, tuple, or list.
I suggest the following change for a more robust implementation:
| if hasattr(tvm_output, "__len__") and len(tvm_output) > 0: | |
| result_tensor = tvm_output[0] | |
| else: | |
| result_tensor = tvm_output | |
| if isinstance(tvm_output, (tvm.runtime.Array, tuple, list)): | |
| result_tensor = tvm_output[0] | |
| else: | |
| result_tensor = tvm_output |
| # if hasattr(tvm_output, "__len__") and len(tvm_output) > 0: | ||
| # result_tensor = tvm_output[0] | ||
| # else: | ||
| # result_tensor = tvm_output |
There was a problem hiding this comment.
This example code has a few issues:
- It uses
tvm_output, but the variable defined in this example script isoutput(fromoutput = vm["main"](input_tensor, *params)). - It defines
result_tensor, but the following print statements useresult. This will cause aNameError. - The logic for unpacking the output is incorrect for single tensor outputs, similar to the issue in the main script.
I suggest correcting the variable names and using a more robust check for the output type.
| # if hasattr(tvm_output, "__len__") and len(tvm_output) > 0: | |
| # result_tensor = tvm_output[0] | |
| # else: | |
| # result_tensor = tvm_output | |
| # if isinstance(output, (tvm.runtime.Array, tuple, list)): | |
| # result = output[0] | |
| # else: | |
| # result = output |
Remove print statement for skipping model conversion.
This pr updates
relax.buildtotvm.compile, and fixes some bugs when we get output and on the website