Update to allow no bounds

[NicolaCourtier]

• Use the spm_adam example to test optimisation without bounds, since Adam does not use them
• Allow bounds=None in place of lower and upper bounds
• Add checks to test_parameters
• Raise an error in plot_cost2d if bounds not available

[codecov]

Codecov Report

Attention: Patch coverage is 94.52055% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 94.34%. Comparing base (29c6ff2) to head (55b6bcf).
Report is 10 commits behind head on develop.

Files	Patch %	Lines
pybop/_problem.py	89.47%	2 Missing ⚠️
pybop/optimisers/pints_optimisers.py	80.00%	1 Missing ⚠️
pybop/optimisers/scipy_optimisers.py	90.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #213      +/-   ##
===========================================
+ Coverage    94.29%   94.34%   +0.04%     
===========================================
  Files           32       32              
  Lines         1614     1661      +47     
===========================================
+ Hits          1522     1567      +45     
- Misses          92       94       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[BradyPlanden]

Looks good, thanks for the addition @NicolaCourtier!

Just to confirm, have you checked the performance of the non-gradient based optimisers without bounds added? Bounds are optional from the Pints side, but it would be good to check that they don't affect our solutions.

It would also be good to remove the bounds from the IRPropMin and GradientDescent examples.

[NicolaCourtier]

Thanks for the comment @BradyPlanden - I've updated the two other examples as suggested.

What kind of performance tests are you thinking of for the non-gradient-based optimisers? If there are parameter values for which there is no solution, the optimiser can fail at the initial point but the likelihood of this is strongly dependent on the choice of prior.

[BradyPlanden]

Thanks - looks great!

I would consider updating the test_parameterisation/test_spm_optimisers test with one parameter that doesn't have bounds. That would give us continual check to ensure the lack of bounds on a parameter doesn't effect any current or future optimiser performance.

[NicolaCourtier]

Hi @BradyPlanden, this might be worth a second review. I've made the following changes in order to implement your suggestion to have one parameter without bounds in the parameterisation tests:

• add mean and sigma as properties to the non-Gaussian priors
• pass sigma from the prior through the base problem, cost and optimisation classes to become sigma0 for PINTS
• implement any missing bounds (if some bounds are given) as +/- np.inf
• move the setting of bounds to __init__ for the SciPy optimisers to match PINTS
• update the bounds check for PINTS PSO and SciPy Differential Evolution which require bounds which are all finite or None
• add set_bounds to Parameter to allow for consistent updating of bounds, upper and lower (in future, it may be good to pass bounds, sigma and x0 all as part of the parameters object - I think this is related to Add likelihood classes #210)
• add a default cost value to the min() in plot_convergence in case of unsuccessful iterations
• update test_optimisation and test_parameterisations to include tests without bounds
• add property checks to test_priors
• update the Changelog

[BradyPlanden]

Thanks for adding in the additional code for unbounded optimisation @NicolaCourtier! I have a few comments in areas that I think we can improve code comprehension. All in all it's looking very good!