Skip to content

Fix AMP tan/abs MINLPTests certification#279

Draft
bernalde wants to merge 2 commits into
jkitchin:mainfrom
bernalde:upstream-pr/issue-89-amp-minlptests
Draft

Fix AMP tan/abs MINLPTests certification#279
bernalde wants to merge 2 commits into
jkitchin:mainfrom
bernalde:upstream-pr/issue-89-amp-minlptests

Conversation

@bernalde

Copy link
Copy Markdown
Contributor

Summary

This fixes the current AMP certification failure for the continuous tan/abs MINLPTests case nlp_004_010.

Fork-side evidence:

Root Cause

For pure-continuous AMP candidates with nlp_solver="ipm", the main AMP loop only tried the POUNCE path. The final pure-continuous recovery path already tried ipopt first when cyipopt was available, then fell back to ipm/POUNCE. As a result, nlp_004_010 could spend 1000 AMP refinements without an in-loop incumbent, then recover only a final feasible incumbent with no certified bound.

This PR uses the same ipopt -> ipm sequence inside the pure-continuous AMP candidate path, and makes backend fallback robust when one backend raises.

Changes

  • Allows _solve_nlp_subproblem to accept an ordered NLP backend sequence.
  • Uses ("ipopt", "ipm") for pure-continuous nlp_solver="ipm" candidates when cyipopt is available.
  • Keeps integer-fixed NLP subproblems on the existing single-backend behavior.
  • Continues to later fallback backends if an earlier backend raises.
  • Adds regression coverage for both backend selection and exception fallback.
  • Corrects the issue reference in the existing tan/abs integration-test docstring.

Validation

Fork PR #97 was reviewed and merged before opening this upstream PR.

Local validation reported on the fork branch:

  • python/tests/test_amp.py::test_solve_nlp_subproblem_uses_pounce_and_restores_bounds
  • python/tests/test_amp.py::test_solve_nlp_subproblem_continues_after_backend_exception
  • python/tests/test_amp.py::test_solve_nlp_subproblem_respects_expired_time_limit
  • python/tests/test_amp.py::test_recover_pure_continuous_solution_uses_best_start_and_maximize_sign
  • python/tests/test_amp.py::test_best_nlp_candidate_uses_ipopt_fallback_for_pure_continuous_ipm
  • python/tests/test_amp_integration.py::TestCurrentCodeWeaknesses::test_amp_certifies_tan_abs_nlp_004_010_at_issue_gap
  • python/tests/test_amp_integration.py::TestCurrentCodeWeaknesses::test_amp_reports_bound_for_tan_abs_nlp_004_010
  • python/tests/test_nlp_pounce.py -> 18 passed
  • Filtered AMP-fast selection from the Makefile -> 135 passed, 15 deselected
  • ruff check on touched/relevant files -> passed
  • Commit hook -> ruff, ruff format, mypy passed

The exact issue repro changed from a local failure after about 146 seconds (status="feasible", no certified bound) to a pass with status="optimal" in about 1-2 seconds locally.

Notes

This PR intentionally does not claim to close broader AMP slow-tail tracking. It addresses the current tan/abs certification failure surfaced while triaging the fork issue.

@bernalde

Copy link
Copy Markdown
Contributor Author

I tried to add the existing amp-integration label so this PR runs the opt-in AMP integration workflow, but my token does not have permission on jkitchin/discopt (HTTP 403). A maintainer can trigger it by adding that label.\n\nSeparate naming note: the skipped job is currently named AMP Alpine/incidence suite. It now points at python/tests/test_amp_integration.py, where the relevant coverage for this PR is AMP incidence / partition behavior and MINLPTests-style cases rather than Alpine as an active dependency or path. Since Alpine is no longer the path we use here, would it make sense to rename the job to something like AMP integration (Ipopt, incidence, MINLPTests) while keeping the existing amp-integration label?

@jkitchin

Copy link
Copy Markdown
Owner

do you have examples where ipopt works and pounce doesn't? if so, I would like to see them; pounce has gotten quite good and has been very comparable to ipopt/ma57 so far, and better than the standard cyipopt (ipopt with mumps) many times.

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