Fix #114: Add Knapsack model by zazabap · Pull Request #171 · CodingThrust/problem-reductions

zazabap · 2026-03-04T19:27:59Z

Summary

Add the 0-1 Knapsack problem model (src/models/misc/knapsack.rs)
Binary configuration space: select/don't select each item
Maximize total value subject to weight capacity constraint
Complexity: O(2^(n/2)) via Horowitz-Sahni meet-in-the-middle (1974)
15 unit tests covering evaluation, edge cases (zero capacity, single item, greedy-not-optimal adversarial), brute force solver, serialization
CLI dispatch + alias (KS)
Paper problem-def entry with formal definition and background
Schema export updated

Test plan

All 15 unit tests pass (cargo test -- knapsack)
Doc-test passes
Clippy clean (no warnings)
Structural review: 16/16 checks pass
Issue compliance: 8/8 checks pass
Code quality review: no critical or important issues

Closes #114

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Re-export Knapsack from models/mod.rs (structural review finding) - Add tests: zero capacity, single item, greedy-not-optimal adversarial case Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR adds the 0-1 Knapsack problem as a new model in the misc/ category, implementing it as described in Issue #114. The Knapsack problem is a foundational NP-complete problem from Karp's 21 problems, where the goal is to select items maximizing total value subject to a weight capacity constraint.

Changes:

New Knapsack model struct with Problem and OptimizationProblem trait implementations, binary configuration space, inventory schema registration, and 15 comprehensive unit tests
CLI integration: dispatch for loading/serializing Knapsack instances and alias KS for the CLI
Documentation: paper problem definition with formal mathematical formulation, two new bibliography entries (Horowitz-Sahni 1974, Ibarra-Kim 1975), and updated schema JSON

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`src/models/misc/knapsack.rs`	Core Knapsack model implementation with `Problem` and `OptimizationProblem` traits
`src/models/misc/mod.rs`	Register and re-export `Knapsack` module
`src/models/mod.rs`	Add `Knapsack` to top-level `misc` re-exports
`src/unit_tests/models/misc/knapsack.rs`	15 unit tests covering evaluation, edge cases, brute force, serialization
`problemreductions-cli/src/dispatch.rs`	CLI dispatch for loading/serializing Knapsack
`problemreductions-cli/src/problem_name.rs`	Add `KS` alias and case-insensitive resolution
`docs/src/reductions/problem_schemas.json`	Add Knapsack schema entry
`docs/paper/reductions.typ`	Add display name and formal problem definition
`docs/paper/references.bib`	Add Horowitz-Sahni 1974 and Ibarra-Kim 1975 references
`docs/plans/2026-03-04-knapsack-model.md`	Implementation plan document

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

docs/paper/reductions.typ

src/models/mod.rs

codecov · 2026-03-06T10:39:55Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.90%. Comparing base (5fbde0e) to head (1130360).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #171      +/-   ##
==========================================
+ Coverage   96.88%   96.90%   +0.01%     
==========================================
  Files         200      202       +2     
  Lines       27537    27679     +142     
==========================================
+ Hits        26680    26823     +143     
+ Misses        857      856       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- Paper definition: use w_0,...,w_(n-1) to match 0-based subset S ⊆ {0,...,n-1} - Add Knapsack to prelude re-export for consistency with other misc models Resolves Copilot review comments on PR CodingThrust#171. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…ing aliases) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

GiggleLiu

Should mention if the set index is 0-based or 1-based.

GiggleLiu · 2026-03-07T15:31:32Z

docs/plans/2026-03-04-knapsack-model.md

Remove this plan file.

GiggleLiu · 2026-03-07T15:32:45Z

problemreductions-cli/src/problem_name.rs

    ("TSP", "TravelingSalesman"),
    ("BP", "BinPacking"),
    ("CVP", "ClosestVectorProblem"),
+    ("KS", "Knapsack"),


Is KS a good shorthand? I doubt it. Also, BP is also bad. Consider removing these two.

Removed both KS and BP aliases (from ALIASES constant and resolve_alias match arms). Case-insensitive full names (knapsack, binpacking) still work.

- Delete docs/plans/2026-03-04-knapsack-model.md - Remove KS and BP short aliases; case-insensitive full names still work Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

zazabap

Addressed: the paper definition now uses consistent 0-based indexing (w_0, ..., w_(n-1), v_0, ..., v_(n-1)) matching the subset S ⊆ {0, ..., n-1} and the code's 0-indexed configs. Fixed in commit 48239c5.

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-08T08:26:10Z

problemreductions-cli/src/problem_name.rs

        "bmf" => "BMF".to_string(),
        "bicliquecover" => "BicliqueCover".to_string(),
-        "bp" | "binpacking" => "BinPacking".to_string(),
+        "binpacking" => "BinPacking".to_string(),


The BP alias for BinPacking is removed from both the ALIASES const and the resolve_alias() function. This is an unrelated breaking change: any existing CLI user who used pred create BP ... or pred evaluate BP ... will now get an "Unknown problem" error. If this removal was intentional, it should be mentioned in the PR description; otherwise, it should be reverted.

