-
-
Notifications
You must be signed in to change notification settings - Fork 235
Add tune_parameters for dynamic optimization #4017
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
take the MTKParameters values into account and substitute the tunable parameters with the appropriate symbolic expression
Co-authored-by: Claude <noreply@anthropic.com>
…ameters in the system
Co-authored-by: Claude <noreply@anthropic.com>
24a225f to
a57a464
Compare
When we have more collocation points than expected the constraints are wrong
…face Co-authored-by: Claude <noreply@anthropic.com>
| function MTK.get_param_for_pmap(m::ConcreteModel, P::PyomoVar, i) | ||
| # Create a symbolic variable that will be used in the pmap | ||
| # The actual PyomoVar will be accessed via the symbolic representation | ||
| @variables MODEL_SYM::Symbolics.symstruct(ConcreteModel) |
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.
Wait, does symstruct even work?
It's also not present in v7, so porting this PR to v11 will be challenging in this form.
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.
Apparently Pyomo uses symstruct even before this PR. Would you happen to know if it can be removed?
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.
What's the difference between this and @variables MODEL_SYM::ConcreteModel? We have pysym_getproperty, so that could just be registered.
function pysym_getproperty(s::Union{Num, Symbolics.Symbolic}, name::Symbol)
Symbolics.wrap(SymbolicUtils.term(
_getproperty, Symbolics.unwrap(s), Val{name}(), type = Symbolics.Struct{PyomoVar}))
end
_getproperty(s, name::Val{fieldname}) where {fieldname} = getproperty(s, fieldname)
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 think that makes sense. symstruct was Yingbo's symbolic struct, but it doesn't work with MTK because it doesn't give MTK enough information.
| function MTK.get_param_for_pmap(m::ConcreteModel, P::PyomoVar, i) | ||
| # Create a symbolic variable that will be used in the pmap | ||
| # The actual PyomoVar will be accessed via the symbolic representation | ||
| @variables MODEL_SYM::Symbolics.symstruct(ConcreteModel) |
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.
The symstruct worries me, since I haven't used it anywhere else and it doesn't exist in Symbolics v7. Do you know if this (and the one other usage) can be removed?
Co-authored-by: Aayush Sabharwal <aayush.sabharwal@gmail.com>
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
This fixes #3939 by adding the
tune_parametersoption for all dynamic optimization extensions. I also did fix some existing issues with the interface, such as the fact that it was ignoring any defaults in the model and added error handling for JuMPDynamicOptProblem in the case where the cost function evaluated variables at points that were not in the support (in that case InfiniteOpt automatically adds new points, which breaks the way we set the constraints for U, as we have completely different time steps).In terms of support, I'd say the best extension is the
InfiniteOptDynamicOptProblem, since it directly uses the InfiniteOpt representation and can automatically handle non-uniform data evaluation. TheJuMPDynamicOptProblem&CasADiDynamicOptProblemwould be next, where CasADi gets ranked lower because for some reason it doesn't work when used in the VS Code REPL. This also happens forPyomoDynamicOptProblem, but that one also has the issue that it crashes julia if it tries to evaluate a variable at a point that's not in the collocation points.Another issue that I noticed is that
solve(jprob, JuMPCollocation(Ipopt.Optimizer, constructTsitouras5()))takes very long if we increase the number of collocation points by decreasingdt. I haven't profiled yet, but I think it might be in the way we are adding the constraints based on the tableau & RGF tracing.One thing that's missing in the interface is the ability to set box constraints on the parameters that we tune, but I think that should go in a separate PR, as there are some more interface questions that I have related to that. In particular, how does one set the bounds after model creation? In most cases, if you want to reuse a model, you would only know the bounds on the parameters to optimize when you are building the dynamic optimization problem. We can change the values of the bounds for a parameter in an out of place way, but as far as I can tell, there's no way of putting the parameter back into the system after we changed its bounds. We can't use
extendeither, since the base system has priority.In a similar note, we also only know the tunable parameters & the cost function after system construction, so the process of setting up the dynamic optimization problem is a bit involved. Would it make sense to have some utility functions that set up the search space in an easier manner from a vector of pairs (i.e. something like
[sys.x=>(1,2), sys.y=>(2,3)]would set the tunables to[x, y]and set the bounds to the appropriate values).Also, regarding the interface, it's a bit strange that the solution does not look like any other SciML solution in that it does have 2 separate solutions and it doesn't have retcodes or parameters directly accessible at the top level.
Other missing things include more solver options that can be passed to the underlying optimizer, but that can be added after the more important aspects are clarified.
cc @baggepinnen