TST: fix CI failures in test_all_nograd_minimizers

[dschmitz89]

Reference issue

#19994 (comment)

What does this implement/fix?

Increase test timeout to 40 seconds

[mdhaber]

Thanks! This would work, but the test took only 3s before adding COBYQA, and now it takes over 20s. Is it that slow locally? It might be worth a look at what's going on.

[tylerjereddy]

For reference, the test runs this fast locally on an i9-13900K:

5.13s call     build-install/lib/python3.11/site-packages/scipy/optimize/tests/test__basinhopping.py::TestBasinHopping::test_all_nograd_minimizers

If I do Matt's suggested experiment of parametrization, the timings drop off pretty fast after that new method.

4.43s call     build-install/lib/python3.11/site-packages/scipy/optimize/tests/test__basinhopping.py::TestBasinHopping::test_all_nograd_minimizers[COBYQA]
0.15s call     build-install/lib/python3.11/site-packages/scipy/optimize/tests/test__basinhopping.py::TestBasinHopping::test_all_nograd_minimizers[COBYLA]
0.13s call     build-install/lib/python3.11/site-packages/scipy/optimize/tests/test__basinhopping.py::TestBasinHopping::test_all_nograd_minimizers[CG]
0.08s call     build-install/lib/python3.11/site-packages/scipy/optimize/tests/test__basinhopping.py::TestBasinHopping::test_all_nograd_minimizers[TNC]
0.07s call     build-install/lib/python3.11/site-packages/scipy/optimize/tests/test__basinhopping.py::TestBasinHopping::test_all_nograd_minimizers[BFGS]

If you did parametrize, I believe you could use pytest.param to apply the timing exception or another marker to just that one test case, so that other regressions don't sneak in. If you apply xslow, then seems like it would only make sense to apply it to the one case that way.

[mdhaber]

The tolerance to pass the test is quite coarse, though. Can the termination settings be adjusted to avoid unnecessary iterations? Or why else is the new method so much slower than the others on this problem?

[andyfaff]

I wonder why cobyqa is taking much longer than the others? @ragonneau, is there something about the architecture of the minimiser that leads to longer runtime?

[mdhaber]

To get this passing, how about niter = 10 if method == 'COBYQA' else self.niter? (and specify niter=niter in the call to basinhopping)? Locally, the test still passes, and it's takes ~1/10 the time.

But it would also be good to see why the wall clock time is so high.

[dschmitz89]

The objective function used in that tests might just be a pathological edge case for cobyqa. Limiting the number of iterations sounds good to me.

[andyfaff]

Looks like the latest commit didn't improve the situation.

I'm not sure I like having a test fail in CI because it takes too long. It seems like a recipe for flaky CI performance. Performance of CI runners can be variable sometimes, so we might get random fails. If the aim is to reduce time used by CI we'd be better off working on other areas.

Lastly, it'd be nice to know why cobyqa takes so long for this specific test.

[andyfaff]

I'm not sure that rosen could be considered pathological, I would regard it as being well behaved. From some preliminary profiling the time spent evaluating the objective function is a small percentage, my impression is that there's a lot of overhead/inefficiencies in the cobyqa code.

e.g.

• merit method calculates everything in this if block, even if there are no constraints or bounds. These should be shortcut if there are no constraints or bounds, and they should all be cached for a given x if they've already been calculated in that position.
• that merit method is called repeatedly from Framework.set_best_index. It's called in a loop which makes inefficiencies in merit worse.
• elsewhere in the Framework.set_best_index method there are repeated calls to [Problem.maxcv]. There are no shortcuts when there are no bounds, no linear constraints, or no nonlinearconstraints, and the computation time for Problem.maxcv is non negligible because there are various computations being carried out that don't need to be.

If I do a naive patch to skip the code that has no effect (because I'm doing an N=10 rosen with no bounds, constraints), then I can speed up by ~25%, and these are the ones I found from a quick look.

[dschmitz89]

Improving COBYQA's performance would be the best option but take a longer time. Until then we should fix this annoying failure still.

[mdhaber]

The iteration reduction brought the duration from over 20s to 5s - but just a hair over 5s, which is why the test still failed. I increased the time limit to 10s on this test to fix CI, but yeah, I think it's worth looking deeper into why it's so slow.

Having slow tests fail is an experiment. Let's let it run for a while. This failure came up in main only because CI didn't run on the COBYQA PR after the pytest_fail_slow PR merged. In the future, the idea is that these sort of issues get fixed as part of PRs so they don't show up in main. Personally, I think pytest_fail_slow already found something valuable here. If CI variability causes too many intermittent failures, we can increase the default time limit or remove it entirely, but please bear with us while we assess whether the cure is worse than the disease here.

FWIW, I don't necessarily think that this is the best way to keep CI time down either. I just don't like it when my tests are in the spotlight for being too slow (e.g. gh-13859, gh-20420), so I thought I'd try to fix the problem once and for all for everyone rather than putting out fires every once in a while.

[ragonneau]

Hi everyone,

Sorry for my very late reply, I am recently very busy with some personal matters. First of all, thank you everyone for looking into this matter. We designed COBYQA for solving DFO problems, which usually requires several minutes/hours/days to perform one function evaluation. Hence, we did not attempt to tune the linear algebra of COBYQA (although we did everything to avoid doing stupid calculations). I agree that the code can definitely be improved in terms of linear algebra (to fasten the execution when the objective function evaluations does not require a long time).

Concerning the Rosenbrock function, it is indeed a pathological case, as it was designed as such. Even with $n = 2$, its valley shape makes it hard for optimisers to find the solution. It is normal that a consequent number of function evaluations is required, especially when $n$ increases (recall that in DFO, high dimension can sometimes mean $n \approx 50$).

@andyfaff, I agree with what you said. This is a typical example of things that can definitely be improved in the source code of COBYQA. I see you opened on issue on that matter, I will take a look at it this weekend. Again, thank you very much for the help! This is much appreciated.

I will try to be more present for this matter,
Thank you again everyone for the help,
Tom.

[andyfaff]

THanks for getting the suggested PR merged @ragonneau.

[ragonneau]

THanks for getting the suggested PR merged @ragonneau.

It's my pleasure. Again, thank you very much for your great help, and sorry for the delay in the merge.

Cheers,
Tom.