-
Notifications
You must be signed in to change notification settings - Fork 24
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
Add MultiFittingProblem class and example #364
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #364 +/- ##
===========================================
- Coverage 98.68% 98.67% -0.02%
===========================================
Files 46 47 +1
Lines 2971 3015 +44
===========================================
+ Hits 2932 2975 +43
- Misses 39 40 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good, nice work! See my comment on the parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this @NicolaCourtier. I think it's coming along well, I've made a few comments for you to look at. As a more general thought, do you see any reason this couldn't be a MultiProblem
class that composes multiple instances ofFittingProblem
or DesignProblem
? Perhaps even combined instances of both (with an update to BaseOptimiser for minimising
logic. This seems to be the logical next step, but we could do it now instead of refactoring this class later.
Merge after #451 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - thanks for the addition @NicolaCourtier!
Description
Add a new Problem class that accepts multiple fitting problems and returns one combined problem for passing to a cost function. This PR builds on and should be merged after #359.
Issue reference
Fixes part of #238.
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 doctest
You 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.