Datatype restructure for optimisation objects

[BradyPlanden]

I'm raising this commit to a PR as I believe there are enough changes to the backend to warrant a review at this stage. This started as an update to the plotting class to include alternative x-axis variables, but moved towards refactoring the pybop backend to make this possible.

This PR restructures PyBOP's datatypes for model output (y & dy). As such, knock-on changes to the problem and cost classes were required. The main benefits of these changes are to move away from numpy array objects where possible and replace them with dictionaries. This PR achieves the following:

• Order independent access from the dictionary. This benefit can be seen in the updates to the plotting classes.
• Clean multi-signal integration, as iteration over the problem.signal becomes the only requirement.
• A reduction in the datatype castings we needed within model.simulate and the corresponding cost function classes.
• Adds a problem.additional_variables object to capture variables needed within the design problem class, as well as plotting.
• Updates tests for the new datatypes
• Fixes the integration tests as the previous assertions where compared to x0 and not the groundtruth. They have been updated to sample from a random normal distribution to set the groundtruth, with x0 also randomly sampled too. This now represents the true optimiser performance.
• Updates SciPyMinimize default to be Nelder-Mead
• Updates initial point in cost landscape to be green circle
• Adds gradient cost landscape plots with optional arg

@NicolaCourtier @martinjrobins, there are quite a few changes in this one, and I would appreciate it if you both could take a good look at any potential bugs or improvements. I've spent a lot of time double-checking the cost function calculations, but it's very possible that I've missed something.

At the moment, the UKF tests are failing, I need to take a second look at them. @martinjrobins, if you can take a look as well that would be appreciated.

[martinjrobins]

Hi @BradyPlanden, I've had a quick look through and all seems good, happy with the change to dictionaries naming the y and dy outputs. I wasn't sure what default_variables was, but I don't really know how the design stuff works so that is probably why

[BradyPlanden]

This change was a bit worrisome, since the value should remain constant. I did a comparison with Pints' logic and it returned the updated value as well, pointing towards a bug in the previous RMSE calculation.

[codecov]

Codecov Report

Attention: Patch coverage is 93.60465% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 94.08%. Comparing base (ed2bf7c) to head (afd4990).

Files	Patch %	Lines
pybop/costs/fitting_costs.py	93.47%	3 Missing ⚠️
pybop/observers/observer.py	90.00%	3 Missing ⚠️
pybop/models/lithium_ion/echem_base.py	50.00%	2 Missing ⚠️
pybop/costs/design_costs.py	85.71%	1 Missing ⚠️
pybop/plotting/plot_cost2d.py	96.00%	1 Missing ⚠️
pybop/plotting/plot_problem.py	87.50%	1 Missing ⚠️

Additional details and impacted files

@@                      Coverage Diff                       @@
##           177b-plotting-capabilities     #224      +/-   ##
==============================================================
+ Coverage                       93.92%   94.08%   +0.15%     
==============================================================
  Files                              35       35              
  Lines                            1762     1826      +64     
==============================================================
+ Hits                             1655     1718      +63     
- Misses                            107      108       +1     

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

[NicolaCourtier]

I think this PR is heading in a very useful direction! Thanks @BradyPlanden. Here are some initial comments.

1. On the naming of additional_variables - can you make it clear what these variables are in addition to, in the description? Are they optional extras for plotting? Or perhaps required_variables, i.e. the variables that are required for the specific optimisation problem?

2. Do you think that Discharge capacity [A.h] will be required for all types of design problem? I would expect different requirements for different design costs. Could further variables be added as part of the Cost?

3. I'm also not sure why the passing of additional_variables is limited to the Base and EChem models, why not the PyBaMM ECM models for example? If this logic is required, please add a warning for times when the input is not being used.

4. In the cost functions, it is not clear to me why these two variables have been selected:

if key not in ["Time [s]", "Discharge capacity [A.h]"]:

and

if signal not in ["Time [s]", "Discharge capacity [A.h]"]

The original check was designed to cope with simulations that terminate early (which we expect in several situations such as reaching a voltage limit during the simulation). However, as written I think it is too general and could hide other, unexpected errors.

5. Please can you increase the coverage and check why one of the tests appears to be failing?

[BradyPlanden]

1. On the naming of additional_variables - can you make it clear what these variables are in addition to, in the description? Are they optional extras for plotting? Or perhaps required_variables, i.e. the variables that are required for the specific optimisation problem?

These variables are in addition to the variables selected in the signal object. At the moment we lack this functionality, so this provides the user with the ability to capture additional variables from the forward model prediction. In the case of plotting, the better way to do this would be to save the pybamm solution object and reuse it in plotting. That's probably worth a separate PR in the near future.

2. Do you think that Discharge capacity [A.h] will be required for all types of design problem? I would expect different requirements for different design costs. Could further variables be added as part of the Cost?

At the moment this is an uncharted area, this PR provides the mechanisms to explore this further in future PRs. The decision on the additional_variables is very much something we can do as it becomes clearer. In the case of Discharge Capacity [A.h], it is within the design cost base as both of the current child classes require it.

I'm also not sure why the passing of additional_variables is limited to the Base and EChem models, why not the PyBaMM ECM models for example? If this logic is required, please add a warning for times when the input is not being used.

Probably because our tests don't pick this up. Another good place to improve. Although it's not specifically passed to EChem models, it's only within BaseModel at the moment. This was missed in the shuffle.

4. In the cost functions, it is not clear to me why these two variables have been selected:

Added a few comments to make this more clear. Probably needs to be added to the docstring as well.

5. Please can you increase the coverage and check why one of the tests appears to be failing?

I need to do a double check on the failing windows examples, but I'm not confident it's isolated to this PR. I'm pretty sure we've been seeing it on the scheduled tests.

[BradyPlanden]

I believe the above issues have been addressed, I'm going to merge this into 177b and we can continue the discussion there.