Issue 573 implement LGM50 #854

brosaplanella · 2020-02-24T12:05:59Z

Description

Addition of the LGM50 parameter set from

Chang-Hui Chen, Ferran Brosa Planella, Kieran O’Regan, Dominika Gastol, W. Dhammika Widanage, and Emma Kendrick. "Development of Experimental Techniques for Parameterization of Multi-scale Lithium-ion Battery Models." Submitted for publication (2020).

Fixes #573

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)
Optimization (back-end change that speeds up the code)
Bug fix (non-breaking change which fixes an issue)

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

…brosa/PyBaMM into issue-573-implement-LGM50

… not converge

…brosa/PyBaMM into issue-573-implement-LGM50

codecov · 2020-02-25T12:44:19Z

Codecov Report

❗ No coverage uploaded for pull request base (master@d34816f). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master     #854   +/-   ##
=========================================
  Coverage          ?   98.24%           
=========================================
  Files             ?      180           
  Lines             ?    10277           
  Branches          ?        0           
=========================================
  Hits              ?    10097           
  Misses            ?      180           
  Partials          ?        0

Impacted Files	Coverage Δ
...bamm/parameters/standard_parameters_lithium_ion.py	`100% <ø> (ø)`
pybamm/parameters/parameter_sets.py	`100% <100%> (ø)`

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 d34816f...f62efc8. Read the comment docs.

rtimms

looks great thanks @ferranbrosa ! just a few minor comments

rtimms · 2020-02-25T17:00:29Z

...input/parameters/lithium-ion/anodes/graphite_Chen2020/graphite_LGM50_diffusivity_Chen2020.py

+      diffusivity is taken to be a constant. The value is taken from [1].
+      References
+      ----------
+      .. [1] Work in progress


is this the paper in the readme? info could be added here if so

rtimms · 2020-02-25T17:00:46Z

...rs/lithium-ion/anodes/graphite_Chen2020/graphite_LGM50_electrolyte_reaction_rate_Chen2020.py

+    Reaction rate for Butler-Volmer reactions between graphite and LiPF6 in EC:DMC.
+    References
+    ----------
+    .. [1] Work in progress


rtimms · 2020-02-25T17:03:14Z

...t/parameters/lithium-ion/anodes/graphite_Chen2020/graphite_LGM50_entropic_change_Chen2020.py

+        + (0.0155 / (0.029 * c_n_max)) * ((cosh((sto - 0.105) / 0.029)) ** (-2))
+    )
+
+    return du_dT * 0


you should just be able to return 0*sto (and remove the unnecessary function for du_dT), or alternatively just set this to be zero in the .csv instead of a [function] and it should work too. the latter is probably cleaner

rtimms · 2020-02-25T17:05:30Z

pybamm/parameters/parameter_sets.py

+    "cathode": "nmc_Chen2020",
+    "electrolyte": "lipf6_Nyman2008",
+    "experiment": "1C_discharge_from_full_Chen2020",
+}


