[Issue 1224] Introduce one-sided constraints to the LP interface. by gutodelazeri · Pull Request #299 · aibasel/downward

gutodelazeri · 2026-07-27T08:39:18Z

This PR changes the LP interface from two-sided constraints

lb <= a^Tx <= ub

to one-sided constraints and equalities:

a^T x <= rhs
a^T x >= rhs
a^T x  = rhs

LPConstraint now stores a Sense (LE, GE, or EQ) and one right-hand side.

This simplifies LP backends. It is also preparatory work for adding further LP solvers in issue 1199 (https://issues.fast-downward.org/issue1199).

Main changes

Replace LPConstraint's lower and upper bounds with a constraint sense and right-hand side.
Replace set_constraint_lower_bound() and set_constraint_upper_bound() with set_constraint_rhs() and set_constraint_sense().
Update the CPLEX backend:
- map constraint senses directly to CPLEX senses;
- remove ranged-row data and conversion logic;
- remove bookkeeping that was only needed for inconsistent two-sided constraints.
Update the SoPlex backend:
- convert each sense/RHS pair to SoPlex's two-sided representation;
- remember constraint senses so dynamically changing the RHS updates the correct side;
Update all existing LP constraint generators to the new interface.

Important notes

The old interface contained two places where two-sided constraints were created, but one side was redundant:

In landmark cost partitioning, the lower bound was implied by nonnegative landmark-cost variables.
In the Rankooh-Rintanen formulation, the upper bound of constraint family (3) was implied by the bounds on f_q and f_{p,a}. In the paper, the authors do not mention an upper bound.

The new interface does not add these redundant constraints. The generated LP models, however, are equivalent, i.e., their feasible region and objective function remain unchanged.

Validation

We compared the new interface with baseline Fast Downward on the full optimal benchmark suite using CPLEX and SoPlex across 11 solver/configuration combinations.

Overall:

coverage increased or remained the same for most configurations;
memory usage generally decreased;
every task solved by both revisions had identical expansions, evaluations, plan cost, and plan length.

We separately tested restoring the redundant Rankooh-Rintanen upper bound as an additional one-sided constraint. This did not recover the behavior of baseline Fast Downward and slightly reduced coverage, so the redundant constraint is not included in this PR.

Experiment details and discussion:
https://issues.fast-downward.org/issue1224

SimonDold · 2026-07-28T14:47:55Z

+enum class Sense {
+    GE, // ax >= b
+    LE, // ax <= b
+    EQ // ax = b


I think those should be spelled out. GREATER_EQUAL, LESS_EQUAL, EQUAL.

SimonDold · 2026-07-28T15:17:34Z

+    void set_right_hand_side(double rhs) {
+        right_hand_side = rhs;
    }
-    void set_upper_bound(double ub) {


I think the interface with set_upper_bound and set_lower_bound is beneficial, because it allows me to not think about the way you write down your constraints.
Is it x+y>=1 or 1<=x+y, they are the same semantics.

if i have the expression x+y I would enjoy to just state a semantic wish "please restrict this with an upper bound of 1", and you take care of writing it down.

I don't think it is very useful to modify the sense or the rhs on its own. When would you ever do this?

We discussed this in person and decided to keep the functions as they are. The main reasons are

We do not have an expression class, so we cannot represent x+y by itself as in the example above. Without a class representing expressions, and only a class representing a one-sided constraint, the functions imply a functionality that we do not support.

with two set-bound functions, it is possible to create a trivially unsolvable constraint 1 <= EXPRESSION <= 0 which cannot be represented in CPLEX. This pull request gets rid of the complexity that was necessary to deal with this situation and having both set-bounds functions would re-introduce it.

FlorianPommerening

Looks good to me. I left comments but mostly they are about superficial things (renaming variables, etc.) that do not require re-running experiments. The only comment that could affect performance is about removing a set_sense in the delete-relaxation constraints.

FlorianPommerening · 2026-07-30T09:58:01Z

    }
 }

+static char constraint_sense_to_cplex_sense(lp::Sense constraint_sense) {


Do we need lp:: here? We should be inside the scope of the namespace already.

Also, to be consistent with LPObjectiveSense, maybe we should rename Sense to LPConstraintSense or ConstraintSense and the function model_sense_to_cplex_sense to objective_sense_to_cplex_sense.

FlorianPommerening · 2026-07-30T10:05:04Z

+        cerr << "Unsupported constraint sense: "
+             << static_cast<int>(constraint_sense) << endl;
+        utils::exit_with(utils::ExitCode::SEARCH_CRITICAL_ERROR);


Suggested change

cerr << "Unsupported constraint sense: "

<< static_cast<int>(constraint_sense) << endl;

utils::exit_with(utils::ExitCode::SEARCH_CRITICAL_ERROR);

ABORT("Unsupported constraint sense: " +

static_cast<int>(constraint_sense));

We usually use ABORT for "this should not be possible", i.e., errors targeted to developers rather than users.

FlorianPommerening · 2026-07-30T10:16:19Z

+    void set_right_hand_side(double rhs) {
+        right_hand_side = rhs;
    }
-    void set_upper_bound(double ub) {


We discussed this in person and decided to keep the functions as they are. The main reasons are

We do not have an expression class, so we cannot represent x+y by itself as in the example above. Without a class representing expressions, and only a class representing a one-sided constraint, the functions imply a functionality that we do not support.

with two set-bound functions, it is possible to create a trivially unsolvable constraint 1 <= EXPRESSION <= 0 which cannot be represented in CPLEX. This pull request gets rid of the complexity that was necessary to deal with this situation and having both set-bounds functions would re-introduce it.

FlorianPommerening · 2026-07-30T10:20:06Z

    for (const LPConstraint &constraint : constraints) {
        num_nonzeros += constraint.get_coefficients().size();
    }
-


Was this intentional? I liked the newline there to separate the block dealing with nonzeros from the block dealing with constraints. If you prefer otherwise, that is also fine with me.

FlorianPommerening · 2026-07-30T10:21:35Z

+        const double b = constraint.get_right_hand_side();
+        const lp::Sense s = constraint.get_sense();


Suggested change

const double b = constraint.get_right_hand_side();

const lp::Sense s = constraint.get_sense();

const double rhs = constraint.get_right_hand_side();

const lp::Sense sense = constraint.get_sense();

FlorianPommerening · 2026-07-30T11:12:15Z

        con_id = get_constraint_id(f);
-        lp_solver.set_constraint_lower_bound(con_id, 0);
-        lp_solver.set_constraint_upper_bound(con_id, 0);
+        lp_solver.set_constraint_sense(con_id, lp::Sense::EQ);


Do we have to set the sense here (and in line 548)? If all these constraints start as equality constraints and are never changed, then not changing the sense here might be ok and more efficient.

FlorianPommerening · 2026-07-30T11:15:36Z

        missing_solver = "SoPlex";
 #endif
        break;
+


There is no newline before the other cases. I'd not add one here.

FlorianPommerening · 2026-07-30T11:18:57Z

-    } else if (lower_bound == -infinity) {
-        stream << " <= infinity";
-    }
+    stream << get_sense() << get_right_hand_side();


I think get_right_hand_side() could still return +/- infinity as a solver-internal constant, which would need to be translated to strings infinity and -infinity.

FlorianPommerening · 2026-07-30T11:44:15Z

-    rhs.resize(num_rows);
-    if (dense_range_values) {
-        range_values.resize(num_rows, 0);
+    if (num_rows > 0) {


Does this if make a difference here?

FlorianPommerening · 2026-07-30T11:45:31Z

-        sense[row_index] = sense_value;
+        double rhs_value = constraint.get_right_hand_side();
+        lp::Sense constraint_sense = constraint.get_sense();
+        char cplex_sense = constraint_sense_to_cplex_sense(constraint_sense);


if the variable is only needed in the next line, I'd directly assign to the array without a temporary variable.

gutodelazeri added 4 commits July 20, 2026 18:30

add patch from issue1199 implementing the new LP interface

3ef163b

cleanup code

7086399

format code

382c282

fix code formatting

b8a5d57

TravisRiveraPetit marked this pull request as ready for review July 28, 2026 10:43

TravisRiveraPetit requested review from FlorianPommerening, SimonDold, abcorrea and jendrikseipp and removed request for FlorianPommerening, SimonDold, abcorrea and jendrikseipp July 28, 2026 10:44

SimonDold reviewed Jul 28, 2026

View reviewed changes

FlorianPommerening reviewed Jul 30, 2026

View reviewed changes

		const double b = constraint.get_right_hand_side();
		const lp::Sense s = constraint.get_sense();

Uh oh!

Conversation

gutodelazeri commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Main changes

Important notes

Validation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FlorianPommerening left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gutodelazeri commented Jul 27, 2026 •

edited

Loading