skills/cuopt-numerical-optimization-api-c: Add eval - #1327
Conversation
Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
/nvskills-ci |
📝 WalkthroughWalkthroughThis PR adds an "API Call Sequence" to the cuOpt C API skill, adds an eval expecting ordered API entry-point names, reformats C example initializers and updates build instructions, expands the skill card and adds a BENCHMARK report, and replaces the Sigstore DSSE bundle. ChangesC API Documentation and Examples
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
skills/cuopt-numerical-optimization-api-c/SKILL.md (1)
27-27: ⚡ Quick winConsider including solver settings creation in the API sequence.
The documented sequence omits
cuOptCreateSolverSettings, which is required beforecuOptSolveaccording to the example code inassets/lp_basic/lp_simple.c(lines 38, 98). The complete sequence is: create problem → create settings → configure settings → solve → get results → destroy all three objects.📝 Suggested documentation improvement
-For LP/MILP, the ordered C entry points are: `cuOptCreateRangedProblem` (sense `CUOPT_MINIMIZE` / `CUOPT_MAXIMIZE`, CSR constraint matrix as `row_offsets` / `col_indices` / `values`, `var_types` char array using `CUOPT_CONTINUOUS` / `CUOPT_INTEGER` macros) → `cuOptSolve(problem, settings, &solution)` → `cuOptGetObjectiveValue(solution, &obj_value)` → matching `cuOptDestroy*` calls. Include `<cuopt/linear_programming/cuopt_c.h>`. Full ordered code with build instructions in [references/examples.md](references/examples.md). +For LP/MILP, the ordered C entry points are: `cuOptCreateRangedProblem` (sense `CUOPT_MINIMIZE` / `CUOPT_MAXIMIZE`, CSR constraint matrix as `row_offsets` / `col_indices` / `values`, `var_types` char array using `CUOPT_CONTINUOUS` / `CUOPT_INTEGER` macros) → `cuOptCreateSolverSettings(&settings)` → optional parameter configuration → `cuOptSolve(problem, settings, &solution)` → `cuOptGetObjectiveValue(solution, &obj_value)` → `cuOptDestroyProblem`, `cuOptDestroySolverSettings`, `cuOptDestroySolution`. Include `<cuopt/linear_programming/cuopt_c.h>`. Full ordered code with build instructions in [references/examples.md](references/examples.md).🤖 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-numerical-optimization-api-c/SKILL.md` at line 27, The sequence for LP/MILP documentation omits creating solver settings; update the paragraph to include calling cuOptCreateSolverSettings (and any config steps) between cuOptCreateRangedProblem and cuOptSolve so the full ordered flow reads: cuOptCreateRangedProblem → cuOptCreateSolverSettings → configure settings → cuOptSolve(problem, settings, &solution) → cuOptGetObjectiveValue(solution, &obj_value) → matching cuOptDestroy* calls; also mention including <cuopt/linear_programming/cuopt_c.h> as before and mirror the example in assets/lp_basic/lp_simple.c for accuracy.
🤖 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.
Inline comments:
In `@skills/cuopt-numerical-optimization-api-c/evals/evals.json`:
- Around line 4-11: Update the expected list to include the required struct
types and missing API calls in order: include
cuopt/linear_programming/cuopt_c.h, then structs cuOptOptimizationProblem,
cuOptSolverSettings, cuOptSolution, then call cuOptCreateRangedProblem,
cuOptCreateSolverSettings, cuOptSolve(problem, settings, &solution),
cuOptGetObjectiveValue, and finally cleanup with cuOptDestroyProblem,
cuOptDestroySolverSettings, cuOptDestroySolution; reference these exact symbols
(cuOptCreateRangedProblem, cuOptCreateSolverSettings, cuOptSolve,
cuOptGetObjectiveValue, cuOptDestroyProblem, cuOptDestroySolverSettings,
cuOptDestroySolution and the structs) so the eval expects both functions and
structs in the proper sequence.
---
Nitpick comments:
In `@skills/cuopt-numerical-optimization-api-c/SKILL.md`:
- Line 27: The sequence for LP/MILP documentation omits creating solver
settings; update the paragraph to include calling cuOptCreateSolverSettings (and
any config steps) between cuOptCreateRangedProblem and cuOptSolve so the full
ordered flow reads: cuOptCreateRangedProblem → cuOptCreateSolverSettings →
configure settings → cuOptSolve(problem, settings, &solution) →
cuOptGetObjectiveValue(solution, &obj_value) → matching cuOptDestroy* calls;
also mention including <cuopt/linear_programming/cuopt_c.h> as before and mirror
the example in assets/lp_basic/lp_simple.c for accuracy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a204a581-f16d-4750-a98f-59aaa276ed49
📒 Files selected for processing (3)
skills/cuopt-numerical-optimization-api-c/SKILL.mdskills/cuopt-numerical-optimization-api-c/evals/evals.jsonskills/cuopt-numerical-optimization-api-c/references/examples.md
| "question": "I want to solve a small MILP (some integer variables, linear objective, linear constraints) with the cuOpt C API. List the C functions and structs I need in order — names only, one line each, no full source.", | ||
| "expected_skill": "cuopt-numerical-optimization-api-c", | ||
| "expected_script": null, | ||
| "ground_truth": "The agent produces an ordered list of C API entry points without writing a full source file: include cuopt/linear_programming/cuopt_c.h, then call cuOptCreateRangedProblem with sense CUOPT_MINIMIZE or CUOPT_MAXIMIZE, then cuOptSolve(problem, settings, &solution), then cuOptGetObjectiveValue.", | ||
| "expected_behavior": [ | ||
| "Lists C API call sequence without writing a complete source file", | ||
| "Names cuOptCreateRangedProblem, cuOptSolve, cuOptGetObjectiveValue in order" | ||
| ] |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Align question, ground_truth, and expected_behavior for completeness.
The question asks for "C functions and structs I need in order" but the ground_truth and expected_behavior only mention functions. The complete answer should include:
Struct types:
cuOptOptimizationProblemcuOptSolverSettingscuOptSolution
Functions missing from current ground_truth:
cuOptCreateSolverSettings(required before solve, seelp_simple.c:38,98)- The three destroy functions:
cuOptDestroyProblem,cuOptDestroySolverSettings,cuOptDestroySolution(mentioned in SKILL.md line 27 and used inlp_simple.c:95-97)
📝 Proposed fix to complete the evaluation criteria
"question": "I want to solve a small MILP (some integer variables, linear objective, linear constraints) with the cuOpt C API. List the C functions and structs I need in order — names only, one line each, no full source.",
"expected_skill": "cuopt-numerical-optimization-api-c",
"expected_script": null,
- "ground_truth": "The agent produces an ordered list of C API entry points without writing a full source file: include cuopt/linear_programming/cuopt_c.h, then call cuOptCreateRangedProblem with sense CUOPT_MINIMIZE or CUOPT_MAXIMIZE, then cuOptSolve(problem, settings, &solution), then cuOptGetObjectiveValue.",
+ "ground_truth": "The agent produces an ordered list of C API types and entry points without writing a full source file: include cuopt/linear_programming/cuopt_c.h, declare cuOptOptimizationProblem/cuOptSolverSettings/cuOptSolution structs, then call cuOptCreateRangedProblem with sense CUOPT_MINIMIZE or CUOPT_MAXIMIZE, cuOptCreateSolverSettings, cuOptSolve(problem, settings, &solution), cuOptGetObjectiveValue, and the three cuOptDestroy* functions.",
"expected_behavior": [
"Lists C API call sequence without writing a complete source file",
- "Names cuOptCreateRangedProblem, cuOptSolve, cuOptGetObjectiveValue in order"
+ "Names the three struct types (cuOptOptimizationProblem, cuOptSolverSettings, cuOptSolution)",
+ "Names cuOptCreateRangedProblem, cuOptCreateSolverSettings, cuOptSolve, cuOptGetObjectiveValue, and destroy functions in order"
]🤖 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-numerical-optimization-api-c/evals/evals.json` around lines 4 -
11, Update the expected list to include the required struct types and missing
API calls in order: include cuopt/linear_programming/cuopt_c.h, then structs
cuOptOptimizationProblem, cuOptSolverSettings, cuOptSolution, then call
cuOptCreateRangedProblem, cuOptCreateSolverSettings, cuOptSolve(problem,
settings, &solution), cuOptGetObjectiveValue, and finally cleanup with
cuOptDestroyProblem, cuOptDestroySolverSettings, cuOptDestroySolution; reference
these exact symbols (cuOptCreateRangedProblem, cuOptCreateSolverSettings,
cuOptSolve, cuOptGetObjectiveValue, cuOptDestroyProblem,
cuOptDestroySolverSettings, cuOptDestroySolution and the structs) so the eval
expects both functions and structs in the proper sequence.
Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>
|
/ok to test 1279c54 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@skills/cuopt-numerical-optimization-api-c/skill-card.md`:
- Around line 60-61: The markdown linter rule MD058 is triggered because the
table starts immediately after the heading "## Evaluation Results:"; fix it by
inserting a single blank line between the heading line that contains "##
Evaluation Results:" and the table start row beginning with "| Dimension" so the
table is separated from the heading and passes markdownlint; after editing, run
pre-commit hooks (pre-commit run --all-files --show-diff-on-failure) to verify.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e71a500e-a2a6-48b6-a403-1067bc99cf57
📒 Files selected for processing (3)
skills/cuopt-numerical-optimization-api-c/BENCHMARK.mdskills/cuopt-numerical-optimization-api-c/skill-card.mdskills/cuopt-numerical-optimization-api-c/skill.oms.sig
✅ Files skipped from review due to trivial changes (1)
- skills/cuopt-numerical-optimization-api-c/BENCHMARK.md
| ## Evaluation Results: <br> | ||
| | Dimension | Num | `claude-code` | `codex` | |
There was a problem hiding this comment.
Add a blank line before the results table to satisfy markdownlint.
MD058 is triggered because the table starts immediately after the heading text. Insert a blank line between Line 60 and Line 61.
Proposed fix
## Evaluation Results: <br>
+
| Dimension | Num | `claude-code` | `codex` |
|---|---:|---:|---:|As per coding guidelines, "Use pre-commit run --all-files --show-diff-on-failure to check code formatting and linting on all files before committing".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Evaluation Results: <br> | |
| | Dimension | Num | `claude-code` | `codex` | | |
| ## Evaluation Results: <br> | |
| | Dimension | Num | `claude-code` | `codex` | |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 61-61: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 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-numerical-optimization-api-c/skill-card.md` around lines 60 -
61, The markdown linter rule MD058 is triggered because the table starts
immediately after the heading "## Evaluation Results:"; fix it by inserting a
single blank line between the heading line that contains "## Evaluation
Results:" and the table start row beginning with "| Dimension" so the table is
separated from the heading and passes markdownlint; after editing, run
pre-commit hooks (pre-commit run --all-files --show-diff-on-failure) to verify.
Add eval