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

Rebuild IpoptProblem only if modified #321

Merged
merged 11 commits into from
Aug 17, 2022
Merged

Rebuild IpoptProblem only if modified #321

merged 11 commits into from
Aug 17, 2022

Conversation

odow
Copy link
Member

@odow odow commented Jul 17, 2022

Part of jump-dev/JuMP.jl#1185
x-ref jump-dev/JuMP.jl#3018

For @ccoffrin's PF example: https://github.com/lanl-ansi/nlp-jump-examples/pull/9 (private link at present)

Without this PR

julia> @time bench_pf_nlparameter(case="pglib_opf_case2000_goc.m", n = 100)
 83.591282 seconds (225.00 M allocations: 34.441 GiB, 14.97% gc time, 1.07% compilation time)

With this PR

julia> @time bench_pf_nlparameter(case="pglib_opf_case2000_goc.m", n = 100)
 21.147063 seconds (27.64 M allocations: 1.517 GiB, 4.47% gc time)

@codecov
Copy link

codecov bot commented Jul 17, 2022

Codecov Report

Merging #321 (5028dd9) into master (5bf1161) will increase coverage by 0.12%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #321      +/-   ##
==========================================
+ Coverage   91.39%   91.52%   +0.12%     
==========================================
  Files           4        4              
  Lines         732      743      +11     
==========================================
+ Hits          669      680      +11     
  Misses         63       63              
Impacted Files Coverage Δ
src/MOI_wrapper.jl 88.78% <100.00%> (+0.30%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@odow odow force-pushed the od/reset-inner branch from 5fd3f8d to d6055e2 Compare July 18, 2022 01:26
@ccoffrin
Copy link
Contributor

A nice improvement for such minor updates.

@odow
Copy link
Member Author

odow commented Jul 18, 2022

It's actually not so straightforward, because it relies on jump-dev/JuMP.jl#3018 (comment). And it isn't obvious whether we need to call MOI.initialize if a parameter changes.

@odow
Copy link
Member Author

odow commented Jul 18, 2022

Regardless of the JuMP change, this is still a win if you want to repeatedly solve the same problem with different primal/dual starting points.

@ccoffrin
Copy link
Contributor

@odow good point! I had not noticed the JuMP change that is required.

Base automatically changed from od/inplace-resolve to master August 17, 2022 04:00
@odow
Copy link
Member Author

odow commented Aug 17, 2022

This does't solve the JuMP issue, because that needs changes in MOI/JuMP, but this is an uncontroversial change in Ipopt that still improves things somewhat (we don't rebuild if you want to change the starting point and re-optimize).

@odow odow merged commit 7162015 into master Aug 17, 2022
@odow odow deleted the od/reset-inner branch August 17, 2022 04:29
@ccoffrin
Copy link
Contributor

Still there are some good uses cases of this. Like multi-start inside of MINLP algorithms.

@odow
Copy link
Member Author

odow commented Aug 17, 2022

Like multi-start inside of MINLP algorithms.

Yes, this is exactly what I had in mind.

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

Successfully merging this pull request may close these issues.

3 participants