Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scaling transformation does not transform named Expressions #3344

Open
dallan-keylogic opened this issue Aug 13, 2024 · 5 comments
Open

Scaling transformation does not transform named Expressions #3344

dallan-keylogic opened this issue Aug 13, 2024 · 5 comments
Labels

Comments

@dallan-keylogic
Copy link

dallan-keylogic commented Aug 13, 2024

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
Copy link
Member

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
Copy link
Author

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
Copy link
Contributor

Robbybp commented Aug 13, 2024

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:

  • Refrain from descending into named expressions
  • Keep named expressions
  • Add Expression to the types of components whose variables are replaced

@jsiirola
Copy link
Member

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants