Map Constraint.Feasible/Infeasible to concrete constraints #3546

jsiirola · 2025-03-28T06:39:52Z

Fixes #2918 .

Summary/Motivation:

We have historically mapped Constraint.Feasible to Constraint.Skip and had Constraint.Infeasible immediately raise a ValueError. This causes some undesirable behavior:

You cannot "evaluate" a Constraint that was set by Constraint.Feasible because the ConstraintData was not created (see this question on SO)
You cannot (easily) have a trivially infeasible constraint anywhere in the model, even if the infeasible constraint would not cause the model to be infeasible (because it was on a disjunct; see Allow Constraint.Infeasible to be used to disable a Disjunct #2918)

This updates Constraint and LogicalConstraint to map Constraint.Feasible to a trivial inequality, and Constraint.Infeasible to a trivial infeasible Inequality.

Changes proposed in this PR:

Map Constraint.Feasible and Constraint.Infeasible to actual trivial constraints
Rework part of Constraint to remove a repeated exception
Port LogicalConstraint to the new structure (initillizers, disable_method, Abstract classes, simplified method overrides) from Constraint
Update logical_to_linear and logical_to_disjunctive to support constant expressions
Update tests

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

I agree my contributions are submitted under the BSD license.
I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

…eption

This permits assigning logical expressions based on IntervalVar objects, restoring previous behavior. We should revisit template expressions in the future to permit better expression type validation.

mrmundt · 2025-04-08T18:37:50Z

A note from the dev call on 4/8 - we don't think we need to especially rush to get this in by the patch release next week, but it would be nice if we could.

michaelbynum · 2025-05-02T05:30:31Z

I feel like I am missing some context for this PR. Other than the example in #2918, I can't think of a use case for Constraint.Infeasible (I would just raise an exception instead of returning Constraint.Infeasible). Having said that, if it is used in a non-GDP model, this PR would postpone an exception to when a solver complains, which would make debugging the model significantly more work. Also, if Constraint.Feasible is used heavily, this PR could lead to unnecessarily large models. Having said that, one can always use Constraint.Skip instead of Constraint.Feasible.

In short, I'm not really a user of Constraint.Feasible or Constraint.Infeasible, but I could see this PR potentially leading to some undesirable effects. Maybe it would be good to discuss when and why these are currently used?

mrmundt · 2025-05-06T19:20:42Z

pyomo/core/base/constraint.py

@@ -622,7 +623,9 @@ class Constraint(ActiveIndexedComponent):
    class Infeasible(object):
        pass

-    Feasible = ActiveIndexedComponent.Skip
+    class Feasible(object):


Suggested change

class Feasible(object):

class Feasible:

pyomo/core/expr/boolean_value.py

mrmundt · 2025-05-06T19:22:55Z

pyomo/core/base/logical_constraint.py

@@ -188,118 +221,94 @@ class LogicalConstraint(ActiveIndexedComponent):
    class Infeasible(object):
        pass

-    Feasible = ActiveIndexedComponent.Skip
+    class Feasible(object):


Suggested change

class Feasible(object):

class Feasible:

mrmundt · 2025-05-06T19:24:16Z

pyomo/core/base/logical_constraint.py

-                self.add(expr)
+        if self.rule is not None:
+            _rule = self.rule(self.parent_block(), ())
+            for cc in iter(_rule):


What does cc stand for?

Not entirely sure - it is just a temporary, and was copied from the implementation in ConstraintList (and preserved for consistency)

pyomo/core/plugins/transform/logical_to_linear.py

emma58

This is really nice! I'm very happy about the implications for GDP. :) I have a few questions and a couple comments, but mostly just so I understand things.

emma58 · 2025-05-13T13:52:10Z

pyomo/core/base/constraint.py

-_known_relational_expressions = {
+_known_relational_expression_types = {


I like this name change a lot--it's much clearer!

emma58 · 2025-05-13T13:55:51Z

pyomo/core/base/constraint.py

+                # Note that an inequality is sufficient to induce
+                # infeasibility and is simpler to relax (in the Big-M
+                # sense) than an equality.
+                self._expr = InequalityExpression((1, 0), False)
+                return
+            elif expr is Constraint.Feasible:
+                # Note that we do not want to provide a trivial equality
+                # constraint as that can confuse solvers like Ipopt into
+                # believing that the model has fewer degrees of freedom
+                # than it actually has.
+                self._expr = InequalityExpression((0, 0), False)
+                return


Just so that I understand: Why do we have to put trivial expressions at all? Could we not consider Constraint.Feasible and Constraint.Infeasible to be (weird) relational expression nodes in the expression tree, themselves?

We certainly could. That is, we could make Constraint.Feasible and Constraint.Infeasible be singleton expression objects (and not types). I think I would still make them special cases of InequalityExpression, mostly so that all the walkers, writers, and whatnot don't have to be updated to handle them as special cases. This just seemed simpler?

Maybe it is simpler, but it's a bit confusing to me why, because it feels like we lose information in this step rather than carrying it through. Because if they were singleton expression objects, then wouldn't we would know after set_value that the Constraint is either non-trivial or it's Constraint.Infeasible or Constraint.Feasible? With this implementation, it could be trivial, and that has to be checked later (by transformations, writers, whatever). Otherwise downstream things would know they either have these special nodes or a non-trivial constraint. Which might be nice...?

pyomo/core/base/logical_constraint.py

emma58 · 2025-05-13T14:14:01Z

pyomo/core/base/logical_constraint.py

-    def body(self):
+    def expr(self):


Does this PR deprecate body? I agree with the change, but missed the magic somewhere, I think?

Nope. .body is still implemented in the LogicalConstraintData base class and is an alias to .expr

jsiirola added 12 commits March 27, 2025 23:13

Map Constraint.Infeasible/Feasible to trivial inequalities

0e04951

Rework set_value to remove duplicate exception

dc53dbc

Clarify reference set name

d5cdeec

Use more modern super() / type() syntax

fe3ccb5

Fix typo

5ddf0b6

Port constraint rework to logical_constraint

537cd7b

Support passing Feasible/Infeasible in Disjunction expression lists

efa4381

Support BooleanConstant in logical_to_disjunctive

57c3a27

Handle constant expressions in logical_to_disjunctive

9e05fa9

Support constant expressions in logical_to_linear

8943ebb

Update tests for trivial feasible/infeasible constraints

ccffb16

NFC: justify using inequalities over equalities

ceca444

jsiirola requested a review from emma58 March 28, 2025 06:39

jsiirola changed the title ~~May Constraint.Feasible/Infeasible to concrete constraints~~ Map Constraint.Feasible/Infeasible to concrete constraints Mar 28, 2025

jsiirola added 14 commits March 29, 2025 23:03

set_value() should not change the constraint data when raising an exc…

f2c0147

…eption

Add 'expr' to disable_methods, mab body through expr

869ef48

Use __name__ for logger scope

40787a9

Remove unreachable code

e8b3219

Leverage is_logical_type for detecting logical expressions/vars

935b443

fix construction log message

0faf9b1

Support starting_index for LogicalConstraintList

592d2fa

Make as_boolean behave similar to as_numeric

3247246

Improve LogicalConstraint test coverage

583593c

NFC: apply black

fd98977

NFC: fix typo

eab8828

Resolve test failures due to message change

847dc5a

Relax LogicalConstraint expression validation.

9151e93

This permits assigning logical expressions based on IntervalVar objects, restoring previous behavior. We should revisit template expressions in the future to permit better expression type validation.

Merge branch 'main' into feas-infeas

5a821ae

jsiirola mentioned this pull request Mar 31, 2025

Bug Report: Pull reports do not track PR branches codecov/feedback#692

Open

Merge branch 'main' into feas-infeas

3d31076

jsiirola added 2 commits April 1, 2025 14:38

Fix bad merge

3e188e8

Merge branch 'main' into feas-infeas

d3ff6d4

mrmundt requested a review from michaelbynum April 8, 2025 18:42

blnicho added this to Pyomo Release April 2025 Apr 14, 2025

github-project-automation bot moved this to Todo in Pyomo Release April 2025 Apr 14, 2025

blnicho moved this from Todo to Review In Progress in Pyomo Release April 2025 Apr 14, 2025

mrmundt self-requested a review April 15, 2025 18:55

mrmundt removed this from Pyomo Release April 2025 Apr 15, 2025

pyomo-autotest added the AT: STALE label Apr 22, 2025

mrmundt requested changes May 6, 2025

View reviewed changes

emma58 reviewed May 13, 2025

View reviewed changes

jsiirola added 3 commits May 13, 2025 11:13

Remove references to LogicalStatement

3084cf8

NFC: fix typos

1c99111

Merge branch 'main' into feas-infeas

e2927ed

pyomo-autotest removed the AT: STALE label May 13, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Map Constraint.Feasible/Infeasible to concrete constraints #3546

Map Constraint.Feasible/Infeasible to concrete constraints #3546

jsiirola commented Mar 28, 2025

mrmundt commented Apr 8, 2025

michaelbynum commented May 2, 2025

mrmundt May 6, 2025

mrmundt May 6, 2025

mrmundt May 6, 2025

jsiirola May 13, 2025 •

edited

Loading

emma58 left a comment

emma58 May 13, 2025

emma58 May 13, 2025

jsiirola May 13, 2025

emma58 May 13, 2025

emma58 May 13, 2025

jsiirola May 13, 2025

		_known_relational_expressions = {
		_known_relational_expression_types = {

Map Constraint.Feasible/Infeasible to concrete constraints #3546

Are you sure you want to change the base?

Map Constraint.Feasible/Infeasible to concrete constraints #3546

Conversation

jsiirola commented Mar 28, 2025

Fixes #2918 .

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

mrmundt commented Apr 8, 2025

michaelbynum commented May 2, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsiirola May 13, 2025 • edited Loading

Choose a reason for hiding this comment

emma58 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsiirola May 13, 2025 •

edited

Loading