-
Notifications
You must be signed in to change notification settings - Fork 251
Add comprehensive copilot instructions for RMG-Py #2881
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
Create .github/copilot-instructions.md to guide AI assistants working in this codebase. Includes: - Architecture overview (core packages, Cython patterns, key base classes) - Development commands and testing conventions - RMG-database integration and data flow patterns - Documentation maintenance guidelines (especially input.rst) - Code patterns for molecules, species, and reactions
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:00:56 aromatics Passed Core Comparison ✅Original model has 15 species. aromatics Failed Edge Comparison ❌Original model has 106 species. Non-identical thermo! ❌
Identical thermo comments: DetailsObservables Test Case: Aromatics Comparison✅ All Observables varied by less than 0.500 on average between old model and new model in all conditions! aromatics Passed Observable Testing ✅Regression test liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:01:58 liquid_oxidation Passed Core Comparison ✅Original model has 37 species. liquid_oxidation Failed Edge Comparison ❌Original model has 214 species. DetailsObservables Test Case: liquid_oxidation Comparison✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! liquid_oxidation Passed Observable Testing ✅Regression test nitrogen:Reference: Execution time (DD:HH:MM:SS): 00:00:01:03 nitrogen Passed Core Comparison ✅Original model has 41 species. nitrogen Passed Edge Comparison ✅Original model has 133 species. DetailsObservables Test Case: NC Comparison✅ All Observables varied by less than 0.200 on average between old model and new model in all conditions! nitrogen Passed Observable Testing ✅Regression test oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:01:50 oxidation Passed Core Comparison ✅Original model has 59 species. oxidation Passed Edge Comparison ✅Original model has 230 species. DetailsObservables Test Case: Oxidation Comparison✅ All Observables varied by less than 0.500 on average between old model and new model in all conditions! oxidation Passed Observable Testing ✅Errors occurred during observable testing
WARNING:root:Initial mole fractions do not sum to one; normalizing.
|
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.
Pull request overview
This PR adds comprehensive Copilot instructions to guide AI coding agents (GitHub Copilot, Claude, Cursor, etc.) when working with the RMG-Py codebase. The instructions provide essential context about the project's architecture, development workflows, testing conventions, and database integration that AI agents typically lack.
Changes:
- Added
.github/copilot-instructions.mdwith detailed guidance on RMG-Py architecture, Cython patterns, development commands, testing conventions, database integration, and documentation maintenance
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 4. Returns `Arrhenius` or pressure-dependent kinetics model | ||
|
|
||
| ## External Dependencies | ||
| - **RMG-database**: Set location via `RMG_DATABASE_BRANCH` env var in CI, or pass path to `database.load()` |
Copilot
AI
Feb 10, 2026
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.
RMG_DATABASE_BRANCH in CI controls which branch of RMG-database gets cloned; it does not set the database location. For local/dev guidance, point readers to settings['database.directory'] (defaulting to ../RMG-database/input) and/or configuring database.directory in an rmgrc file.
| - **RMG-database**: Set location via `RMG_DATABASE_BRANCH` env var in CI, or pass path to `database.load()` | |
| - **RMG-database**: In CI, `RMG_DATABASE_BRANCH` controls which RMG-database branch is cloned. Locally, the database location is set via `settings['database.directory']` (default `../RMG-database/input`) or `database.directory` in an `rmgrc` file; you may also pass an explicit path to `database.load()`. |
| 1. `Species.get_thermo_data()` → `ThermoDatabase.get_thermo_data(species)` | ||
| 2. First checks thermo libraries for exact match (via graph isomorphism) | ||
| 3. Falls back to group additivity estimation using functional group contributions | ||
| 4. Returns `ThermoData`, `NASA`, or `Wilhoit` object |
Copilot
AI
Feb 10, 2026
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.
The species thermo flow is oversimplified/inaccurate: Species.get_thermo_data() goes through rmgpy.thermo.thermoengine.submit() and may resolve futures, rather than directly calling ThermoDatabase.get_thermo_data(species). Suggest updating this section to reflect the actual call path so agents know where to make changes.
| 1. `Species.get_thermo_data()` → `ThermoDatabase.get_thermo_data(species)` | |
| 2. First checks thermo libraries for exact match (via graph isomorphism) | |
| 3. Falls back to group additivity estimation using functional group contributions | |
| 4. Returns `ThermoData`, `NASA`, or `Wilhoit` object | |
| 1. `Species.get_thermo_data()` → `rmgpy.thermo.thermoengine.submit(species)` (may create/resolve futures) | |
| 2. `thermoengine` dispatches to the loaded `ThermoDatabase` and ultimately calls `ThermoDatabase.get_thermo_data(species)` | |
| 3. `ThermoDatabase` first checks thermo libraries for an exact match (via graph isomorphism) | |
| 4. If no library match is found, `ThermoDatabase` falls back to group additivity estimation using functional group contributions | |
| 5. The resolved result is returned as a `ThermoData`, `NASA`, or `Wilhoit` object |
| - Follow PEP 8 | ||
| - Docstrings describe purpose, not implementation | ||
| - Use `logging` module (not print statements) | ||
| - MIT/X11 license header required on all source files |
Copilot
AI
Feb 10, 2026
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.
The repository license is the standard MIT text (see LICENSE.txt). Consider changing "MIT/X11" to simply "MIT" (or match the exact wording used in file headers) to avoid ambiguity.
| - MIT/X11 license header required on all source files | |
| - MIT license header required on all source files |
| make test-database # Run database tests | ||
| make test-all # Run all tests | ||
| make clean # Remove build artifacts | ||
| make decython # Remove .so files for "pure Python" debugging. This target probably broken. |
Copilot
AI
Feb 10, 2026
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.
The note on make decython says "This target probably broken." which is both grammatically incorrect and an unverified/subjective claim. Consider rephrasing to a factual statement (e.g., describe known limitations or remove the claim) so the instructions stay accurate.
| make decython # Remove .so files for "pure Python" debugging. This target probably broken. | |
| make decython # Remove .so files for "pure Python" debugging. |
| ### Database Structure (in RMG-database repo) | ||
| - `thermo/` - Thermodynamic libraries and group additivity data | ||
| - `kinetics/families/` - Reaction family templates with rate rules (e.g., `H_Abstraction`, `R_Addition_MultipleBond`) | ||
| - `kinetics/libraries/` - Curated rate coefficient libraries | ||
| - `solvation/` - Solvent and solute parameters | ||
| - `transport/` - Transport properties |
Copilot
AI
Feb 10, 2026
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.
The "Database Structure" paths listed here (e.g., thermo/, kinetics/, etc.) are under the RMG-database input/ directory when used by RMG-Py (see default settings['database.directory'] pointing to RMG-database/input). Please clarify the base path (e.g., RMG-database/input/thermo/) to avoid confusion.
| ### Database Structure (in RMG-database repo) | |
| - `thermo/` - Thermodynamic libraries and group additivity data | |
| - `kinetics/families/` - Reaction family templates with rate rules (e.g., `H_Abstraction`, `R_Addition_MultipleBond`) | |
| - `kinetics/libraries/` - Curated rate coefficient libraries | |
| - `solvation/` - Solvent and solute parameters | |
| - `transport/` - Transport properties | |
| ### Database Structure (in RMG-database `input/` directory) | |
| The main database subdirectories live under the `input/` directory (e.g., `RMG-database/input/thermo/`): | |
| - `RMG-database/input/thermo/` - Thermodynamic libraries and group additivity data | |
| - `RMG-database/input/kinetics/families/` - Reaction family templates with rate rules (e.g., `H_Abstraction`, `R_Addition_MultipleBond`) | |
| - `RMG-database/input/kinetics/libraries/` - Curated rate coefficient libraries | |
| - `RMG-database/input/solvation/` - Solvent and solute parameters | |
| - `RMG-database/input/transport/` - Transport properties |
| database.load( | ||
| path='/path/to/RMG-database', | ||
| thermo_libraries=['primaryThermoLibrary'], | ||
| kinetics_families='default', | ||
| reaction_libraries=[], | ||
| ) |
Copilot
AI
Feb 10, 2026
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.
In the RMGDatabase.load() example, path should point to the database input folder that contains thermo/, kinetics/, etc. (typically .../RMG-database/input), not the repo root. As written, the example will fail unless the user happens to pass the input directory.
Motivation or Problem
AI coding agents (GitHub Copilot, Claude, Cursor, etc.) lack context about RMG-Py's architecture, conventions, and workflows when assisting with development. Without guidance, they may generate code that doesn't follow project patterns, miss Cython requirements, or fail to update documentation appropriately.
Description of Changes
Added
.github/copilot-instructions.mdto provide AI coding agents with essential context including:RMGObject,Graph)make install,make test, etc.)users/rmg/input.rstfile)Testing
Reviewer Tips