Skip to content

feat: add objective offset field c0 to MILP#45

Open
gdalle wants to merge 1 commit intomainfrom
gd/claude_expe
Open

feat: add objective offset field c0 to MILP#45
gdalle wants to merge 1 commit intomainfrom
gd/claude_expe

Conversation

@gdalle
Copy link
Copy Markdown
Member

@gdalle gdalle commented Mar 18, 2026

This is my first experiment with Claude Code

Closes #29

Summary

This PR adds a c0::T field to the MILP struct to store the constant term of the objective function, so that the full objective is cᵀx + c0 rather than just cᵀx.

Changes

Core

  • src/problems/milp.jl: Added c0::T field to MILP (after uc, before D1). The constructor accepts c0 = false as a default (promotes correctly to the float type T). Updated Base.isapprox to compare c0 values. The QPSData constructor now passes c0 = qps.c0.
  • src/problems/solution.jl: objective_value(x, milp) now returns dot(x, milp.c) + milp.c0.

Propagation

  • src/problems/modify.jl: c0 is threaded through set_eltype (with type conversion T(c0)), set_indtype, set_matrix_type, and Adapt.adapt_structure.
  • src/components/preconditioning.jl: c0 is passed unchanged through precondition(milp, prec) — the diagonal scalings D1, D2 do not affect a scalar constant.

MOI wrapper

  • src/MOI_wrapper.jl: obj_constant is now passed as c0 at MILP construction time. For maximization problems, obj_constant is negated alongside c so that the stored c0 always corresponds to the equivalent minimization. The dual objective value is updated to ±(raw_dual_obj + milp.c0).

Tests

  • test/problems/solution.jl: verifies objective_value includes the offset.
  • test/problems/milp.jl: verifies isapprox correctly distinguishes MILPs with different c0 values.
  • test/components/preconditioning.jl: verifies c0 is unchanged after preconditioning.

Notes

  • KKT errors (kkt_errors!) are intentionally not modified: the primal-dual gap is scale-invariant with respect to the objective constant (it cancels between primal and dual sides), so the convergence check is unaffected.
  • The c0 field does not interact with preconditioning: it is a scalar constant, not scaled by D1 or D2.

🤖 Generated with Claude Code

Store the constant term of the objective (cᵀx + c0) directly in the
`MILP` struct. `objective_value` now includes the offset, and `c0` is
threaded through preconditioning, type conversion, and GPU adaptation.
The MOI wrapper passes `obj_constant` as `c0` at construction time
instead of reapplying it manually after solving.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/MOI_wrapper.jl 97.05% <100.00%> (+0.02%) ⬆️
src/components/preconditioning.jl 81.94% <ø> (ø)
src/problems/milp.jl 83.87% <100.00%> (+0.53%) ⬆️
src/problems/modify.jl 69.56% <ø> (ø)
src/problems/solution.jl 78.37% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@klamike
Copy link
Copy Markdown
Collaborator

klamike commented Mar 18, 2026

I don't like giving them free advertising by letting it put itself as a "contributor"... I believe there is a setting somewhere to turn that off. Or you can just commit yourself (which I have come to prefer since it forces me to review each commit)

@klamike
Copy link
Copy Markdown
Collaborator

klamike commented Mar 18, 2026

KKT errors (kkt_errors!) are intentionally not modified: the primal-dual gap is scale-invariant with respect to the objective constant (it cancels between primal and dual sides), so the convergence check is unaffected.

it's worth thinking about this some more. it does change things since we are computing relative metrics not absolute

like min 1e6 * x - 1e6 s.t. x ≥ 2

@gdalle
Copy link
Copy Markdown
Member Author

gdalle commented Mar 18, 2026

It's my very first vibe-coding experiment, I haven't reviewed it in detail but of course I plan to, just wanted to start off with a really easy feature.

As for commit signature, I actually consider it a good practice. Not to wash my hands clean of the responsibility, but to be able to track AI use in my codebases, with commit statistics and history. This is necessary for some publications which require disclosure of LLM use, and possibly in the future for legal aspects.

See e.g. https://www.deployhq.com/blog/how-to-use-git-with-claude-code-understanding-the-co-authored-by-attribution for both sides of the debate. Note that once I move to a containerized setup where I give the agent free rein to run stuff in a safe environment, I will give it a secondary GitHub account like @gdalle-agent anyway to avoid misuse of my personal repo permissions.

@gdalle
Copy link
Copy Markdown
Member Author

gdalle commented Mar 18, 2026

As for scaling and sign issues:

  • I don't believe c0 should be a minimization-only setting, I believe the objective should always be c x + c0
  • I think you're right and we need to change the KKT denominator. Essentially, adding a constant term is like adding one more variable which is constrained to the value 1 (that's another way we could implement it btw)

@klamike
Copy link
Copy Markdown
Collaborator

klamike commented Mar 18, 2026

Regarding the attribution, just a personal preference thing. Fine to keep it if you prefer that.

Agreed on + c0 for min and max. I believe it is what MOI/JuMP has too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support objective offset

2 participants