-
Notifications
You must be signed in to change notification settings - Fork 12
update kernel generator example to be more clear #101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe kernel generator example is enhanced with definition filtering capability, allowing users to select a specific definition for processing instead of only rmsnorm definitions. It adds validation, informational messaging, and modifies the traceset path from an absolute to a generic placeholder. Changes
Sequence DiagramsequenceDiagram
participant User
participant Generator as Kernel Generator
participant Validator as Definition Validator
participant Processor as Definition Processor
User->>Generator: Run with optional definition parameter
alt Specific definition requested
Generator->>Validator: Validate definition exists
alt Definition found
Validator-->>Generator: Valid
Generator->>User: Print selected scope
Generator-->>User: Complete (single definition)
else Definition not found
Validator-->>Generator: Not found
Generator->>User: Print error message
Generator-->>User: Exit
end
else No specific definition
Generator->>User: Print full scope (all definitions)
Generator->>Processor: Process all definitions
Processor-->>Generator: Complete
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)examples/kernel_generator/kernel_generator_example.py (1)
🔇 Additional comments (3)
Comment |
Summary of ChangesHello @zanderjiang, 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 refines the 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.
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 improves the kernel generator example by making it more flexible and clearer for users. It introduces an option to generate a solution for a specific definition and replaces a hardcoded user path with a placeholder. My feedback includes a suggestion to improve user-friendliness by adding a check for a valid traceset path, and a minor style fix to align with PEP 8 guidelines for inline comments.
| language = "triton" # Target solution language | ||
| target_gpu = "B200" # Choose solution target GPU | ||
| definition = "" # Leave empty to generate solutions for all definitions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better readability and adherence to Python's PEP 8 style guide, inline comments should be separated by at least two spaces from the statement. I've also aligned the comments for better visual structure.1
| language = "triton" # Target solution language | |
| target_gpu = "B200" # Choose solution target GPU | |
| definition = "" # Leave empty to generate solutions for all definitions | |
| language = "triton" # Target solution language | |
| target_gpu = "B200" # Choose solution target GPU | |
| definition = "" # Leave empty to generate solutions for all definitions |
Style Guide References
Footnotes
-
PEP 8 E261 states that inline comments should be separated by at least two spaces from the statement. ↩
|
|
||
| # TODO: adjust local path to traceset | ||
| traceset_path = "/home/akj2/flashinfer-trace" | ||
| traceset_path = "/path/to/flashinfer-trace" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While using a placeholder path is an improvement, the script doesn't handle the case where the user forgets to change it. If run with the default path, it will create an empty directory, find no definitions, and do nothing without a clear explanation. This can be confusing for new users. It would be more robust to check if any definitions were loaded from the provided path and exit with a helpful error message if not.
For example, you could add this check after loading the traceset:
if not traceset.definitions:
print(f"Error: No definitions found in traceset at '{traceset_path}'.")
print("Please ensure `traceset_path` points to a valid flashinfer-trace directory.")
return
This PR updates the kernel generator example, gives user more freedom to generate their own solutions to run on FlashInfer-Bench
Summary by CodeRabbit
Release Notes
New Features
Chores