take a look at the latest master (see PR #818)-- you can add a reference here so people know to cite your paper if they use your parameter set

I talked to @tinosulzer and he said he would do so after the PR is merged.

codecov · 2020-02-25T17:11:23Z

Codecov Report

❗ No coverage uploaded for pull request base (master@d34816f). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master     #854   +/-   ##
=========================================
  Coverage          ?   98.24%           
=========================================
  Files             ?      180           
  Lines             ?    10277           
  Branches          ?        0           
=========================================
  Hits              ?    10097           
  Misses            ?      180           
  Partials          ?        0

Impacted Files	Coverage Δ
...bamm/parameters/standard_parameters_lithium_ion.py	`100% <ø> (ø)`
pybamm/parameters/parameter_sets.py	`100% <100%> (ø)`

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 d34816f...f62efc8. Read the comment docs.

codecov · 2020-02-25T17:11:49Z

Codecov Report

❗ No coverage uploaded for pull request base (master@d34816f). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master     #854   +/-   ##
=========================================
  Coverage          ?   98.24%           
=========================================
  Files             ?      180           
  Lines             ?    10277           
  Branches          ?        0           
=========================================
  Hits              ?    10097           
  Misses            ?      180           
  Partials          ?        0

Impacted Files	Coverage Δ
...bamm/parameters/standard_parameters_lithium_ion.py	`100% <ø> (ø)`
pybamm/parameters/parameter_sets.py	`100% <100%> (ø)`

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 d34816f...f62efc8. Read the comment docs.

valentinsulzer · 2020-02-25T19:07:59Z

Thanks @ferranbrosa , looks great! Merging

brosaplanella added 30 commits February 2, 2020 19:28

pybamm-team#573 update final LGM50 parameters

26621ab

pybamm-team#573 included Chen2020 as a PyBaMM parameter set

1311b65

pybamm-team#573 update final LGM50 parameters

63006d5

pybamm-team#573 included Chen2020 as a PyBaMM parameter set

8594e56

Merge branch 'issue-573-implement-LGM50' of https://github.com/ferran…

451e973

…brosa/PyBaMM into issue-573-implement-LGM50

pybamm-team#573 update final LGM50 parameters

9e8309f

pybamm-team#573 included Chen2020 as a PyBaMM parameter set

b0f0fec

Merge branch 'issue-573-implement-LGM50' of https://github.com/ferran…

8c623d7

…brosa/PyBaMM into issue-573-implement-LGM50

updated .gitignore to only omit data

73e2c8e

pybamm-team#573 setup test as experiment, still fails to converge

de69496

pybamm-team#573 changed reference OCV anode otherwise the solver does…

16ccdb9

… not converge

pybamm-team#573 update test files to check parameters work

1383600

pybamm-team#573 updated comparison with experiments

78c4717

issue-573 data folder in the root directory

7887ea9

update according to Emma's feedback

8cacf9f

update final comments paper

d2dc2b0

changed label to half cell

b0ba975

Merge remote-tracking branch 'upstream/master'

063e1b9

Merge remote-tracking branch 'upstream/master'

d0825be

pybamm-team#573 update final LGM50 parameters

1b9ee46

pybamm-team#573 included Chen2020 as a PyBaMM parameter set

84fd3ca

updated .gitignore to only omit data

bcf4f36

pybamm-team#573 setup test as experiment, still fails to converge

cd293a9

pybamm-team#573 changed reference OCV anode otherwise the solver does…

9e454c9

… not converge

pybamm-team#573 update test files to check parameters work

98ae683

pybamm-team#573 updated comparison with experiments

580d979

issue-573 data folder in the root directory

dd6c5ed

update according to Emma's feedback

567095e

update final comments paper

fdc33e0

changed label to half cell

135543f

brosaplanella added 12 commits February 25, 2020 12:41

update final comments paper

691ebd5

updated anode parameters to 1C tuned

94dbdff

updated README files

555b6eb

added Chen2020 as a parameter set

2127dd7

removed results file

4a5a17e

parameters for 1C tuning

1d0b9d5

added LGM50 parameter test

c75b6fb

flake8

f092242

flake8

c9d871c

added contact details README

b43c721

added LGM50 parameters

c317a43

Merge branch 'issue-573-implement-LGM50' of https://github.com/ferran…

c99d487

…brosa/PyBaMM into issue-573-implement-LGM50

brosaplanella added 5 commits February 25, 2020 12:52

pybamm-team#573 removed the old input folder

f17ffb9

pybamm-team#573 added Chen2020 to new input directory

4e1bc74

flake8

f6dd257

fixed for the new input directory

249f46d

fix test name

f62efc8

rtimms requested changes Feb 25, 2020

View reviewed changes

rtimms approved these changes Feb 25, 2020

View reviewed changes

brosaplanella added 5 commits February 25, 2020 17:47

pybamm-team#573 remove the dUdT functions as they returned 0

6bc0eef

fixed path to parameters .gitignore

06fced3

updated parameters_cli test for Chen2020

29d3491

added reference Chen2020

92cff6c

added csv files

bed5fb2

brosaplanella requested a review from rtimms February 25, 2020 18:10

valentinsulzer merged commit d35e94b into pybamm-team:master Feb 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 573 implement LGM50 #854

Issue 573 implement LGM50 #854

brosaplanella commented Feb 24, 2020

codecov bot commented Feb 25, 2020 •

edited

rtimms left a comment

rtimms Feb 25, 2020

brosaplanella Feb 25, 2020

rtimms Feb 25, 2020

rtimms Feb 25, 2020

rtimms Feb 25, 2020

brosaplanella Feb 25, 2020

codecov bot commented Feb 25, 2020

codecov bot commented Feb 25, 2020

valentinsulzer commented Feb 25, 2020

Issue 573 implement LGM50 #854

Issue 573 implement LGM50 #854

Conversation

brosaplanella commented Feb 24, 2020

Description

Type of change

Key checklist:

Further checks:

codecov bot commented Feb 25, 2020 • edited

Codecov Report

rtimms left a comment

Choose a reason for hiding this comment

rtimms Feb 25, 2020

Choose a reason for hiding this comment

brosaplanella Feb 25, 2020

Choose a reason for hiding this comment

rtimms Feb 25, 2020

Choose a reason for hiding this comment

rtimms Feb 25, 2020

Choose a reason for hiding this comment

rtimms Feb 25, 2020

Choose a reason for hiding this comment

brosaplanella Feb 25, 2020

Choose a reason for hiding this comment

codecov bot commented Feb 25, 2020

Codecov Report

codecov bot commented Feb 25, 2020

Codecov Report

valentinsulzer commented Feb 25, 2020

codecov bot commented Feb 25, 2020 •

edited