Benchmark knapsack penalties vs projection constraints

[bernalde]

Summary

• add an isolated knapsack benchmark environment that is not loaded by package
  tests or documentation
• compare bounded-slack penalty QUBOs with native SumConstraint projection
  MPOs on identical, exactly checked Martello-Pisinger-Toth instances
• define both formulations through one KnapsackMethod abstraction and execute
  them as a BenchmarkTools.BenchmarkGroup
• use TenSolver's public solve, sampling, and SolverStatistics APIs without
  reconstructing internal tensor-network objects
• keep the script as a five-line default entry point and expose custom workloads
  through ordinary run_benchmarks keyword arguments in the REPL
• seed the shared instance RNG once, write run-wide metadata once in the report
  header, and keep only case-specific measurements in result rows
• follow up the Preserve solver statistics compatibility #119 review by removing transitional top-level stats aliases,
  validating the variance-check cadence, and consolidating redundant solver tests

Generated benchmark results are deliberately not committed; the suite remains
an opt-in tool for producing results in the current environment.

Validation

• julia +1.12 --project=benchmarks --startup-file=no -e 'using Pkg; Pkg.instantiate()'
• one-instance projection/penalty pilot with one sweep, two reads, and one timing
  sample; both cases completed and the projection case reached the exact feasible
  optimum
• public-statistics resource sweep over all 11 probes; projection MPO bonds were
  (3, 4, 6, 10) for capacities (1, 2, 4, 8), and remained 5 when varying
  item count or weight magnitude
• julia +1.12 --project=. --startup-file=no -e 'using Pkg; Pkg.test(; coverage=false)'
• julia +1.12 --project=docs --startup-file=no docs/make.jl local
• git diff --check

Design and attribution

The inequality penalty uses bounded-binary slack so the squared residual
encodes weight <= capacity; penalizing (weight - capacity)^2 alone would
incorrectly turn the problem into an equality constraint. The benchmark is
motivated by CoTenN's direct-constraint framing but is not presented as a
reproduction of its published physics experiments.

Each timing sample covers formulation, solve, and sampling. Results are scored
against the original knapsack objective and exact feasible optimum. The
controlled resource table reads objective, projection, and effective-Hamiltonian
bond dimensions from the same public statistics API used by the main benchmark.

Coordination

Closes #66
Refs #55

Prerequisites #58, #62, #63, #64, #65, #118, and #119 are merged. The benchmark
is isolated under benchmarks/ and does not add package-test or documentation
coupling.

Risks

• finite DMRG runs are not guaranteed to reach the encoded global optimum
• exhaustive reference solutions intentionally limit the default workload to
  at most 16 items
• timing results remain machine- and environment-specific
• heavy solver runs remain opt-in and outside regular CI

[iagoleal]

We should make the benchmarks independent from the rest of the codebase. Ideally, the benchmarks depend on TenSolver as if it was an external package while the package itself does not know of the benchmarks. For this:

• There should be a benchmarks/Project.toml
• Do not mix how-to documentation and tests with benchmarks.
• The package tests should not track the benchmarks nor depend on them.

On another note, it makes sense to use "standard" tools and instances for benchmarking. In particular,

• Julia has BenchmarkTools.jl and PkgBenchmark.jl as standard packages for performing benchmarks.)
• Random instances are feeble. In particular, we should never do hand manipulation and passing of random seeds. This is prone to bugs. Just set the seed on the start of each item and then let the standard library do its thin.
• Random weights are ok for initial runs but I'd prefer to have more robust Knapsack problems. Can you find some standard instances in the literature?

[bernalde]

