Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates to #236 to avoid breaking change to pybop.Optimisation #309

Merged
merged 13 commits into from
May 22, 2024

Conversation

BradyPlanden
Copy link
Member

@BradyPlanden BradyPlanden commented Apr 26, 2024

Description

This PR applies changes to #236 so that v24.5 does not have breaking changes in the pybop.Optimisation API.

Issue reference

Fixes # N/A

Review

Before you mark your PR as ready for review, please ensure that you've considered the following:

  • Updated the CHANGELOG.md in reverse chronological order (newest at the top) with a concise description of the changes, including the PR number.
  • Noted any breaking changes, including details on how it might impact existing functionality.

Type of change

  • New Feature: A non-breaking change that adds new functionality.
  • Optimization: A code change that improves performance.
  • Examples: A change to existing or additional examples.
  • Bug Fix: A non-breaking change that addresses an issue.
  • Documentation: Updates to documentation or new documentation for new features.
  • Refactoring: Non-functional changes that improve the codebase.
  • Style: Non-functional changes related to code style (formatting, naming, etc).
  • Testing: Additional tests to improve coverage or confirm functionality.
  • Other: (Insert description of change)

Key checklist:

  • No style issues: $ pre-commit run (or $ nox -s pre-commit) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)
  • All unit tests pass: $ nox -s tests
  • The documentation builds: $ nox -s doctest

You can run integration tests, unit tests, and doctests together at once, using $ nox -s quick.

Further checks:

  • Code is well-commented, especially in complex or unclear areas.
  • Added tests that prove my fix is effective or that my feature works.
  • Checked that coverage remains or improves, and added tests if necessary to maintain or increase coverage.

Thank you for contributing to our project! Your efforts help us to deliver great software.

…sation APIs, updts where required, moves _optimisation to optimisers/
Copy link

codecov bot commented Apr 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.98%. Comparing base (afaf69e) to head (e4ca965).

Additional details and impacted files
@@                 Coverage Diff                  @@
##           236-scipy-kwargs     #309      +/-   ##
====================================================
+ Coverage             96.81%   96.98%   +0.16%     
====================================================
  Files                    40       41       +1     
  Lines                  2201     2222      +21     
====================================================
+ Hits                   2131     2155      +24     
+ Misses                   70       67       -3     

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

examples/notebooks/optimiser_interface.ipynb Show resolved Hide resolved
examples/scripts/spm_pso.py Outdated Show resolved Hide resolved
pybop/optimisers/_optimisation.py Outdated Show resolved Hide resolved
@NicolaCourtier NicolaCourtier added the blocker An issue which must be resolved before work can resume on other issues. label Apr 27, 2024
@BradyPlanden
Copy link
Member Author

Ugh, the recent file rename messed with the number of changes. Happy to revert it to help with review, it's needed to remove circular import issue in the Optimisation class. @NicolaCourtier

@NicolaCourtier
Copy link
Member

Yes please do revert it @BradyPlanden, let's change the filenames at the end once both PRs are ready.

CHANGELOG.md Outdated Show resolved Hide resolved
examples/notebooks/optimiser_interface.ipynb Show resolved Hide resolved
examples/notebooks/optimiser_interface.ipynb Outdated Show resolved Hide resolved
examples/notebooks/optimiser_interface.ipynb Outdated Show resolved Hide resolved
examples/notebooks/optimiser_interface.ipynb Outdated Show resolved Hide resolved
examples/notebooks/optimiser_interface.ipynb Outdated Show resolved Hide resolved
tests/integration/test_parameterisations.py Outdated Show resolved Hide resolved
tests/integration/test_parameterisations.py Outdated Show resolved Hide resolved
Copy link
Member

@NicolaCourtier NicolaCourtier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @BradyPlanden for the addition of the Optimisation wrapper!

@NicolaCourtier NicolaCourtier merged commit dec74d7 into 236-scipy-kwargs May 22, 2024
29 checks passed
@NicolaCourtier NicolaCourtier deleted the 236b-scipy-kwargs branch May 22, 2024 15:45
NicolaCourtier added a commit that referenced this pull request May 22, 2024
* Enable passing of kwargs to SciPy

* Update checks on bounds

* Add update_options and error for Pints

* Rename opt to optim

* Remove stray comments

* Add BaseSciPyOptimiser and BasePintsOptimiser

* Align optimiser option setting

* Update notebooks with kwargs

* Update scripts with kwargs

* Update notebooks

* Align optimisers with Optimisation as base class

* Update stopping criteria in spm_NelderMead.py

Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>

* Update stopping criteria in spm_adam.py

Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>

* Update sigma0 in spm_descent.py

Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>

* Update GradientDescent

