PR type: bugfix

Summary

We have observed some instability during model training, where occasionally the SGPR model loss is unexpectadly very poor. We have tracked the problem down to how the gpflow wrapper calls the scipy optimiser (using BFGS in our case).

The wrapper uses tf.Variables for the model hyperparameters and these are updated on every evaluation of the training loss. On those occasions where the BFGS line-search fails for an iteration, the scipy optimiser returns the previous good point that it found and sets appropriate flags in the OptimizeResult class. However, the model hyperparameters will have the value of the last evaluation of the failed line-search, which can be quite poor. One solution is for the caller to look at the returned result, realise that there was a fail and copy over the final point from OptimizeResult back to the model hyperparameters. However it's perhaps better for the wrapper to always copy the final scipy optimiser point to the model just before returning.

Proposed changes

• ...
• ...
• ...

What alternatives have you considered?

Minimal working example

# Put your example code in here

Release notes

Fully backwards compatible: yes / no

If not, why is it worth breaking backwards compatibility:

PR checklist

• New features: code is well-documented
  • detailed docstrings (API documentation)
  • notebook examples (usage demonstration)
• The bug case / new feature is covered by unit tests
• Code has type annotations
• Build checks
  • I ran the black isort formatter (make format)
  • I locally tested that the tests pass (make check-all)
• Release management
  • RELEASE.md updated with entry for this change
  • New contributors: I've added myself to CONTRIBUTORS.md