Copilot · 2026-03-08T08:26:10Z

problemreductions-cli/src/problem_name.rs

-        "bp" | "binpacking" => "BinPacking".to_string(),
+        "binpacking" => "BinPacking".to_string(),
        "cvp" | "closestvectorproblem" => "ClosestVectorProblem".to_string(),
+        "knapsack" => "Knapsack".to_string(),


The PR description mentions "CLI dispatch + alias (KS)" but no KS alias was actually added. The ALIASES const does not include ("KS", "Knapsack") and resolve_alias() does not handle "ks". If you intended to add this alias (matching the pattern of TSP, MIS, CVP, etc.), it should be added to both ALIASES and resolve_alias().

Copilot · 2026-03-08T08:26:11Z

src/models/misc/knapsack.rs

+crate::declare_variants! {
+    Knapsack => "2^(num_items / 2)",
+}


The reduction_graph.json is auto-generated (via cargo run --example export_graph or pred export-graph) but has not been regenerated to include the new Knapsack node. Other misc models like Factoring and PaintShop appear as nodes in this file even without reduction edges. Since the declare_variants! macro registers Knapsack in the variant registry, re-running the export should add it. Without this update, the paper's completeness check (which compares display-name keys against reduction_graph.json nodes) may produce a warning for the missing Knapsack entry.

…ude Knapsack The Knapsack node was missing from the exported graph and schemas, which would cause the paper's completeness check to fail. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

zazabap · 2026-03-08T08:36:03Z

Addressing Copilot review comments

1. Indexing inconsistency in paper definition (reductions.typ:895)
Fixed in 48239c5 — switched to consistent 0-based indexing (w_0, ..., w_(n-1)) matching the code implementation.

2. Knapsack missing from prelude (mod.rs:18)
Fixed in 48239c5 — Knapsack is now included in the prelude.

3. BP alias removal is breaking change (problem_name.rs:52)
Intentional — reviewer requested removal of both KS and BP aliases as they are not standard abbreviations.

4. KS alias not added (problem_name.rs:54)
Intentional — KS alias was initially added but removed per reviewer feedback. The alias was dropped during review.

5. reduction_graph.json not regenerated (knapsack.rs:139)
Fixed in 1130360 — regenerated both reduction_graph.json and problem_schemas.json to include the Knapsack node.

zazabap · 2026-03-09T11:42:32Z

Reduction rule is here #116 and implemented #172 @GiggleLiu

zazabap and others added 6 commits March 4, 2026 19:06

Add plan for CodingThrust#114: Knapsack

3a74983

feat: add Knapsack model with unit tests

0a8d61c

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

feat: register Knapsack in CLI dispatch and aliases

8da012e

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

docs: add Knapsack problem definition to paper

d843c38

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

chore: regenerate schemas after Knapsack addition

c189700

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

fix: add Knapsack re-export and edge-case tests

3d0898c

- Re-export Knapsack from models/mod.rs (structural review finding) - Add tests: zero capacity, single item, greedy-not-optimal adversarial case Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

GiggleLiu requested a review from Copilot March 6, 2026 10:23

Copilot started reviewing on behalf of GiggleLiu March 6, 2026 10:24 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

docs/paper/reductions.typ Outdated Show resolved Hide resolved

src/models/mod.rs Show resolved Hide resolved

zazabap and others added 2 commits March 7, 2026 15:17

merge: resolve conflict in problem_name.rs (keep both KS and MaxMatch…

692337e

…ing aliases) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

GiggleLiu reviewed Mar 7, 2026

View reviewed changes

fix: remove plan file and KS/BP aliases per review

ff649bc

- Delete docs/plans/2026-03-04-knapsack-model.md - Remove KS and BP short aliases; case-insensitive full names still work Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

zazabap commented Mar 8, 2026

View reviewed changes

zazabap requested a review from Copilot March 8, 2026 08:18

Copilot started reviewing on behalf of zazabap March 8, 2026 08:19 View session

Copilot AI reviewed Mar 8, 2026

View reviewed changes

fix: regenerate reduction_graph.json and problem_schemas.json to incl…

1130360

…ude Knapsack The Knapsack node was missing from the exported graph and schemas, which would cause the paper's completeness check to fail. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

zazabap requested a review from GiggleLiu March 9, 2026 07:10

zazabap mentioned this pull request Mar 9, 2026

Skills update for the implementation workflow. #191

Closed

zazabap self-assigned this Mar 9, 2026

isPANN mentioned this pull request Mar 10, 2026

[Model] Knapsack #214

Closed

3 tasks

Conversation

zazabap commented Mar 4, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

GiggleLiu left a comment

Choose a reason for hiding this comment

Uh oh!

GiggleLiu Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

zazabap Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

GiggleLiu Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

zazabap Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

zazabap left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

zazabap commented Mar 8, 2026

Addressing Copilot review comments

Uh oh!

zazabap commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Mar 6, 2026 •

edited

Loading

zazabap commented Mar 9, 2026 •

edited

Loading