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

Allow multiple datasets & models at once #238

Open
brosaplanella opened this issue Mar 14, 2024 · 6 comments
Open

Allow multiple datasets & models at once #238

brosaplanella opened this issue Mar 14, 2024 · 6 comments
Assignees
Labels
enhancement New feature or request

Comments

@brosaplanella
Copy link
Contributor

brosaplanella commented Mar 14, 2024

Feature description

Allow the FittingProblem class to take lists of datasets and of models, which would streamline certain processes. The key idea is that when multiple models are present, they all use the same parameters (otherwise one can just write a for loop with N FittingProblems).

Cases:

  • 1 model & 1 dataset: current case, just fit the model to the dataset.
  • 1 model & N datasets: fit the model to minimise the cost function across datasets. A practical case for this would be when you run the same test on 3 different cells and want to fit the parameters.
  • N models & N datasets: fit first model to first dataset, second to second and so on. Given that models are for specific operating conditions, an example of this would be if we wanted to fit a model to multiple C-rate experiments.
  • N models & 1 dataset: I am still not convinced that this one would have any practical application, but it is the missing one so just listing it here.

Motivation

Provided for each of the cases above.

Possible implementation

Loop over and concatenate the error arrays before passing to cost function.

Additional context

Here is the equivalent issue in pybamm-param (paramm-team/pybamm-param#38), for reference (we didn't do any progress on it). This issue arises from discussion in the meeting at Imperial on 13/03/24.

@brosaplanella brosaplanella added the enhancement New feature or request label Mar 14, 2024
@BradyPlanden
Copy link
Member

Excellent thanks @brosaplanella, it looks like we were thinking along the same lines (#232). Happy to leave this one open as it has more background info.

To note, adding a weighting function to enable cost scaling across the datasets should be added at the same time as this functionality.

@BradyPlanden
Copy link
Member

Hi @MarkBlyth, just checking in to see how you are getting on with issue. If you are running into issues, or don't have the time to continue please let me know so that I can support or reassign.

@MarkBlyth
Copy link
Contributor

Hi Brady, I've been out the office for most of the past month so haven't had the chance to look at this yet. Still keen to work on it, and aiming to get started by the end of next week.

@MarkBlyth
Copy link
Contributor

Hi all, I've made a first pass, with code here. Currently it will break the observer methods, hasn't been tested with multiple models, and has no unit tests, so it's very much work in progress.

The approach I've gone for is to make a DatasetCollection class, representing multiple datasets, and have the fitting_problem evaluate multiple models over multiple datasets, with the plotting and cost functions updated to accept multiple results at once. Given that this means making a large number of breaking changes to the code base, I was keen to get some feedback before I keep going with tests etc.

@NicolaCourtier
Copy link
Member

Sounds good, thanks @MarkBlyth! It would helpful at this stage to make a (draft) pull request so we can more easily see where you have made changes.

@MarkBlyth MarkBlyth mentioned this issue May 1, 2024
13 tasks
@BradyPlanden BradyPlanden moved this from Todo to In Progress in v24.6 May 8, 2024
@NicolaCourtier
Copy link
Member

Hi @brosaplanella and @MarkBlyth,

I've made an example MultiFittingProblem class to tackle this open issue. It would be great if you could take a look at the draft PR #364. It's not a robust method yet, but I wanted to demonstrate the direction I think we should go in (similar to #329).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants