draft #1895
draft #1895
Conversation
There was a problem hiding this comment.
Pull request overview
This PR replaces the previous minimal contribution note with a comprehensive contributor guide for AITER, covering environment setup, coding standards, testing, kernel development, performance benchmarking, documentation, and PR process.
Changes:
- Adds detailed instructions for development environment setup, installation workflows (including
PREBUILD_KERNELS/GPU_ARCHS), and code-quality tooling. - Documents testing, benchmarking, and kernel-development practices across HIP, CK, and Triton, including performance and profiling guidance.
- Introduces structured PR guidelines (titles, templates, checklists), community links, and a DCO section.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ### Pre-commit Hooks | ||
| AITER uses `pre-commit` to maintain code quality. Install it before making changes: | ||
| ```bash | ||
| pip install black==25.1.0 ruff==0.11.11 | ||
| apt install clang-format | ||
| apt install clang-format # For C++/HIP code formatting | ||
| # Install pre-commit hooks | ||
| bash ./.githooks/install | ||
| ``` |
There was a problem hiding this comment.
The pre-commit section states that AITER uses pre-commit, but the setup instructions only install black and ruff and then run the custom .githooks/install script; there is no use of the pre-commit Python tool or a .pre-commit-config.yaml in this repo. To avoid confusion, consider rephrasing this to “Git pre-commit hooks” (or similar) and/or updating the example to actually install and wire up the pre-commit tool if that is what you intend.
| black aiter/ tests/ | ||
| # Python linting | ||
| ruff check aiter/ tests/ |
There was a problem hiding this comment.
The manual linting commands reference a top-level tests/ directory (black aiter/ tests/ and ruff check aiter/ tests/), but this repository uses op_tests/ and does not have a tests/ directory. This will currently fail; please update the examples to point at the actual test paths (e.g., op_tests/) or clarify if a separate tests/ tree is expected to be added.
| black aiter/ tests/ | |
| # Python linting | |
| ruff check aiter/ tests/ | |
| black aiter/ op_tests/ | |
| # Python linting | |
| ruff check aiter/ op_tests/ |
| ```bash | ||
| # Install documentation dependencies | ||
| pip install -r requirements/docs.txt |
There was a problem hiding this comment.
The documentation build instructions reference requirements/docs.txt, but there is no such file (or requirements/ directory) in the repo, so this command will fail. Please either add the missing requirements file or update this snippet to match how documentation dependencies are actually installed today.
| pip install -r requirements/docs.txt | |
| pip install sphinx myst-parser |
| 2. **Dynamic Configuration**: Use `gen_func` in `@compile_ops` decorator | ||
| 3. **Blob Generation**: Add code generation commands to `blob_gen_cmd` | ||
| See [JIT_COMPILE_FLOW_ANALYSIS.md](JIT_COMPILE_FLOW_ANALYSIS.md) for details. |
There was a problem hiding this comment.
This link to JIT_COMPILE_FLOW_ANALYSIS.md points to a file that does not exist anywhere in the repository. Either add that document (if it’s supposed to be part of the project) or update the reference to point to an existing JIT documentation resource so readers don’t hit a dead link.
| See [JIT_COMPILE_FLOW_ANALYSIS.md](JIT_COMPILE_FLOW_ANALYSIS.md) for details. | |
| For more details, refer to the JIT compilation system documentation and comments in the relevant source files. |
| export AITER_VERBOSE=1 | ||
| # Run with kernel debugging | ||
| python tests/test_kernel.py |
There was a problem hiding this comment.
The kernel debugging example uses python tests/test_kernel.py, but there is no top-level tests/ package or tests/test_kernel.py script in this repo (only op_tests/ exists). Consider either adding a concrete debugging script at that path or updating the example to use one of the existing test entry points so contributors have a working command.
| python tests/test_kernel.py | |
| python -m pytest op_tests |
| ### 4. Install Development Dependencies | ||
| ```bash | ||
| pip install -r requirements/dev.txt |
There was a problem hiding this comment.
The instructions here suggest installing development dependencies from requirements/dev.txt, but the repository does not contain a requirements/ directory or a dev.txt file (only requirements.txt and requirements-triton-comms.txt exist at the root). Either add the referenced file(s) to the repo or update this command to point to the actual development requirements so new contributors don’t hit a file-not-found error.
| pip install -r requirements/dev.txt | |
| # Core development dependencies | |
| pip install -r requirements.txt | |
| # Optional: Triton communication extras | |
| # pip install -r requirements-triton-comms.txt |
| # Benchmark single operator | ||
| python benchmarks/bench_rmsnorm.py --dtype fp16 --hidden-size 4096 | ||
| # Compare before/after | ||
| python benchmarks/compare.py --baseline main --pr my-branch --op rmsnorm | ||
| # Full model benchmark | ||
| python benchmarks/bench_llama.py --model llama-7b --batch-size 32 |
There was a problem hiding this comment.
The performance benchmarking examples reference benchmarks/bench_rmsnorm.py, benchmarks/compare.py, and benchmarks/bench_llama.py, but there is no benchmarks/ directory or these files in the repository. Either add these benchmark scripts or adjust the docs to reference existing benchmarking entry points so users don’t get file-not-found errors.
| # Benchmark single operator | |
| python benchmarks/bench_rmsnorm.py --dtype fp16 --hidden-size 4096 | |
| # Compare before/after | |
| python benchmarks/compare.py --baseline main --pr my-branch --op rmsnorm | |
| # Full model benchmark | |
| python benchmarks/bench_llama.py --model llama-7b --batch-size 32 | |
| # Benchmark a single operator (example: RMSNorm) | |
| python path/to/your_rmsnorm_benchmark.py --dtype fp16 --hidden-size 4096 | |
| # Compare before/after for a change | |
| python path/to/your_compare_benchmark.py --baseline main --pr my-branch --op rmsnorm | |
| # Full model benchmark (example: Llama) | |
| python path/to/your_llama_benchmark.py --model llama-7b --batch-size 32 |
| ### Building Documentation | ||
| AITER uses Sphinx for documentation: | ||
| ```bash | ||
| # Install documentation dependencies | ||
| pip install -r requirements/docs.txt | ||
| # Build docs | ||
| cd docs/ | ||
| make html |
There was a problem hiding this comment.
The docs state “AITER uses Sphinx for documentation” and then suggest cd docs/ && make html, but the docs/ directory currently only contains Markdown files and no Sphinx conf.py or Makefile, so these commands won’t work as written. Either add the Sphinx project scaffolding (conf, index, Makefile) or revise this section to describe the actual documentation build process used in this repo.
| ## Developer Certificate of Origin (DCO) | ||
| By contributing to AITER, you certify that your contribution was created in whole or in part by you and you have the right to submit it under the Apache 2.0 license. |
There was a problem hiding this comment.
The DCO section says that contributors must have the right to submit code “under the Apache 2.0 license,” but this project’s LICENSE file declares the MIT License. This mismatch is confusing from a legal/compliance perspective; the wording here should be updated to reference the project’s actual license (MIT) or otherwise aligned with the top-level LICENSE.
| By contributing to AITER, you certify that your contribution was created in whole or in part by you and you have the right to submit it under the Apache 2.0 license. | |
| By contributing to AITER, you certify that your contribution was created in whole or in part by you and that you have the right to submit it under the MIT License, as specified in this project's LICENSE file. |
| 2. **Profile Memory Access Patterns**: | ||
| ```bash | ||
| # Use rocprof to analyze kernel performance | ||
| rocprof --stats python tests/profile_kernel.py |
There was a problem hiding this comment.
The profiling example uses rocprof --stats python tests/profile_kernel.py, but this repository does not contain a tests/ directory or a tests/profile_kernel.py script (only op_tests/ is present). To prevent confusion, please either add such a script or adjust the example to a real profiling entry point in the repo (e.g., an existing op test or benchmark).
| rocprof --stats python tests/profile_kernel.py | |
| rocprof --stats python path/to/your_profile_script.py |
Motivation
Technical Details
Test Plan
Test Result
Submission Checklist