Fix transformations#474
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #474 +/- ##
========================================
Coverage 99.04% 99.04%
========================================
Files 52 52
Lines 3545 3546 +1
========================================
+ Hits 3511 3512 +1
Misses 34 34 ☔ View full report in Codecov by Sentry. |
BradyPlanden
left a comment
There was a problem hiding this comment.
Thanks for the updates @NicolaCourtier! The transformation changes look good, I have a few comments on the change from applying transformations within the cost class to optimisation class.
|
I would recommend keeping properties, such as transformations, attached to a single base class for modularity (to reduce the chance of errors and unexpected behaviour). In this case, I think BaseOptimiser is conceptually the correct place for transformations. |
…__call_, updates PintsOptimiser.f
# Conflicts: # tests/unit/test_cost.py
Description
Move the cost evaluation error catches to
BaseOptimiser, addlog_update()and fix inconsistencies in the application of transformations.Issue reference
Fixes #473
Review
Before you mark your PR as ready for review, please ensure that you've considered the following:
Type of change
Key checklist:
$ 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)$ nox -s tests$ nox -s doctestYou can run integration tests, unit tests, and doctests together at once, using
$ nox -s quick.Further checks:
Thank you for contributing to our project! Your efforts help us to deliver great software.