Issue 1594 add O'Regan 2021 parameters

[brosaplanella]

Description

Added LG M50 parameter set from O'Regan 2021 (submitted).

Fixes #1594

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.

• New feature (non-breaking change which adds functionality)

Key checklist:

• No style issues: $ flake8
• All tests pass: $ python run-tests.py --unit
• The documentation builds: $ cd docs and then $ make clean; make html

You can run all three at once, using $ python run-tests.py --quick.

Further checks:

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[codecov]

Codecov Report

Merging #1598 (b1b4c86) into develop (00a9357) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##           develop    #1598    +/-   ##
=========================================
  Coverage    99.26%   99.26%            
=========================================
  Files          326      343    +17     
  Lines        18529    18677   +148     
=========================================
+ Hits         18392    18540   +148     
  Misses         137      137            

Impacted Files	Coverage Δ
...ls/LGM50_ORegan2021/aluminium_heat_capacity_CRC.py	100.00% <100.00%> (ø)
...cells/LGM50_ORegan2021/copper_heat_capacity_CRC.py	100.00% <100.00%> (ø)
...GM50_ORegan2021/copper_thermal_conductivity_CRC.py	100.00% <100.00%> (ø)
...Regan2021/graphite_LGM50_diffusivity_ORegan2021.py	100.00% <100.00%> (ø)
...electrolyte_exchange_current_density_ORegan2021.py	100.00% <100.00%> (ø)
...n2021/graphite_LGM50_entropic_change_ORegan2021.py	100.00% <100.00%> (ø)
...gan2021/graphite_LGM50_heat_capacity_ORegan2021.py	100.00% <100.00%> (ø)
...graphite_ORegan2021/graphite_LGM50_ocp_Chen2020.py	100.00% <100.00%> (ø)
.../graphite_LGM50_thermal_conductivity_ORegan2021.py	100.00% <100.00%> (ø)
...nmc_ORegan2021/nmc_LGM50_diffusivity_ORegan2021.py	100.00% <100.00%> (ø)
... and 25 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 00a9357...b1b4c86. Read the comment docs.

[rtimms]

great, thanks @brosaplanella !

[TomTranter]

As you need to change to quite a fine mesh for this set to give decent results it might be work printing a warning when using it or maybe include an example. Also I'm getting the kernel problem again where I needed to add empty inits to the parameter subfolders so I can run a script twice without restarting the kernel but I checked them all and can't see any missing. Very strange

[brosaplanella]

As you need to change to quite a fine mesh for this set to give decent results it might be work printing a warning when using it or maybe include an example.

I am happy either way, but the example will take a few minutes to run so that will make GitHub actions even slower. Where would it be best to include the warning?

Also I'm getting the kernel problem again where I needed to add empty inits to the parameter subfolders so I can run a script twice without restarting the kernel but I checked them all and can't see any missing. Very strange

Where do you get this? I mean, is it related to how the parameters are implemented or is it a separate issue?

[TomTranter]

It's my guess that it relates to the parameters as this is what it was last time. #1479 I think a notebook example would be best - maybe just run it for a few hundred seconds

[valentinsulzer]

If the parameter set is graphite + SiOx don't you need #1433 ?

[brosaplanella]

If the parameter set is graphite + SiOx don't you need #1433 ?

No, because the parameterization is for a DFN model with one component, so the parameters capture the complexity of the two phases. When #1433 is merged, we will need separate parameter sets (OCVs, diffusivities, etc) for each one independently.

[valentinsulzer]

Looks good but seems like this PR changes the test time from 1h to 2h, can we make it quicker? https://github.com/brosaplanella/PyBaMM/actions

[brosaplanella]

Looks good but seems like this PR changes the test time from 1h to 2h, can we make it quicker? https://github.com/brosaplanella/PyBaMM/actions

Is it from this PR? Because I have only added 2 tests (one is citation where the addition is negligible and the other runs in 6s in my laptop). For the notebook I can shorten the solving time (or even comment it out) but that was 20s in my laptop. Also, checking the Actions in this repo I have noticed that other tests took nearly 2h too (e.g. this one). Is there a way of knowing where the bottleneck is?

Update: I shortened the solving time in the example notebook anyway, but when I pushed the test time went back to normal for all tests (even unit tests). I also merged develop which I think included the big removal of tests. Anyway, it now runs in a reasonable amount of time.

[valentinsulzer]

@TomTranter needs to approve before we can merge