From 7905a5d754deeb9fb284a10310ef86106e15c6de Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= <edwin@etorok.net>
Date: Sun, 4 Jun 2023 15:46:52 +0100
Subject: [PATCH] SLSQP: fix wrong answer due to bogus stopping condition
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

SLSQP was giving the wrong answer on the tutorial in 't_python.py':
```
optimum at  [0.50777849 4.76252907]
minimum value =  2.182321944309753
```
instead of:
```
optimum at  [0.33333333 0.29629629]
minimum value =  0.5443310523133087
```

Slightly tweaking the function (e.g. to use repeated multiplication
instead of pow), or increasing xtol_rel to 1e-9 made it find the answer.
But the bogus optimum and real optimum are different by more than
xtol_rel.

Printing the function evaluations show that it reaches xtol_rel at an
infeasible point, and then returns the last feasible point:
```
f(0.337085195789626, 0.287031459045266)=0.535753169888211
c(0.337085195789626, 0.287031459045266)=0.019382838080164
c(0.337085195789626, 0.287031459045266)=0.004290454106768
f(0.337085195789626, 0.287031459045266)=0.535753169888211
c(0.337085195789626, 0.287031459045266)=0.019382838080164
c(0.337085195789626, 0.287031459045266)=0.004290454106768
optimum at  [0.50777849 4.76252907]
minimum value =  2.182321944309753
result code =  4
nevals =  22
```

Fix this by checking xtol_rel on mode==-1 only if the current point is
feasible, not when any previous point was feasible
(which matches how the original SLSQP's stopping condition was, and how
the other xtol_rel check is done in this same function).

Now the search continues and finds the correct answer:

```
f(0.337085195789626, 0.287031459045266)=0.535753169888211
c(0.337085195789626, 0.287031459045266)=0.019382838080164
c(0.337085195789626, 0.287031459045266)=0.004290454106768
f(0.298264335607084, 0.000001000000000)=0.001000000000000
c(0.298264335607084, 0.000001000000000)=0.212271613303731
c(0.298264335607084, 0.000001000000000)=0.345556758201188
[...]
f(0.333333334001826, 0.296296294513450)=0.544331052314168
c(0.333333334001826, 0.296296294513450)=0.000000003565495
c(0.333333334001826, 0.296296294513450)=0.000000000891522
optimum at  [0.33333333 0.29629629]
minimum value =  0.5443310523133087
result code =  4
nevals =  48
```

Fixes: 42c43f3 ("Only exit SLSQP successfully if solution is feasible (#465)")

Signed-off-by: Edwin Török <edwin@etorok.net>
---
 src/algs/slsqp/slsqp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/algs/slsqp/slsqp.c b/src/algs/slsqp/slsqp.c
index 635a8558..9e6f2158 100644
--- a/src/algs/slsqp/slsqp.c
+++ b/src/algs/slsqp/slsqp.c
@@ -2605,7 +2605,7 @@ nlopt_result nlopt_slsqp(unsigned n, nlopt_func f, void *f_data,
 	  /* note: mode == -1 corresponds to the completion of a line search,
 	     and is the only time we should check convergence (as in original slsqp code) */
 	  if (mode == -1) {
-	       if (!nlopt_isinf(fprev) && feasible) {
+	       if (!nlopt_isinf(fprev) && feasible_cur) {
 		    if (nlopt_stop_ftol(stop, fcur, fprev))
 			 ret = NLOPT_FTOL_REACHED;
 		    else if (nlopt_stop_x(stop, xcur, xprev))