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

preallocate stats #25

Merged
merged 1 commit into from
Jul 12, 2022
Merged

preallocate stats #25

merged 1 commit into from
Jul 12, 2022

Conversation

dpo
Copy link
Member

@dpo dpo commented Jul 11, 2022

The objective of this PR is to preallocate the contents of the stats so they can be passed in to a solver for a re-solve, and so we can say something like stats.solution .= x to update the solution.

cf: #3
cc @sshin23

@dpo dpo requested review from abelsiqueira and tmigot July 11, 2022 23:46
@codecov
Copy link

codecov bot commented Jul 11, 2022

Codecov Report

Merging #25 (080fa9f) into main (b93373d) will decrease coverage by 0.21%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #25      +/-   ##
==========================================
- Coverage   88.67%   88.46%   -0.22%     
==========================================
  Files           3        3              
  Lines         106      104       -2     
==========================================
- Hits           94       92       -2     
  Misses         12       12              
Impacted Files Coverage Δ
src/stats.jl 88.23% <100.00%> (-0.28%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b93373d...080fa9f. Read the comment docs.

@github-actions
Copy link
Contributor

Package name latest stable
CaNNOLeS.jl
DCI.jl
DerivativeFreeSolvers.jl
JSOSolvers.jl
NLPModelsIpopt.jl
Percival.jl
RipQP.jl
SolverTest.jl

src/stats.jl Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

Package name latest stable
CaNNOLeS.jl
DCI.jl
DerivativeFreeSolvers.jl
JSOSolvers.jl
NLPModelsIpopt.jl
Percival.jl
RipQP.jl
SolverTest.jl

Copy link
Member

@tmigot tmigot left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

Copy link
Member

@abelsiqueira abelsiqueira left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.
Should we have a flag to let the user know if the solution and multipliers were touched? Along with a reset! method.

@github-actions
Copy link
Contributor

Package name latest stable
CaNNOLeS.jl
DCI.jl
DerivativeFreeSolvers.jl
JSOSolvers.jl
NLPModelsIpopt.jl
Percival.jl
RipQP.jl
SolverTest.jl

@dpo
Copy link
Member Author

dpo commented Jul 12, 2022

Should we have a flag to let the user know if the solution and multipliers were touched? Along with a reset! method.

What would reset! reset?

@geoffroyleconte
Copy link
Member

Should we have a flag to let the user know if the solution and multipliers were touched? Along with a reset! method.

What would reset! reset?

The reset! method is a good idea! Maybe it could reset the flag, elapsed_time, iter and counters? And maybe empty solver_specific?

@dpo
Copy link
Member Author

dpo commented Jul 12, 2022

I'm not sure I see the point of resetting the stats structure. Either it comes out a solve, and in that case it holds the solve info, or it's passed to solve!(), and in that case the solver will overwrite all of it. The counters are in the model, and those can be reset.

I don't see how to reset solver specific stuff, since only the solver knows what to do with that info.

@dpo dpo merged commit a8481ed into main Jul 12, 2022
@dpo dpo deleted the preallocate-stats branch July 12, 2022 17:58
@dpo
Copy link
Member Author

dpo commented Jul 12, 2022

Thanks guys!

@abelsiqueira
Copy link
Member

The flags are useful in particular for the contained solvers that don't touch the multipliers. That means that the solver needs to let the user know that the output.multipliers is not reliable.
The reset! would set the flags to their initial state.

@dpo
Copy link
Member Author

dpo commented Jul 12, 2022

In what situation would that occur? Presumably, the user knows if they're solving a constrained or unconstrained problem.

@abelsiqueira
Copy link
Member

The solver is for constrained optimization but does not give an estimate of the multipliers because it uses something different.

@dpo
Copy link
Member Author

dpo commented Jul 12, 2022

Ok, like TRON or the nonsmooth solvers.I'll add that.

@dpo dpo mentioned this pull request Jul 15, 2022
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.

4 participants