Skip to content

fix: correct dual signs for >= constraints - #133

Closed
DuexDEVG wants to merge 2 commits into
rust-or:mainfrom
DuexDEVG:fix/dual-sign-for-greater-equal
Closed

DuexDEVG wants to merge 2 commits into
rust-or:mainfrom
DuexDEVG:fix/dual-sign-for-greater-equal

Conversation

@DuexDEVG

@DuexDEVG DuexDEVG commented Aug 2, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #82

Summary

Corrects the sign of dual values returned for constraints written with >= while preserving the solver-specific objective-direction conventions already used by the dual integrations.

Problem

good_lp normalizes every inequality sent to a solver as an upper-bound constraint. For a user constraint written as a >= b, the normalized expression is b - a <= 0, so the normalized right-hand side is the negation of the right-hand side written by the user. A dual returned for that normalized right-hand side therefore has the opposite sign from the corresponding user-facing shadow price.

The original constraint direction was discarded during normalization, and ConstraintReference retained only the row index. As a result, the HiGHS and Clarabel integrations returned the raw normalized value for the case reported in #82.

Implementation

  • Preserve the original constraint direction (<=, ==, or >=) while retaining the existing normalized solver representation.
  • Carry that direction through ConstraintReference without changing its existing index-based equality or debug representation.
  • Map normalized dual values back to the user-facing convention in the dual-capable integrations. The reported maximization >= case is negated; existing <= and equality behavior is preserved.
  • Keep Clarabel's objective direction in its solution so the correction is applied only on the maximization path, preserving its existing minimization behavior.
  • Update all solver model implementations to construct direction-aware references. Solvers without dual-value support retain their existing runtime behavior.

Regression coverage

Extends tests/shadow_price.rs with a solver-independent >= case:

maximise  -2x
subject to x >= 1

The optimum is x = 1, and increasing the lower bound by one decreases the maximized objective by two, so the expected dual value is -2. A minimization counterpart also verifies that the Clarabel and HiGHS objective-direction handling remains consistent. Both cases run for HiGHS and Clarabel alongside the existing <= dual tests.

Validation

  • cargo fmt -- --check
  • git diff --check
  • cargo clippy --all-targets --no-default-features --features "microlp,clarabel"
  • cargo test --no-default-features --features clarabel --test shadow_price
  • WSL2 Ubuntu: cargo test --no-default-features --features highs --test shadow_price
  • WSL2 Ubuntu: cargo build --features all_default_solvers --tests
  • WSL2 Ubuntu: cargo test --quiet --features all_default_solvers -- --test-threads=1

The local CPLEX-specific check could not be run because this environment does not have a CPLEX installation. The repository's CPLEX CI job passed for the original revision; the follow-up revision is being rechecked by CI.

@lovasoa

lovasoa commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

I'm not a huge fan of the solution here. It makes all solvers pay the complexity and runtime cost to benefit just one feature of two solvers. Could we discuss the feature design first ?

@lovasoa

lovasoa commented Aug 2, 2026 •

Copy link
Copy Markdown
Collaborator

reading #82 (comment) it sounds like the proper fix is to avoid early normalization on our side, so that HiGHS sees the constraint as pb.add_row(400.., [(x4, 1.)]) and can thus return the right result transparently without any hack on our side.

I think it should be possible to fix the bug with a much smaller diff than you have here

@lovasoa
lovasoa marked this pull request as draft August 2, 2026 19:02
@DuexDEVG DuexDEVG closed this Aug 2, 2026
@lovasoa

lovasoa commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

see #135 for my take on this

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.

Sign Error on Dual values for >= Constraints

2 participants