Add ifail enum#4275
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4275 +/- ##
==========================================
+ Coverage 48.79% 48.82% +0.03%
==========================================
Files 151 151
Lines 29383 29402 +19
==========================================
+ Hits 14337 14356 +19
Misses 15046 15046 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a typed IntEnum for solver ifail exit conditions (SolverOutputCondition) and updates several runtime and test call sites to use the enum instead of hard-coded numeric values, improving readability and consistency around solver status handling.
Changes:
- Added
SolverOutputConditionenum to centralize solver output (ifail) codes. - Replaced
ifail == 1/ifail != 1checks withSolverOutputCondition.CONVERGEDin core, tracking, and vary-run utilities. - Updated regression/integration tests to assert against the enum values.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tracking/tracking_data.py | Uses SolverOutputCondition.CONVERGED for strict convergence checks when reading MFILEs. |
| tests/regression/test_process_input_files.py | Updates regression assertion to use SolverOutputCondition.CONVERGED. |
| tests/integration/test_vmcon.py | Replaces numeric ifail expectations with enum values. |
| process/data_structure/numerics.py | Adds the new SolverOutputCondition IntEnum definition. |
| process/core/solver/solver_handler.py | Uses enum values in VMCON retry logic and imports the enum. |
| process/core/scan.py | Uses enum values in optimisation/solver status reporting and VMCON error handling. |
| process/core/io/vary_run/tools.py | Uses enum values when determining feasibility/convergence from MFILE contents. |
| process/core/io/vary_run/config.py | Uses enum values to report convergence status and generate README status messages. |
| process/core/final.py | Uses enum value for final feasible/unfeasible output header selection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # First solution attempt failed (ifail != 1): supply ifail value | ||
| # to next attempt |
| class SolverOutputCondition(IntEnum): | ||
| """Enum for the possible conditions that can be returned by the solvers. | ||
| This is for the `ifail` condition | ||
| """ |
| IMPROPER_INPUT = 0 | ||
| """Solver failed due to improper input (e.g. invalid parameters, or failure to | ||
| satisfy solver preconditions)""" | ||
| CONVERGED = 1 | ||
| """Solver converged successfully""" | ||
|
|
||
| MAX_ITERATIONS = 2 | ||
| """Solver failed to converge within the maximum number of iterations""" | ||
|
|
||
| MAX_LINE_SEARCHES = 3 | ||
| """Line search required 10 function calls without finding a better solution""" | ||
|
|
||
| UPHILL_SEARCH = 4 | ||
| """Uphill search direction was calculated""" | ||
|
|
||
| NO_SOLUTION = 5 | ||
| """No feasible solution or bad approximation of Hessian""" | ||
|
|
||
| SINGLE_MATRIX_OR_BOUNDS = 6 | ||
| """Singular matrix in quadratic subproblem or restriction by artificial bounds""" |
chris-ashe
force-pushed
the
add_ifail_enum
branch
from
6764908 to
6396b58
Compare
clmould
left a comment
clmould
left a comment
There was a problem hiding this comment.
Some docs points:
Can the SolverOutputCondition enum be included in the vmcon page in the docs, eg something like
- a brief sentence to say that the ifail values are now contained in this enum
- an extra column in the
ifailtable to link the ifail values to the enum
Also the troubleshooting docs page mentions ifail = 3 - think it'd be worth adding a reference to the the enum for this value and adding a brief description of what this means here
| ) | ||
| process_output.oblnkl(constants.IOTTY) | ||
|
|
||
| logger.critical("Solver returns with ifail /= 1. %s", ifail) |
There was a problem hiding this comment.
Can the SolverOutputCondition also be incorporated into this error message as it would make the message more descriptive for a user to see
| IMPROPER_INPUT = 0 | ||
| """Solver failed due to improper input (e.g. invalid parameters, or failure to | ||
| satisfy solver preconditions)""" | ||
| CONVERGED = 1 |
There was a problem hiding this comment.
| CONVERGED = 1 | |
| CONVERGED = 1 |
| NO_SOLUTION = 5 | ||
| """No feasible solution or bad approximation of Hessian""" | ||
|
|
||
| SINGLE_MATRIX_OR_BOUNDS = 6 |
There was a problem hiding this comment.
| SINGLE_MATRIX_OR_BOUNDS = 6 | |
| SINGLULAR_MATRIX_OR_BOUNDS = 6 |
| @@ -72,7 +73,7 @@ def run(self): | |||
| # to next attempt | |||
There was a problem hiding this comment.
I agree with the copilot suggestion on this line, as all the checks now use SolverOutputCondition which means comments referencing ifail != 1 etc aren't as clear any more so should be updated where this occurs
| class SolverOutputCondition(IntEnum): | ||
| """Enum for the possible conditions that can be returned by the solvers. | ||
| This is for the `ifail` condition | ||
| """ |
There was a problem hiding this comment.
Copilot comment is worth doing - some instances of if ifail == 1 remain
| This is for the `ifail` condition | ||
| """ | ||
|
|
||
| USER_TERMINATED = -1 |
There was a problem hiding this comment.
While self-explanatory, a short description here would be good for consistency
Description
Checklist
I confirm that I have completed the following checks: