Fix #534: Add PartiallyOrderedKnapsack model#631
Merged
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- New optimization problem model in src/models/misc/ - Precedence-constrained knapsack with downward-closure check - Full CLI support (dispatch, alias, create) - 17 unit tests covering evaluation, solver, serialization - Paper documentation with problem definition and references Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
Implementation SummaryChanges
Deviations from Plan
Open Questions
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #631 +/- ##
========================================
Coverage 97.34% 97.34%
========================================
Files 329 331 +2
Lines 42297 42556 +259
========================================
+ Hits 41175 41428 +253
- Misses 1122 1128 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds a new optimization model, PartiallyOrderedKnapsack, extending the library and CLI to support knapsack instances with precedence (downward-closure) constraints, and updates the paper docs accordingly.
Changes:
- Introduces
PartiallyOrderedKnapsackmodel with schema registration, evaluation logic, and variant declaration. - Adds unit tests covering evaluation edge-cases, brute-force optimality, and serde roundtrip.
- Integrates the model into the CLI (alias/dispatch/create flags) and paper documentation (+references).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/models/misc/partially_ordered_knapsack.rs |
New model implementation + schema registration + variants + test module hook. |
src/unit_tests/models/misc/partially_ordered_knapsack.rs |
Unit tests for correctness, solver integration, and serialization. |
src/models/misc/mod.rs |
Registers and re-exports the new misc model. |
src/models/mod.rs |
Re-exports the new model at the top-level models module. |
problemreductions-cli/src/problem_name.rs |
Adds alias resolution for the new problem name. |
problemreductions-cli/src/dispatch.rs |
Enables load/serialize dispatch for the new model. |
problemreductions-cli/src/commands/create.rs |
Adds pred create PartiallyOrderedKnapsack ... support and parsing. |
problemreductions-cli/src/cli.rs |
Adds new CLI flags (--values, --item-precedences) and help text line. |
docs/paper/references.bib |
Adds bibliography entries used by the new paper section. |
docs/paper/reductions.typ |
Adds problem definition section and display-name mapping for the paper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+131
to
+160
| /// Uses the transitive closure of the precedence relation: if item `b` is | ||
| /// selected and `a` is a (transitive) predecessor of `b`, then `a` must | ||
| /// also be selected. | ||
| fn is_downward_closed(&self, config: &[usize]) -> bool { | ||
| let n = self.num_items(); | ||
| // Build adjacency matrix for transitive closure | ||
| let mut reachable = vec![vec![false; n]; n]; | ||
| for &(a, b) in &self.precedences { | ||
| reachable[a][b] = true; | ||
| } | ||
| // Floyd-Warshall for transitive closure | ||
| for k in 0..n { | ||
| for i in 0..n { | ||
| for j in 0..n { | ||
| if reachable[i][k] && reachable[k][j] { | ||
| reachable[i][j] = true; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| // Check: for every selected item b, all predecessors must be selected | ||
| for b in 0..n { | ||
| if config[b] == 1 { | ||
| for a in 0..n { | ||
| if reachable[a][b] && config[a] != 1 { | ||
| return false; | ||
| } | ||
| } | ||
| } | ||
| } |
Comment on lines
+12
to
+24
| inventory::submit! { | ||
| ProblemSchemaEntry { | ||
| name: "PartiallyOrderedKnapsack", | ||
| module_path: module_path!(), | ||
| description: "Select items to maximize total value subject to precedence constraints and weight capacity", | ||
| fields: &[ | ||
| FieldInfo { name: "sizes", type_name: "Vec<i64>", description: "Item sizes s(u) for each item" }, | ||
| FieldInfo { name: "values", type_name: "Vec<i64>", description: "Item values v(u) for each item" }, | ||
| FieldInfo { name: "precedences", type_name: "Vec<(usize, usize)>", description: "Precedence pairs (a, b) meaning a must be included before b" }, | ||
| FieldInfo { name: "capacity", type_name: "i64", description: "Knapsack capacity B" }, | ||
| ], | ||
| } | ||
| } |
| #[arg(long)] | ||
| pub values: Option<String>, | ||
| /// Precedence pairs (e.g., "0>2,0>3,1>4") for PartiallyOrderedKnapsack | ||
| #[arg(long)] |
Comment on lines
+518
to
+519
| "PartiallyOrderedKnapsack requires --sizes, --values, --capacity, and --item-precedences\n\n\ | ||
| Usage: pred create PartiallyOrderedKnapsack --sizes 2,3,4,1,2,3 --values 3,2,5,4,3,8 --item-precedences \"0>2,0>3,1>4,3>5,4>5\" --capacity 11" |
| #problem-def("PartiallyOrderedKnapsack")[ | ||
| Given a finite set $U$ with $|U| = n$ items, a partial order $<$ on $U$ (given by its cover relations), for each $u in U$ a size $s(u) in ZZ^+$ and a value $v(u) in ZZ^+$, and a capacity $B in ZZ^+$, find a downward-closed subset $U' subset.eq U$ (i.e., if $u in U'$ and $u' < u$ then $u' in U'$) maximizing $sum_(u in U') v(u)$ subject to $sum_(u in U') s(u) lt.eq B$. | ||
| ][ | ||
| Garey and Johnson's problem A6 MP12 @garey1979. Unlike standard Knapsack, the partial order constraint makes the problem _strongly_ NP-complete --- it remains NP-complete even when $s(u) = v(u)$ for all $u$, so no pseudo-polynomial algorithm exists unless $P = "NP"$. The problem arises in manufacturing scheduling, project selection, and mining operations. For tree partial orders, Johnson and Niemi @johnson1983 gave pseudo-polynomial $O(n dot B)$ tree DP and an FPTAS. Kolliopoulos and Steiner @kolliopoulos2007 extended the FPTAS to 2-dimensional partial orders with $O(n^4 slash epsilon)$ running time. |
…rdered-knapsack # Conflicts: # docs/paper/reductions.typ # docs/paper/references.bib # problemreductions-cli/src/cli.rs # problemreductions-cli/src/commands/create.rs # problemreductions-cli/src/dispatch.rs # problemreductions-cli/src/problem_name.rs # src/models/misc/mod.rs # src/models/mod.rs
- Precompute transitive predecessors in constructor instead of recomputing Floyd-Warshall on every evaluate() call - Custom Serialize/Deserialize to rebuild predecessors on deserialization - Rename CLI flag --item-precedences to --precedences (with alias for compat) - Fix error message to reflect --precedences is optional - Fix Typst $P = "NP"$ to $P = N P$ for correct math typography - Add missing schema fields (display_name, aliases, dimensions) - Add opt keyword to declare_variants! - Regenerate problem_schemas.json and reduction_graph.json Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add canonical model example builder for PartiallyOrderedKnapsack - Add PartiallyOrderedKnapsack to trait_consistency tests - Add nonnegative validation for sizes and capacity - Add cycle detection in precedence graph - Fix corrupted reduction_graph.json (was mixing stdout/JSON) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
Review Pipeline Report
🤖 Generated by review-pipeline |
…rdered-knapsack # Conflicts: # docs/src/reductions/problem_schemas.json # docs/src/reductions/reduction_graph.json # problemreductions-cli/src/cli.rs # problemreductions-cli/src/commands/create.rs # src/models/graph/maximum_independent_set.rs # src/models/misc/mod.rs # src/unit_tests/trait_consistency.rs
…apsack - Migrate ModelExampleSpec from old `build` closure to new `instance`/`optimal_config`/`optimal_value` struct format - Align paper notation with standard Knapsack: w_i/v_i, capacity C, subset S, partial order ≺ Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Merge two duplicate `misc` import blocks into one - Add missing `values` and `precedences` fields to test `empty_args()` Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rdered-knapsack # Conflicts: # docs/paper/reductions.typ # problemreductions-cli/src/commands/create.rs
- Rename `sizes` field to `weights` throughout (code, schema, docs, CLI, tests) to match standard Knapsack terminology - Paper uses w_i notation consistent with Knapsack section - Add non-negative validation for values (matching Knapsack pattern) - Add #[should_panic] tests for cycle detection, negative capacity, negative weight, and negative value - Fix doc comment leak on PartiallyOrderedKnapsackRaw Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
isPANN
approved these changes
Mar 19, 2026
Collaborator
|
Community call checklist (PR #631):
|
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add the PartiallyOrderedKnapsack problem model — a knapsack variant where items are subject to a partial order (precedence constraints). Including an item requires including all its predecessors (downward-closed set). Modeled as an optimization problem (maximize total value subject to precedence + capacity).
Fixes #534