Skip to content

Comments

add step_status and step_status_reliable to stats#130

Merged
dpo merged 2 commits intomainfrom
step-accepted
Feb 19, 2026
Merged

add step_status and step_status_reliable to stats#130
dpo merged 2 commits intomainfrom
step-accepted

Conversation

@dpo
Copy link
Member

@dpo dpo commented Feb 13, 2026

Each solver can now set the step status (accepted or rejected) for use in a callback.
This is particularly useful to perform quasi-Newton updates.

Related issues

There is no related issue.

Checklist

  • I am following the contributing guidelines
  • Tests are passing
  • Lint workflow is passing
  • Docs were updated and workflow is passing

Each solver can now set the step status (accepted or rejected)
for use in a callback.
This is particularly useful to perform quasi-Newton updates.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR extends GenericExecutionStats with a per-iteration step_status (and step_status_reliable) so solvers can report whether the most recent step was accepted/rejected for downstream callbacks (e.g., quasi-Newton updates).

Changes:

  • Added step_status/step_status_reliable fields and a set_step_status! setter, plus supporting STEP_STATUSES utilities.
  • Extended statsgetfield to render :step_status as a human-readable string.
  • Updated the NLPModels extension constructor and expanded tests to cover the new setter and reliability flag.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/stats.jl Adds step-status storage, validation, setter, and formatted access via statsgetfield.
ext/SolverCoreNLPModelsExt.jl Threads the new step_status keyword through the NLPModels-specific constructor.
test/test-stats.jl Adds tests for invalid step statuses and for the new reliability flag.
Comments suppressed due to low confidence (1)

ext/SolverCoreNLPModelsExt.jl:85

  • In the NLPModels extension constructor you validate status via SolverCore.check_status(status), but the newly added step_status keyword is not validated. This allows invalid step_status values to be stored at construction time and later crash when rendering/logging (e.g., getStepStatus indexing STEP_STATUSES). Consider calling SolverCore.check_step_status(step_status) here as well (mirroring the status check).
  step_status::Symbol = :unknown,
  elapsed_time::Real = Inf,
  solver_specific::Dict{Symbol, Tsp} = Dict{Symbol, Any}(),
) where {T, S, V, Tsp}
  SolverCore.check_status(status)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 13, 2026

Package name latest stable
AdaptiveRegularization
CaNNOLeS
DCISolver
FletcherPenaltySolver
JSOSolvers
JSOSuite
NLPModelsIpopt
NLPModelsJuMP
NLPModelsKnitro
PartiallySeparableSolvers
Percival
QuadraticModels
QuadraticModelsCPLEX
QuadraticModelsGurobi
QuadraticModelsXpress
RegularizedOptimization
RipQP
SolverBenchmark
SolverTest

src/stats.jl Outdated
- `multipliers_L`: The Lagrange multipliers wrt to the lower bounds on the variables (default: an uninitialized vector like `nlp.meta.x0` if there are bounds, or a zero-length vector if not);
- `multipliers_U`: The Lagrange multipliers wrt to the upper bounds on the variables (default: an uninitialized vector like `nlp.meta.x0` if there are bounds, or a zero-length vector if not);
- `iter`: The number of iterations computed by the solver (default: `-1`);
- `step_status`: The status of the most recently computed step (`:unknown`, `:accepted` or `:rejected`);
Copy link
Member

Choose a reason for hiding this comment

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

see show_step_statuses() for a list

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

@tmigot tmigot mentioned this pull request Feb 15, 2026
@dpo dpo requested a review from tmigot February 18, 2026 19:54
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.

It looks good, thanks. Can you make a new release after that ? I think Maxence was waiting for it (see pr bumping the version number)

@dpo dpo merged commit 75feed0 into main Feb 19, 2026
43 of 45 checks passed
@dpo dpo deleted the step-accepted branch February 19, 2026 15:09
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