[RFC] TileOPs Architecture Refactoring#170
Conversation
Summary of ChangesHello @zhen8838, 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 introduces a Request for Comments (RFC) document detailing a significant architectural refactoring for the TileOPs project. The primary motivation is to simplify the system, making it easier for developers to contribute new operators, improve overall code quality, and accelerate development cycles by addressing current issues like excessive abstraction layers and coupled testing/benchmarking practices. Highlights
🧠 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. Changelog
Activity
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 is a well-structured and detailed RFC for refactoring the TileOPs architecture. The proposal effectively communicates the current problems and the planned solutions. My review includes a few minor suggestions to correct typos, resolve an inconsistency in the proposed test structure, and improve the accuracy of a performance metric calculation in an example. Overall, the proposed changes seem well-considered and should significantly improve the developer experience.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This RFC proposes a comprehensive architectural refactoring of the TileOPs codebase to reduce development complexity and improve efficiency. The document outlines a plan to eliminate redundant abstraction layers (layers/ and functions/ directories), separate test and benchmark responsibilities, and introduce an experimental directory for rapid prototyping.
Changes:
- Proposes eliminating the four-layer architecture (layers → functions → ops → kernels) down to two layers (ops → kernels)
- Introduces separation of concerns between correctness tests and performance benchmarks through new TestBase and BenchmarkBase classes
- Adds new experimental/ directory for prototyping and microbenchmarks/ directory for GPU micro-benchmarks
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | `tests/experimental/` | Experimental operator tests | New | | ||
| | `benchmarks/` | Performance benchmarks | Core | | ||
| | `benchmarks/microbenchmarks/` | GPU micro-benchmarks | New | | ||
|
|
There was a problem hiding this comment.
Maybe ops/experimental/ and kernels/experimental where we can place experimental operator interfaces and experimental tilelang kernels.
| Directory | Responsibility | Status |
|---|---|---|
ops/ |
Stable operator implementations | Core |
ops/experimental/ |
Experimental operator implementations | New |
kernels/ |
High-performance kernel implementations | Core |
kernels/experimental/ |
Experimental kernel implementations | New |
tests/ |
Correctness tests | Core |
tests/experimental/ |
Experimental operator tests | New |
benchmarks/ |
Performance benchmarks | Core |
benchmarks/microbenchmarks/ |
GPU micro-benchmarks | New |
|
If you want to completely separate benchmarks from tests, it is recommended to add a utility module to uniformly manage test parameters. Managing pytest parameters separately for tests and benchmarks can lead to redundancy and potential inconsistencies in parameters. Using a set of parameters that have not been verified for correctness to test performance seems meaningless. |
|
I think the current benchmarks don't need to be executed automatically via |
|
After reading through this RFC, my takeaway is that we should first focus on making the high-level decisions and then move on to the “how.” There are four key points:
I think the first two can be agreed on quickly and moved into action. The latter two can be introduced first, and then we can decide what content should go into each category. |
I think this falls into the “how” category. That's it to say we all agree that unit tests should be separated from benchmarks, as they are two very different things. |
Description
This RFC proposes a comprehensive refactoring to reduce development barriers, optimize architecture design, and improve development efficiency.
Type of Change
Checklist
pre-commit run --all-filesand fixed all linting issues.Benchmarkclass inbenchmarks/.