[Feat][Platform] Add ops_manifest.yaml with rmsnorm_fwd entry#636
[Feat][Platform] Add ops_manifest.yaml with rmsnorm_fwd entry#636lcy-seso wants to merge 2 commits intotile-ai:mainfrom
Conversation
Summary of ChangesHello, 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 establishes a foundational mechanism for defining and managing operations within the TileOPs framework by introducing 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. 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. Footnotes
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a well-structured ops_manifest.yaml for spec-driven op registration and includes a comprehensive test suite. The implementation is solid. My feedback focuses on improving the accuracy of the roofline model, enhancing readability in the manifest, and adhering to Python style conventions in the test file. These adjustments will increase the correctness and maintainability of the new components.
There was a problem hiding this comment.
Code Review
This pull request introduces a new ops_manifest.yaml file to serve as a centralized, spec-driven registry for operations, starting with rmsnorm_fwd. It also adds a comprehensive test suite to validate the manifest's structure and content. The changes are well-structured and the tests are thorough. I have a few suggestions to improve the accuracy of the roofline model in the manifest and to align the test code with standard Python style conventions.
There was a problem hiding this comment.
Pull request overview
Adds a spec-driven operator registry to the repo and validates it via a new test suite, using rmsnorm_fwd as the first registered op.
Changes:
- Introduce a root-level
ops_manifest.yamldefining the initial manifest schema and registeringrmsnorm_fwd. - Add
tests/test_ops_manifest.pyto validate manifest structure, required fields, source-path existence, and workload coverage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
ops_manifest.yaml |
Adds the initial ops manifest schema and rmsnorm_fwd entry (signature/workloads/roofline/source). |
tests/test_ops_manifest.py |
Adds pytest validation for manifest schema, required fields, source paths, and workload completeness. |
817a051 to
8082ede
Compare
Create spec-driven op registry at repo root with rmsnorm_fwd as the first entry. Includes signature, workloads (Llama-3.1 8B/70B/405B prefill+decode), roofline cost model, and source paths. Adds validation test suite and pyyaml dev dependency. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
7db2cc9 to
744edc0
Compare
Closes #627
Summary
ops_manifest.yamlat repo root as the spec-driven op registry, establishing the schema for all future op definitions.rmsnorm_fwdas the first entry with all four required fields:signature,workloads,roofline, andsource.tests/test_ops_manifest.py) with 16 tests covering schema structure, field presence, source path existence, and workload completeness.Test plan
python -m pytest tests/test_ops_manifest.py-- 16/16 pass🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com