Maintainer review — knapsack penalty-vs-projection benchmark (Closes #66)

This lands the #66 benchmark: a like-for-like comparison of the two ways to enforce the knapsack capacity constraint — a penalized QUBO vs. a hard projection MPO — on the original objective. I authored it, so this posts as COMMENT (GitHub blocks author APPROVE). No blocking findings.

Meets #66's acceptance criteria

• Both formulations, apples-to-apples. penalty_qubo builds the bounded-binary-slack QUBO (slack_weights covers 0:capacity) and projection_row/penalty_row solve each with DMRG, then score both on the same original objective via item_value — so the comparison is on the real knapsack value, not each method's internal penalized objective.
• Exact reference. brute_force_optimum gives the ground-truth optimum; the reference instance's is value 13.0, items [1,0,1,0], weight 6.
• Required metrics, including feasibility. benchmark_result_row reports original_value, feasible, optimality_gap (gated to missing when infeasible so an infeasible high value can't masquerade as a small gap), plus the resource bonds (objective_mpo_bond, projection_mpo_bond, effective_hamiltonian_bond) and timing.
• Penalty sensitivity. The penalty family sweeps the factor; the CI test's exact_penalty_minimizer checks the sensitivity exhaustively (no DMRG).
• Docs + CoTenN framing. README, docs/src/constraints.md, and benchmarks/knapsack/README.md link the benchmark and frame projection as the CoTenN-style hard-constraint alternative.

Verification (ran locally)

• Bond story confirmed. julia --project=. benchmarks/knapsack/run.jl --resources produces the scaling CSV: projection_mpo_bond = capacity + 2, constant across item counts (8/16/32 → bond 5 at capacity 3) and across weight magnitudes (weights 4…128 → same bond). The projection MPO's bond tracks the capacity register, not the problem size — the headline claim holds.
• Both methods reach the feasible optimum. A heavy slice on the reference instance (3 sweeps, 8 reads): projection → original_value=13.0, feasible=true, gap=0.0, projbond=8 (= capacity 6 + 2); penalty at factor 1.1 → original_value=13.0, feasible=true, gap=0.0. The full penalty-vs-projection DMRG path executes end-to-end and both land on the exact optimum.
• CI stays light. The in-CI test/knapsack_benchmark.jl uses brute force and MPO-bond assertions only — grep confirms the sole minimize/DMRG call sits in the run.jl heavy path (KnapsackBenchmark.jl:383), never in the test. CI is 11/11 green on the reviewed head.

Findings

None blocking. One Nonblocking note on test-suite placement — inline.

Merge-readiness

Complete and verified against #66's criteria; heavy runs are opt-in (run.jl with no args) and stay out of CI, consistent with the #102 CI-slimming goal. This account can't post a formal APPROVE on its own PR; main is not branch-protected, so no approval gate is reported. Closes #66 is the correct link (this fully delivers the benchmark the issue asks for).

[bernalde]

Addressed the review feedback and refreshed the branch on current main in 3bc727d.

Commits and changes

• The review-specific fixes remain in e48839a: the benchmark is isolated behind benchmarks/Project.toml; package tests, package docs, the root README, and src/ do not depend on it; timing uses BenchmarkTools; instances use the standard Martello–Pisinger–Toth families; RNG seeds are reset directly; on_iteration records final variance; and the module exports only the three runner-facing names.
• Pushed 3bc727d, an append-only merge of the four newer main commits. It was conflict-free, and the PR diff remains exactly the six standalone benchmark files.

Verification on 3bc727d

• julia +1.12 --project=. --startup-file=no -e 'using Pkg; Pkg.test(; coverage=false)' — passed.
• julia +1.12 --project=docs --startup-file=no docs/make.jl — passed.
• julia +1.12 --project=benchmarks --startup-file=no benchmarks/knapsack/run.jl --resources — passed; all 11 resource rows retained the documented capacity/item-count/weight-magnitude behavior.
• Bounded five-row solver pilot (--pilot --iterations=1 --reads=8 --timing-samples=1 --time-limit=30) — completed without a timeout; the projection row reached the exact feasible optimum.
• Current-head GitHub checks — documentation build, all nine Julia OS/version jobs, and documenter/deploy passed.
• git diff --check origin/main...HEAD — clean.

No review comment was intentionally declined. Truncation error remains empty by design because the public callback receives the retained MPS after discarded singular values are unavailable; the benchmark records final variance and documents that limitation.

All seven inline threads are resolved. The formal review decision remains CHANGES_REQUESTED until @iagoleal updates or dismisses the earlier review; a re-review is already requested. main is not branch-protected, so there is no enforced approving-review gate.

[bernalde]

Maintainer re-review — e48839a

Verdict: merge-ready, with no blocking findings. I authored this PR, so GitHub only permits this review to be recorded as COMMENT; this is the author-account equivalent of an approval.

Truncation-error decision

The limitation is acceptable for #66. TenSolver invokes on_iteration after each DMRG sweep has returned and supplies the retained MPS plus iteration/objective/bond/time metadata. The discarded singular values—and therefore the sweep truncation error—are no longer available and cannot be reconstructed from that retained state. Exposing them would require expanding the core solver callback/API, which is outside this benchmark PR's scope.

e48839a handles that boundary correctly: it records final-state variance from the callback MPS, leaves truncation_error empty, and documents why. I consider the corresponding review thread addressed.

Prior review resolution

The current head also isolates the benchmark under its own benchmarks/Project.toml, removes package-test/root-doc coupling, uses the standard Martello–Pisinger–Toth knapsack families with direct seed resets, measures with BenchmarkTools, and exports only the runner-facing API. All seven existing inline threads are addressed or superseded.

The absence of benchmark-specific package tests is acceptable here: #66 and #63 explicitly assign CI-suitable correctness regressions to #63 while keeping this benchmark independent and opt-in.

Verification

• Documented local-development environment setup succeeded with the current checkout.
• Focused reviewer assertions covering the exact reference, generator determinism, QUBO/projection Hamiltonians, exports, variance schema, and resource bounds: 151/151 passed on Julia 1.12.
• julia +1.12 --project=benchmarks benchmarks/knapsack/run.jl --resources: passed; projection bond dimensions follow the documented capacity-state bound and stay constant in the item-count and weight-magnitude sweeps.
• git diff --check origin/main...e48839a: clean.
• Current-head package and documentation CI: all 11 reported checks passed.

No blocking, nonblocking, or question findings remain. The reviewed head is merge-ready.

[bernalde]

Coordination update from closed #110:

Better to keep #112 as the canonical benchmarking infrastructure.
I'll start another draft after that one is merged.
We will only really need these files to address #1, not really for
benchmarking right now.

Source: PR 110 closing comment

Carrying that decision here makes the intended order explicit:

1. Treat Benchmark knapsack penalties vs projection constraints #112 as the canonical standalone benchmark infrastructure and merge
   it first.
2. Defer the VRP data/harness work to a separate draft after Benchmark knapsack penalties vs projection constraints #112 merges,
   scoped to issue LAPACKException on some QUBO examples #1 rather than the present knapsack benchmark.

[iagoleal]

I've left some high-level review notes. The code is too large, so I still did not have the time to go for all details. Let's address these points and then I will go again over the whole of the changes.

[iagoleal]

Why are we writing an argument parser from scratch. Let's keep it simple.
I'd prefer to either use ArgParse.jl or, even better, to have no command-line arguments at all. The script has only the simplest parameters and, if we want to use different parameters, we should be able to just call the runner function from the REPL with the desired parameters. Much simpler and easier to maintain.

[bernalde]

Addressed in ba249d5: the handmade parser is gone, run.jl is now five lines, and custom workloads use ordinary run_benchmarks keywords from the REPL.

[iagoleal]

With all iterations, this became too gargantuan for a simple benchmarking script. For the sake of our future selves, it must be rewritten in a leaner way before merging.
Here go some guidelines:

• Divide the file into 3 sections (using comment headers): types and infrastructure, instance definition and building helpers, runners.
• Instead of writing ad-hoc runners, use BenchmarkTools.BenchmarkGroup to provide a declarative benchmark suite.
• Do not repeat yourself. We have two almost equal parallel code paths for Projections vs Penalties. Write an abstraction for benchmark a knapsack and then just instantiate for each case. We should not have parallel functions for each methodology. Also keep it in a way that we can extend with other backends or methodologies in the future without hassle.
• Use the exposed bond dimension and data from PR Expose operator bond dims in solutions #118 instead of recalculating the operators. Or even better: only use TenSolver's public API and do not reach to internal methods. If we need anything internal, leave a comment in this thread and we can discuss how to implement an appropriate way to expose it.
• Variance and truncation error are irrelevant for this kind of benchmark. Stop talking about not calculating them. Variance is extremely expensive to calculate (almost the same as one extra DMRG sweep) and is only really important if we are not using an iteration limit. If you consider the variance an important data point, leave a comment in this thread and we can discuss how to reformulate the benchmarks based on the variance instead of iterations.
• Only set an RNG seed at the top of the file. Do not write per-instance seeds manually --- it is extremely error-prone. Even better, define a single RNG object and pass it directly instead of relying on seeds.
• There is a lot of NameTuple wrapping and unwrapping. If (and only if) it makes the code shorter and more readable, load DataFrames as a dependency and use it instead.

[bernalde]

Reworked in ba249d5. The module is split into the three requested sections, builds a declarative BenchmarkGroup, and routes projection and penalty cases through one extensible KnapsackMethod/runner path. It now uses only public TenSolver solve, sample, constraint, and statistics APIs; uses one shared instance RNG; and removes variance/truncation reporting and internal operator reconstruction. The module shrank from 706 to 497 lines, the runner from 133 to 5, and this review commit removes 482 net lines overall.

Focused pilots, all 11 resource probes, the full package suite, and the local docs build pass.

[bernalde]

Addressed the two post-merge review comments from #119 in #112 with commit b9de6e3.

Changes

• Removed the transitional top-level statistics aliases and their documentation/tests; solution.stats is now the canonical interface for the planned breaking release.
• Replaced trivial bond-dimension positivity assertions with semantic statistics checks, retained the infeasible/debug-logging regression, and removed duplicate solver calls.
• Added a typed, positive check_variance_every_iteration contract and documented it.
• Folded mixed checked/unchecked variance coverage into the existing statistics solve.

Verification

• Before the implementation, the new validation test reproduced both defects: zero raised a raw DivideError, while a negative cadence was accepted. Both now raise the documented ArgumentError.
• Targeted QUBO tests: 107/107 passed.
• Targeted constrained-solver tests: 126/126 passed.
• julia +1.12 --project=. --startup-file=no -e 'using Pkg; Pkg.test(; coverage=false)' — passed.
• julia +1.12 --project=docs --startup-file=no docs/make.jl — passed.
• The projection-resource table and the exact callback-free five-row PR pilot both completed successfully.
• Current-head documentation/deploy and all nine Julia OS/version jobs passed.
• git diff --check origin/main...HEAD — clean.

No #119 review comment was declined. This batch does not address the 12 separate unresolved benchmark-review threads on #112; its formal review decision remains CHANGES_REQUESTED.

[bernalde]

Addressed Iago's July 29 review batch in ba249d5.

• removed the committed CSV, results README, and stale recorded-results claims
• replaced parallel projection/penalty runners with one extensible KnapsackMethod path in a declarative BenchmarkTools.BenchmarkGroup
• switched resource accounting to TenSolver's public SolverStatistics API and removed internal operator reconstruction
• reduced run.jl from 133 lines to 5, with custom workloads called directly from the REPL
• moved run-wide metadata into a one-time report header and switched table serialization to DelimitedFiles.writedlm
• removed variance/truncation reporting and per-instance seed literals
• reorganized the module into the three requested sections; the review commit removes 482 net lines

Validation passed:

• exact benchmark-environment instantiate command
• deterministic instance-generation check
• one-instance projection/penalty pilot
• all 11 public-statistics resource probes
• full package test suite
• local documentation build
• all 11 current-head GitHub checks

All 12 inline comments in this review batch were implemented and replied to; none were declined. I left the threads unresolved for reviewer verification. The formal review state remains CHANGES_REQUESTED pending Iago's rereview.