Skip to content

Enable symbolic remake for optimization problems #1972

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

ValentinKaisermayer
Copy link
Contributor

@ValentinKaisermayer ValentinKaisermayer commented Nov 26, 2022

@ValentinKaisermayer ValentinKaisermayer changed the title Enable remake for optimization problems Enable symbolic remake for optimization problems Nov 26, 2022
src/variables.jl Outdated
"""
$(SIGNATURES)

Intercept the call to `process_p_u0_symbolic` and process symbolic maps of `p` and/or `u0` if the
user has `ModelingToolkit` loaded.
"""
function SciMLBase.process_p_u0_symbolic(prob::ODEProblem, p, u0)
function SciMLBase.process_p_u0_symbolic(prob::Union{ODEProblem, OptimizationProblem}, p,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we still need to do a lot more problem types 😓

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this function not also cover DEProblem in full and NonlinearProblem?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I think it can. It only expects parameters and states to be defined for the underlying system in prob.f.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's a DEProblem?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean an AbstractDEProblem?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

@ChrisRackauckas
Copy link
Member

That's an odd failure I haven't seen before... ?

@ValentinKaisermayer
Copy link
Contributor Author

That's an odd failure I haven't seen before... ?

Yeah, added a parameter and forgot to change the test. I will fix it.

@test prob_.p == [1.1, 1.2, 1.3]

prob_ = remake(prob, u0 = Dict(sys.y => 2.0), p = Dict(sys.a => 2.0))
@test_broken prob_.u0 == [1.0, 2.0, 1.0] # TODO: needs a `remake(prob::NonlinearProblem, ...)` in SciMLBase
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remember

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ChrisRackauckas ChrisRackauckas merged commit 0c83fdf into SciML:master Nov 27, 2022
@ChrisRackauckas
Copy link
Member

Thanks for seeing this through. It's absolutely fantastic to see this completed!

@ValentinKaisermayer ValentinKaisermayer deleted the vk-enable-remake-for-opt-problem branch November 27, 2022 16:31
@ValentinKaisermayer
Copy link
Contributor Author

thx

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

Successfully merging this pull request may close these issues.

2 participants