-
-
Notifications
You must be signed in to change notification settings - Fork 572
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
Issue 1143 equivalent circuit model #2478
Conversation
pybamm/models/submodels/electrolyte_conductivity/integrated_conductivity 2.py
Outdated
Show resolved
Hide resolved
tests/unit/test_parameters/test_parameter_sets/test_LCO_Ai2020 2.py
Outdated
Show resolved
Hide resolved
…com/pybamm-team/PyBaMM into issue-1143-equivalent-circuit-model
Codecov ReportBase: 99.72% // Head: 99.72% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #2478 +/- ##
=========================================
Coverage 99.72% 99.72%
=========================================
Files 257 268 +11
Lines 18961 19285 +324
=========================================
+ Hits 18908 19232 +324
Misses 53 53
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great Scott, thanks! Just a couple of minor points regarding the naming of the submodels. Happy to merge once the coverage tests pass.
pybamm/models/full_battery_models/equivalent_circuit/thevenin.py
Outdated
Show resolved
Hide resolved
pybamm/models/submodels/equivalent_circuit_elements/ocv_element.py
Outdated
Show resolved
Hide resolved
…com/pybamm-team/PyBaMM into issue-1143-equivalent-circuit-model
Description
Implementation of Theverin Equivalent Circuit Model (resistor + RC pair). Main things implemented:
If anyone has better ideas on where Equivalent circuit models and the associated submodel should be place, would be very helpful. There also is some hacky stuff (e.g. adding additional variables to the model, some additional options) that would also be nice to avoid
Additional modifications:
I some modifications to the 2D interpolation so that it now also used RegularGridInterpolator (as implemented for 3D) instead of interp2d. 2D and 3D data can now be read from csv files following the "C" array convention (slowest changing a variable in the first column). The previous .json method still passes it's test so is still compatible.
Fixes #1143
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.
Key checklist:
$ flake8
$ python run-tests.py --unit
$ cd docs
and then$ make clean; make html
You can run all three at once, using
$ python run-tests.py --quick
.Further checks: