-
Notifications
You must be signed in to change notification settings - Fork 533
BUG FIXES: Solver Refactor for IDAES Integration #3214
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
Conversation
pyomo/contrib/solver/base.py
Outdated
# Something wacky happens in IDAES due to the usage of ScalarBlock | ||
# instead of PyomoModel. This is an attempt to fix that. | ||
from pyomo.core.base.PyomoModel import ModelSolutions | ||
|
||
setattr(model, 'solutions', ModelSolutions(model)) | ||
model.solutions.add_symbol_map(symbol_map) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems weird to include here. Is this pointing to an edge case that was missed in #110 that should be fixed directly in the Block
or _BlockData
class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely, yes, it should be. It was something that Andrew ran into while trying to get this working for IDAES, since they inherit from ScalarBlock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really wacky - and probably not something that should be added to Block (in the new world order, there should never be a reason to attach a symbolmap to the model)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would you like to handle this? It causes issues in IDAES if it's not present because they use ScalarBlock
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a commit to @andrewlee94's branch on IDAES to resolve this: andrewlee94/idaes-pse@532d23b
Thanks! I too was confused by this when trying the new interface a few days ago. |
pyomo/contrib/solver/base.py
Outdated
# Something wacky happens in IDAES due to the usage of ScalarBlock | ||
# instead of PyomoModel. This is an attempt to fix that. | ||
from pyomo.core.base.PyomoModel import ModelSolutions | ||
|
||
setattr(model, 'solutions', ModelSolutions(model)) | ||
model.solutions.add_symbol_map(symbol_map) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really wacky - and probably not something that should be added to Block (in the new world order, there should never be a reason to attach a symbolmap to the model)
… sol-refac-nconstraint
Fixes
Summary/Motivation:
Working with @andrewlee94 on adding initial support for v2 solvers in IDAES, we discovered some bugs. This PR addresses those bugs.
Changes proposed in this PR:
model.nobjectives
(etc.) due to them not existing onScalarBlocks
solver_options
andwriter_config
inLegacySolverWrapper
options
andsolver_options
in the same commandLegal Acknowledgement
By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution: