This feels like a move in the wrong direction. The use of two inequalities to express a constraint may be intentional, for reasons outside of Pyomo.

Codecov Report

Merging #2097 (01878cf) into main (828c551) will increase coverage by 0.00%.
The diff coverage is 83.33%.

@@           Coverage Diff           @@
##             main    #2097   +/-   ##
=======================================
  Coverage   82.87%   82.88%           
=======================================
  Files         619      619           
  Lines       76848    76852    +4     
=======================================
+ Hits        63689    63696    +7     
+ Misses      13159    13156    -3     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 828c551...01878cf. Read the comment docs.

@ghackebeil I spent some time thinking about exactly this. Some discussion points included should we relax this more (that is, allow an 'equality tolerance'), or leave it more strict (leaning toward a very narrow relaxation of the is test, e.g., just to cover the originally reported issue with 0. is 0. occasionally not being True and comparison of 0 with 0.). The final argument for this approach centered on:

• this removes a regression and restores behavior seen pre-6.1
• this is restricted to structural equality: it will only return True if the lower bound and upper bound are constants and equal
• if a user really wants to express this as two inequalities, they still can.

I'm simply saying that ranged inequality expressions may be important in whatever form the Pyomo model is translated into, whether or not the upper and lower bounds are constant, equal, parameters, or whatever. A user may want that structure retained (in, say, the LP or NL file). It seems inconsistent to turn a constraint into equality form, simply because the bound types or mutability changes.

My vote is for the entire elif self._expr.__class__ is logical_expr.RangedExpression: block to just be removed, but it's just a suggestion. ¯\_(ツ)_/¯

Should we have an explicit mechanism for users to have control over this behavior? This could be done a few different ways. We could have special constraint types or we could have functions that construct "the right thing" based on user input. I think it is best to make the default behavior something reasonable (probably what is implemented in this PR) but also allow users to have greater control if necessary.

@blnicho and I were talking about this, too. I think there are three possible interpretations of ConstraintData.equality:

1. Was the Constraint initialized with an EqualityExpression object (as opposed to an InequalityExpression or RangedExpression)?
2. Does the ConstraintData represent a "structural equality"; that is (1) plus RangedExpression where the upper and lower bounds are symbolically equal?
3. Does the ConstraintData represent a "numerical equality"; that is (1) plus RangedExpression where the upper and lower bounds are have equal value()?

Looking through the code base, there is not real consistency ... some places assume equality means case (1) (primarily transformations), and others (primarily writers) assume something closer to (3). @ghackebeil makes a good point that users may have had a good reason for expressing an equality as a ranged expression. Similarly, some solvers (like PyROS) would be much more interested in (3) (that is, the algorithm treats equality constraints very differently from inequalities).

We should probably document which one equality refers to (the property has never been documented), as well as how clients that care can get the other two versions if they care. One possible path forward is to deprecate equality and replace it with 1 or more new methods / properties that have more explicit names.

Closing this PR and replacing it with PEP #2105