Map Constraint.Feasible/Infeasible to concrete constraints by jsiirola · Pull Request #3546 · Pyomo/pyomo

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.

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

        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

        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.

pyomo/core/base/constraint.py

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...?

OK - I have re-implemented Feasible and Infeasible to be singleton inequality expressions. These are preserved and stored in the ConstraintData.expr. This resulted in some baseline changes (the least of which is that the string representation of them is Feasible and Infeasible now). I made a similar change to LogicalConstraint (except that they are singleton BooleanConstant objects).

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

Convert Feasible/Infeasible to singletons

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

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 8 commits May 13, 2025 11:13

Remove references to LogicalStatement

3084cf8

NFC: fix typos

1c99111

Merge branch 'main' into feas-infeas

e2927ed

Update Constraint.Feasible/Infeasible from type to singleton

506888b

Make BooleanConstants singletons

f7cc5dc

Move LogicalConstraint.Feasible/Infeasible from type to BooleanConstant

329ee05

Relax (slightly) the simplification in Or/And.add()

a995d74

Update test baseline (new string output)

73054ec

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

jsiirola and others added 5 commits May 20, 2025 07:34

Update tests to reflect change in Feasible/Infeasible

0e21fea

Calling methods on a class can raise a TypeError; catch it

b8d1de7

Update constraint processing to track change in Constraint.(In)Feasible

1e6e15b

Merge branch 'main' into feas-infeas

c750576

Merge pull request #13 from jsiirola/singleton-feas-infeas

a75e382

Convert Feasible/Infeasible to singletons

jsiirola requested review from emma58 and mrmundt May 20, 2025 15:35

mrmundt approved these changes Jun 3, 2025

View reviewed changes

blnicho approved these changes Jun 3, 2025

View reviewed changes

blnicho merged commit a8d0bfa into Pyomo:main Jun 4, 2025
35 checks passed

jsiirola deleted the feas-infeas branch June 13, 2025 18:48

Conversation

jsiirola commented Mar 28, 2025

Fixes #2918 .

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

Uh oh!

michaelbynum commented May 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsiirola May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

emma58 left a comment

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jsiirola May 13, 2025 •

edited

Loading