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

#238 multiple datasets #311

Draft
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

MarkBlyth
Copy link
Contributor

@MarkBlyth MarkBlyth commented May 1, 2024

Description

Towards implementing multiple models and multiple datasets. fitting_problem now evaluates to give a list of solutions, one per model/dataset pair. Cost functions weight each error-set to get an overall error score, which is then optimised against. Draft PR, not fully tested and with open todos.

Issue reference

#238

Type of change

  • [ x ] 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:

  • [x ] 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.

@BradyPlanden
Copy link
Member

Hi @MarkBlyth, thanks for this! I'll try to take a look through this afternoon or Monday morning 👍

@NicolaCourtier
Copy link
Member

Hi @MarkBlyth, thanks for making a start on this issue! From a first look, the additions required to loop over datasets and/or models seem quite major and worth some discussion. I'm wondering how much of the work can be done outside the base classes. I'm interested to know what @BradyPlanden thinks.

A couple of suggestions for next steps if helpful:

  1. Write an example use case for multiple datasets and write a script demonstrating the intended workflow. Consider how fitting multiple datasets differs from fitting multiple signals.

  2. Start from the other end and try to optimise the sum of two cost objects. For example, create a new wrapper cost class that accepts a list of costs and associated weights and returns a new cost object? We already have one cost that takes another cost as input (MAP takes a likelihood, which is also a cost) so this makes me think it may be possible. Weighted costs would be useful for both fitting and design optimisation problems that consider multiple simulations. Tackling this issue may help us understand the current one.

Thanks again for sharing your progress!

@NicolaCourtier
Copy link
Member

Hi @MarkBlyth, I made a start on developing a cost wrapper for multiple cost objects to demonstrate what I meant in point (2) above - see issue #327 and the draft PR #329. I'd be happy to discuss pros/cons of different approaches.

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.

Hi @MarkBlyth, would you be interested in recreating this functionality with the new MultiFittingProblem and WeightedCost?

@MarkBlyth
Copy link
Contributor Author

@NicolaCourtier yep sounds good! I won't be able to put too much time into it until after the FI conference, but I'm keen to do some work on it.

@MarkBlyth MarkBlyth self-assigned this Aug 28, 2024
@MarkBlyth MarkBlyth removed the request for review from BradyPlanden August 28, 2024 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants