Skip to content

Conversation

@AshwiniBokka
Copy link

No description provided.

- Add prerequisite note about building TVM with CUDA support
- Add error handling for 'tvm_ffi' import failure with helpful message
- Direct users to build instructions when TVM is not built
- Fixes issue apache#18584 where users get confusing import errors

Test Plan:
- File compiles without syntax errors
- Error message guides users to correct solution
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @AshwiniBokka, 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 primarily focuses on enhancing the user experience and reliability of a specific tutorial by addressing a common dependency issue. It ensures that users encountering problems related to CUDA support receive clear guidance. Additionally, it includes minor but important updates to license links, promoting secure access to Apache License documentation.

Highlights

  • Tutorial Robustness: The customize_opt.py tutorial has been made more robust by adding explicit checks and error messages for TVM's CUDA build dependency, guiding users on how to resolve ImportError related to tvm_ffi.
  • License URL Updates: All Apache License URLs across LICENSE and README.md files have been updated from http to https for improved security and best practices.
  • File Encoding: A UTF-8 Byte Order Mark (BOM) has been added to the customize_opt.py file.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request primarily updates HTTP links to HTTPS across the repository, which is a good security improvement. It also enhances the customize_opt.py tutorial by adding a check for CUDA support, improving the user experience. My review includes a few suggestions for this tutorial script to align with best practices, such as removing an unnecessary Byte Order Mark, directing error messages to stderr, and cleaning up trailing newlines.

@@ -1,4 +1,4 @@
# Licensed to the Apache Software Foundation (ASF) under one
# Licensed to the Apache Software Foundation (ASF) under one
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This line contains a Unicode Byte Order Mark (BOM) \ufeff, which is unnecessary for UTF-8 encoded files and can cause issues with some tools. It should be removed.

Suggested change
# Licensed to the Apache Software Foundation (ASF) under one
# Licensed to the Apache Software Foundation (ASF) under one

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls take a look

Comment on lines 116 to 119
print("Error: TVM needs to be built with CUDA support.")
print("Please build TVM from source with CUDA enabled.")
print("See: https://tvm.apache.org/docs/install/from_source.html")
import sys
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Error messages should be printed to standard error (stderr) instead of standard output (stdout). This is a standard practice for separating diagnostic output from normal program output. Also, it's better to move the import sys statement to the top of the except block.

Suggested change
print("Error: TVM needs to be built with CUDA support.")
print("Please build TVM from source with CUDA enabled.")
print("See: https://tvm.apache.org/docs/install/from_source.html")
import sys
import sys
print("Error: TVM needs to be built with CUDA support.", file=sys.stderr)
print("Please build TVM from source with CUDA enabled.", file=sys.stderr)
print("See: https://tvm.apache.org/docs/install/from_source.html", file=sys.stderr)

# iterate the optimization and improve the performance of the model.
#


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There are multiple empty lines at the end of the file. It's a common convention to have only a single newline at the end of a file. Please remove the extra ones.

- Properly indent except ImportError block
- Remove duplicate import sys statement
- Ensure if/else block has correct indentation
- All error messages use sys.stderr
- File now compiles without syntax errors

Addresses all code review feedback
@@ -1,4 +1,4 @@
# Licensed to the Apache Software Foundation (ASF) under one
# Licensed to the Apache Software Foundation (ASF) under one
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls take a look

from tvm.relax.frontend import nn

# Note: This tutorial requires TVM to be built with CUDA support.
# If you encounter 'No module named ''tvm_ffi''' error,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tvm_ffi can be installed with cd 3rdparty/tvm-ffi; pip install .

Apache License
Version 2.0, January 2004
http://www.apache.org/licenses/
https://www.apache.org/licenses/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The LICENSE update isn’t related to this PR. I’d suggest we avoid changing it here and handle it in a separate PR if needed.


try:
import tvm.relax.backend.cuda.cublas as _cublas
except ImportError as e:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably we don't have to handle the tvm_ffi specifically here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants