Pyomo / pyomo

An object-oriented algebraic modeling language in Python for structured optimization problems.
https://www.pyomo.org
Other
2.05k stars 523 forks source link

Scaling transformation does not transform named Expressions #3344

Open dallan-keylogic opened 3 months ago

dallan-keylogic commented 3 months ago

Summary

When a model is transformed using the model scaling transformation, Expressions are created on the scaled model, but variable scaling factors are not added to them like they are for Constraints, resulting in them returning erroneous values. I've encountered this issue twice now and spent significant amounts of time debugging before I realized the scaling transform was the issue.

Steps to reproduce the issue

# expression_not_transformed.py
import pyomo.environ as pyo

m = pyo.ConcreteModel()

m.x = pyo.Var(initialize=10)
m.y = pyo.Var(initialize=0.1)

m.sum = pyo.Expression(expr=m.x+m.y)

m.scaling_factor = pyo.Suffix(direction=pyo.Suffix.EXPORT)
m.scaling_factor[m.x] = 0.1
m.scaling_factor[m.y] = 10

m_scaled = pyo.TransformationFactory('core.scale_model').create_using(m, rename=False)

print(f"Unscaled value: {pyo.value(m.sum)}")
print(f"Scaled value: {pyo.value(m_scaled.sum)}")

Results:

Unscaled value: 10.1
Scaled value: 2.0

Information on your system

Pyomo version: 6.7.4.dev0 Python version: 3.10.14 Operating system: Windows 10 How Pyomo was installed: Sources Solver (if applicable): n/a

Additional information

The behavior I would expect would be to transform the Expression like a Constraint, using a scaling suffix if provided and otherwise just using a value of 1. In that case, both values in the example script should return 10.1. If Pyomo does not want to support scaling of Expressions, then it shouldn't even generate them on the scaled model---returning some Exception for pyo.value(m_scaled.sum).

jsiirola commented 3 months ago

I am pretty sure it is not mathematically valid to have a scaling factor for an Expression. That said, I see a value in scaling the variables in the Expression objects.

dallan-keylogic commented 3 months ago

How is it not valid to have scaling factors for an Expression? It would be pointless if you're just going to totally substitute the expression into whatever constraints you have (because it would just cancel out with its inverse).

The reason we have scaling factors for Expressions in IDAES at the moment, though, is because: 1) We don't want to assume whether a property has been created as a Var or Expression at a unit model level, leaving that to the property package to decide 2) Often we have a much better idea of the scale of an Expression (for say, viscosity) than can be derived through just analyzing its contents. If we're calculating constraint scaling factors based on the values of the Vars and Expressions it contains, it is a useful intermediate.

Arguably, we shouldn't hijack Pyomo's scaling suffix system for those uses. We'll just have to see what Andrew comes up with to get rid of them (but I think it's going to be harder than he thinks).

Robbybp commented 3 months ago

The current scaling transformation behavior is to descend into named expressions while replacing scaled variables in constraints and objectives, and to remove these named expressions. Removing named expressions here seems unnecessary and costly, but was probably done so we don't scale the same named expression twice. Maybe we could:

jsiirola commented 3 months ago

I believe the other reason we removed named expressions was that we couldn't guarantee that there weren't other Constriaints / Objectives outside of the scope of the block that was being transformed that also referred to the same Expression object. Scaling the Expression in place would inadvertently (partially) scale the other Constraint (that was out-of-scope).

dallan-keylogic commented 3 months ago

I've found another consequence of the current implementation: the transformed model doesn't get to take advantage of the Expression-aware .nl writer. Unfortunately, that results in major slowdown in the model I'm currently working on, because eNRTL contains some chonky Expressions. The write is 6-12 times slower when I used the transformed model vs the untransformed model.