Replaced nlopt with scipy in dispersion fitter

[tylerflex]

In an attempt to remove nlopt from requirements, this PR replaces nlopt with the scipy.optimize.minimize from scipy.

It needs a 2nd pair of eyes on it to make sure I set up the constriaints and various parameters correctly, could you take a look Weiliang?

When running the dispersion fitter tests in test/plugins, I get similar (but slightly worse) final results:

with nlopt

_________________________________ test_dispersion _________________________________
------------------------------ Captured stdout call -------------------------------
best RMS error so far: 3.20e-01 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00
-------------------------------- Captured log call --------------------------------
WARNING  rich:fit.py:364 	warning: did not find fit with RMS error under tolerance_rms of 1.00e-02
INFO     rich:fit.py:368 	returning best fit with RMS error 3.20e-01
______________________________ test_dispersion_load _______________________________
------------------------------ Captured stdout call -------------------------------
best RMS error so far: 9.95e-02 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00
-------------------------------- Captured log call --------------------------------
WARNING  rich:fit.py:364 	warning: did not find fit with RMS error under tolerance_rms of 1.00e-02
INFO     rich:fit.py:368 	returning best fit with RMS error 9.95e-02
______________________________ test_dispersion_plot _______________________________
------------------------------ Captured stdout call -------------------------------
best RMS error so far: 9.95e-02 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00
-------------------------------- Captured log call --------------------------------
WARNING  rich:fit.py:364 	warning: did not find fit with RMS error under tolerance_rms of 1.00e-02
INFO     rich:fit.py:368 	returning best fit with RMS error 9.95e-02
__________________________ test_dispersion_set_wvg_range __________________________
------------------------------ Captured stdout call -------------------------------
best RMS error so far: 2.99e-01 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00
-------------------------------- Captured log call --------------------------------
WARNING  rich:fit.py:364 	warning: did not find fit with RMS error under tolerance_rms of 1.00e-02
INFO     rich:fit.py:368 	returning best fit with RMS error 2.99e-01

with scipy:

_________________________________ test_dispersion _________________________________
------------------------------ Captured stdout call -------------------------------
best RMS error so far: 4.05e-01 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00
-------------------------------- Captured log call --------------------------------
WARNING  rich:fit.py:364 	warning: did not find fit with RMS error under tolerance_rms of 1.00e-02
INFO     rich:fit.py:368 	returning best fit with RMS error 4.05e-01
______________________________ test_dispersion_load _______________________________
------------------------------ Captured stdout call -------------------------------
best RMS error so far: 9.95e-02 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00
-------------------------------- Captured log call --------------------------------
WARNING  rich:fit.py:364 	warning: did not find fit with RMS error under tolerance_rms of 1.00e-02
INFO     rich:fit.py:368 	returning best fit with RMS error 9.95e-02
______________________________ test_dispersion_plot _______________________________
------------------------------ Captured stdout call -------------------------------
best RMS error so far: 9.95e-02 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00
-------------------------------- Captured log call --------------------------------
WARNING  rich:fit.py:364 	warning: did not find fit with RMS error under tolerance_rms of 1.00e-02
INFO     rich:fit.py:368 	returning best fit with RMS error 9.95e-02
__________________________ test_dispersion_set_wvg_range __________________________
------------------------------ Captured stdout call -------------------------------
best RMS error so far: 3.31e-01 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00
-------------------------------- Captured log call --------------------------------
WARNING  rich:fit.py:364 	warning: did not find fit with RMS error under tolerance_rms of 1.00e-02
INFO     rich:fit.py:368 	returning best fit with RMS error 3.31e-01

[momchil-flex]

What about having the option to use nlopt if installed, and using that on our webservice?

[weiliangjin2021]

What about having the option to use nlopt if installed, and using that on our webservice?

Webservice is largely independent of frontend code except for medium. So we can use NLOPT in webservice as long as we add NLOPT as requirement in webservice part independently.

[momchil-flex]

Well we need to keep the code that uses nlopt in here. I don't think we should have special code for the webservice. So what I'm suggesting is an extra parameter to the fit function, like optimizer = ["scipy", "nlopt"] or something (could be even more specific to which optimize function exactly to use from these packages), and in the webservice we just use the nlopt one. We can remove nlopt as a general dependence and raise an appropriate error prompting the user to try to install it if someone tries to run the fitter locally with nlopt optimizer and nlopt missing.

[tylerflex]

I guess if we can get scipy working as well as nlopt, i see no reason to keep nlopt personally.

…

On Fri, Aug 5, 2022 at 6:03 PM momchil-flex ***@***.***> wrote: Well we need to keep the code that uses nlopt in here. I don't think we should have special code for the webservice. So what I'm suggesting is an extra parameter to the fit function, like optimizer = ["scipy", "nlopt"] or something (could be even more specific to which optimize function exactly to use from these packages), and in the webservice we just use the nlopt one. We can remove nlopt as a general dependence and raise an appropriate error prompting the user to try to install it if someone tries to run the fitter locally with nlopt optimizer and nlopt missing. — Reply to this email directly, view it on GitHub <#457 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AWDVXGH477HICC524FL3V43VXVCM5ANCNFSM55VIOKCQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[momchil-flex]

Well, sure, but I'm dubious we'll be able to (there's a reason nlopt exists...)

[tylerflex]

Well with almost no effort i was able to get pretty close to nlopt in my PR removing nlopt so i feel like it is probably possible

…

On Fri, Aug 5, 2022 at 6:14 PM momchil-flex ***@***.***> wrote: Well, sure, but I'm dubious we'll be able to (there's a reason nlopt exists...) — Reply to this email directly, view it on GitHub <#457 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AWDVXGG7FYUVIB3D33BDRHLVXVDX7ANCNFSM55VIOKCQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[weiliangjin2021]

One issue I read from scipy doc is that constraints only workfor COBYLA, SLSQP and trust-constr. So we need to check if we switch from Nelder-Mead to any of those algorithm, will the optimization be fast and converge to good result?

[weiliangjin2021]

Looks good, but we shouldn't change anything in fit_web.py yet. The webserivce will still be using NLOPT for now, and changing it will cause compatibility issue. Other than that, feel free to merge.

[tylerflex]

Thanks @weiliangjin2021 , I guess the remaining question is how we should test how scipy works vs. nlopt. or whether this is even necessary with Momchil's recent changes in #452. I guess the main advantage of removing nlopt is to make it less likely users will have installation problems, like #441 , but if the performance of the fit is much worse, it's probably not worth it. If we test on the notebooks and test_plugins.py is that enough to say for sure that the scipy optimizer is working adequately?

[weiliangjin2021]

I think the test should be purely on webservice side, so nlopt can be completely removed from the fronted part.

[momchil-flex]

So can we go ahead and wrap this up?

[weiliangjin2021]

For the frontend, it should be good to go.