* Change update to set and check pints_method

* Update test_optimisation_options

* Update notebooks

* Update set learning rate

* Pop threshold

* Fix bug in model.simulate

* Update notebooks

* Update test_models.py

* Store SciPy result

* Update x0 input and add tests

* Update bounds to avoid x0 outside

* Re-initialise pints_method on certain options

* Update x0_new test

* Update test_optimisation.py

* Create initialise_method for PINTS optimisers

* Align optimisation result

* Update checks on bounds

* Apply suggestions

Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>

* Add standalone optimiser

* Simplify optimiser set-up and align _minimising

* Update option setting in notebooks

* Take abs of cost0

* Implement suggestions from Brady

* Update tests and base option setting

* Update test_invalid_cost

* Increase coverage

* Sort out notebook changes

* Reset scale parameter

* Move settings into arguments

* Update comments

* Update optimiser call

* Move check on jac

* Add assertions

* Add maxiter to test

* Add assertion

* Update to lambda functions

Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>

* Update comment

Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>

* Update to list comprehension

Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>

* Formatting

* Revert "Update to lambda functions"

This reverts commit aa73bff.

* Move minimising out of costs

* Update description

* Updates to #236 to avoid breaking change to `pybop.Optimisation` (#309)

* Splits Optimisation -> BaseOptimiser/Optimisation, enables two optimisation APIs, updts where required, moves _optimisation to optimisers/

* increase coverage

* Pass optimiser_kwargs though run()

* updt examples

* Converts DefaultOptimiser -> Optimisation

* split Optimisation and BaseOptimsier classes, loosen standalone cost unit test

* add incorrect attr test

* fix: updt changelog entry, optimsation_interface notebook, review suggestions

* fix: updt notebook state

* Updt assertions, optimisation object name

---------

Co-authored-by: NicolaCourtier <45851982+NicolaCourtier@users.noreply.github.com>

* Rename method to pints_optimiser

* Rename base_optimiser to pints_base_optimiser

* Rename _optimisation to base_optimiser

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>
BradyPlanden added a commit that referenced this pull request Jun 3, 2024
* Enable passing of kwargs to SciPy

* Update checks on bounds

* Add update_options and error for Pints

* Rename opt to optim

* Remove stray comments

* Add BaseSciPyOptimiser and BasePintsOptimiser

* Align optimiser option setting

* Update notebooks with kwargs

* Update scripts with kwargs

* Update notebooks

* Align optimisers with Optimisation as base class

* Update stopping criteria in spm_NelderMead.py

Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>

* Update stopping criteria in spm_adam.py

Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>

* Update sigma0 in spm_descent.py

Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>

* Update GradientDescent

* Change update to set and check pints_method

* Update test_optimisation_options

* Update notebooks

* Update set learning rate

* Pop threshold

* Fix bug in model.simulate

* Update notebooks

* Update test_models.py

* Store SciPy result

* Update x0 input and add tests

* Update bounds to avoid x0 outside

* Re-initialise pints_method on certain options

* Update x0_new test

* Update test_optimisation.py

* Create initialise_method for PINTS optimisers

* Align optimisation result

* Update checks on bounds

* Apply suggestions

Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>

* Add standalone optimiser

* Simplify optimiser set-up and align _minimising

* Update option setting in notebooks

* Take abs of cost0

* Implement suggestions from Brady

* Update tests and base option setting

* Update test_invalid_cost

* Increase coverage

* Sort out notebook changes

* Reset scale parameter

* Move settings into arguments

* Update comments

* Update optimiser call

* Move check on jac

* Add assertions

* Add maxiter to test

* Add assertion

* Update to lambda functions

Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>

* Update comment

Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>

* Update to list comprehension

Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>

* Formatting

* Revert "Update to lambda functions"

This reverts commit aa73bff.

* Move minimising out of costs

* Update description

* Updates to #236 to avoid breaking change to `pybop.Optimisation` (#309)

* Splits Optimisation -> BaseOptimiser/Optimisation, enables two optimisation APIs, updts where required, moves _optimisation to optimisers/

* increase coverage

* Pass optimiser_kwargs though run()

* updt examples

* Converts DefaultOptimiser -> Optimisation

* split Optimisation and BaseOptimsier classes, loosen standalone cost unit test

* add incorrect attr test

* fix: updt changelog entry, optimsation_interface notebook, review suggestions

* fix: updt notebook state

* Updt assertions, optimisation object name

---------

Co-authored-by: NicolaCourtier <45851982+NicolaCourtier@users.noreply.github.com>

* Rename method to pints_optimiser

* Rename base_optimiser to pints_base_optimiser

* Rename _optimisation to base_optimiser

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker An issue which must be resolved before work can resume on other issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants