Add VRP dimension developer skill#1233
Conversation
rg20
added
Agentic
📝 WalkthroughWalkthroughThis PR adds developer-focused documentation for implementing VRP (Vehicle Routing Problem) dimension logic in cuOpt. A new comprehensive guide ( ChangesVRP Dimension Implementation Guide
🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
skills/cuopt-developer/vrp_skills.md (2)
20-22: ⚡ Quick winConsider adding language specifiers to code blocks.
The pseudo-code blocks at lines 20-22, 52-57, 64-67, 71-74, and 78-81 lack language specifiers. Adding identifiers (e.g.,
pseudo,python, orcpp) improves syntax highlighting and rendering in documentation viewers.📝 Example fix for the combine formula block
-``` +```pseudo combine(k, k+1) = fwd_excess[k] + bwd_excess[k+1] - excess(fwd_state[k])</details> Also applies to: 52-57, 64-67, 71-74, 78-81 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@skills/cuopt-developer/vrp_skills.mdaround lines 20 - 22, Add explicit
language specifiers to each pseudo-code fenced block (e.g., ```pseudo orupdate the blocks that contain the formula "combine(k, k+1) = fwd_excess[k] + bwd_excess[k+1] - excess(fwd_state[k])" and the other pseudo-code blocks referenced at lines 52-57, 64-67, 71-74, and 78-81 by prefixing their opening fences with a language token (pseudo, python, or cpp) while leaving the block contents unchanged.
24-36: ⚡ Quick winAdd blank lines around the interface table.
Markdown best practices require blank lines before and after tables for consistent rendering across parsers.
📝 Proposed fix
### Required interface for every dimension + | Method | Description | |--------|-------------| | `calculate_forward(next)` | Propagate fwd state from `this` to `next`; update `next.fwd_excess` | + ---🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/cuopt-developer/vrp_skills.md` around lines 24 - 36, Add a blank line immediately before the table that starts with "### Required interface for every dimension" and another blank line immediately after the table so Markdown renders it consistently; locate the table by the method names listed (calculate_forward, calculate_backward, combine, get_cost, compute_cost, forward_excess, backward_excess, forward_feasible, backward_feasible) and insert a single empty line above the "### Required interface for every dimension" heading (or between the previous paragraph and the heading) and one empty line after the table before the following content/heading.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@skills/cuopt-developer/vrp_skills.md`:
- Around line 20-22: Add explicit language specifiers to each pseudo-code fenced
block (e.g., ```pseudo or ```python) so documentation renderers apply proper
highlighting; specifically update the blocks that contain the formula
"combine(k, k+1) = fwd_excess[k] + bwd_excess[k+1] - excess(fwd_state[k])" and
the other pseudo-code blocks referenced at lines 52-57, 64-67, 71-74, and 78-81
by prefixing their opening fences with a language token (pseudo, python, or cpp)
while leaving the block contents unchanged.
- Around line 24-36: Add a blank line immediately before the table that starts
with "### Required interface for every dimension" and another blank line
immediately after the table so Markdown renders it consistently; locate the
table by the method names listed (calculate_forward, calculate_backward,
combine, get_cost, compute_cost, forward_excess, backward_excess,
forward_feasible, backward_feasible) and insert a single empty line above the
"### Required interface for every dimension" heading (or between the previous
paragraph and the heading) and one empty line after the table before the
following content/heading.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 60de93ad-54b0-420b-8c46-d19b726de6b6
📒 Files selected for processing (3)
AGENTS.mdskills/cuopt-developer/SKILL.mdskills/cuopt-developer/vrp_skills.md
rgsl888prabhu
left a comment
rgsl888prabhu
left a comment
There was a problem hiding this comment.
Awesome @rg20, first developer skill for core work.
I would suggest review from @hlinsen and @akifcorduk as well for any correctness or improvments.
| @@ -0,0 +1,191 @@ | |||
| # cuOpt VRP Dimension Developer Skills | |||
There was a problem hiding this comment.
Lets move this file under reference/
| ### Rules | ||
| - `skills/cuopt-user-rules/` — Base rules for end users calling cuOpt (routing, LP, MILP, QP, install, server). Not for cuOpt internals — see `skills/cuopt-developer/`. Read first for user-facing tasks; choose skills from the index below by task and interface. | ||
| - `skills/cuopt-developer/` — Modify, build, test, debug, and contribute to cuOpt internals (C++/CUDA, Python, server, CI). Use for solver internals, PRs, DCO, and code conventions. | ||
| - `skills/cuopt-developer/` — Modify, build, test, debug, and contribute to cuOpt internals (C++/CUDA, Python, server, CI). Use for solver internals, PRs, DCO, and code conventions. For **VRP dimension** work (combine invariants, fwd/bwd propagation, new constraints/objectives in the routing engine), read **`skills/cuopt-developer/vrp_skills.md`** in addition to `SKILL.md`. |
There was a problem hiding this comment.
Lets update this accordingly
|
|
||
| --- | ||
|
|
||
| ## Skill 1: `cuopt-dimension-architecture` |
There was a problem hiding this comment.
Lets remove Skill X notation, and just use detail, refer to skills/cuopt-developer/resources/build_and_test.md
| ## Skill 1: `cuopt-dimension-architecture` | |
| ## cuopt-dimension-architecture |
There was a problem hiding this comment.
Same for all other instances
|
|
||
| **When to use**: After writing node/route logic, to ensure the dimension is fully integrated into the framework. | ||
|
|
||
| ### Files to create |
There was a problem hiding this comment.
Did it needed specific instruction to follow this structure ?
|
|
||
| **When to use**: After implementing a new dimension, to write tests that validate correctness end-to-end. | ||
|
|
||
| ### C++ unit tests (`cpp/tests/routing/`) |
There was a problem hiding this comment.
Would C++ test structure and expectation be same as python, may be can we collapse it in to 1 ?
2 participants
Description
Issue
Checklist