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

Ecm with diffusion #4254

Merged
merged 28 commits into from
Aug 5, 2024
Merged

Conversation

MehrdadBabazadeh
Copy link
Contributor

@MehrdadBabazadeh MehrdadBabazadeh commented Jul 9, 2024

Description

The existing Equivalent Circuit Model (ECM) has been extended to create a more accurate model named the ECMD (ECM+Diffusion). This improvement involved adding a "diffusion_element" to the circuit alongside updates to the voltage_model and other relevant files. The key difference lies in how terminal voltage is calculated. The original ECM relied on average State of Charge (SoC). With the diffusion option enabled, the ECMD utilizes a newly calculated Surface SoC, providing a more precise representation. Additionally, a new term, eta_diffusion, is introduced. This term represents the voltage drop caused by diffusion limitations and is placed in series with the existing Open Circuit Voltage (OCV) within the circuit. This approach isolates the pure effect of diffusion on the voltage calculations. Finally, a Python script named Compare_ECM_ECMD.py is included as an example to demonstrate the comparison between the original ECM and the new ECMD.

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: $ pre-commit run (or $ nox -s pre-commit) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)
  • All tests pass: $ python run-tests.py --all (or $ nox -s tests)
  • The documentation builds: $ python run-tests.py --doctest (or $ nox -s doctests)

You can run integration tests, unit tests, and doctests together at once, using $ python run-tests.py --quick (or $ nox -s quick).

Further checks:

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

@valentinsulzer
Copy link
Member

Thanks! Is there any reference for this in the literature?

@MehrdadBabazadeh
Copy link
Contributor Author

Thanks! Is there any reference for this in the literature?

Hi. The used references are in the following. We have validated the results with an implementation of Chebyshev Collocation method developed by separate Python code:

  1. Fan, Chuanxin, et al. "Data-driven identification of lithium-ion batteries: A nonlinear equivalent circuit model with diffusion dynamics." Applied Energy 321 (2022): 119336.
  2. Binous, Housam, Abdullah A. Shaikh, and Ahmed Bellagi. "Chebyshev orthogonal collocation technique to solve transport phenomena problems with matlab® and mathematica." Computer Applications in Engineering Education 23.3 (2015): 422-431.

The address for the developed Python code, used to validate SoC profile is:
https://github.com/MehrdadBabazadeh/Chebyshev-Collocation.git

Copy link

codecov bot commented Jul 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.46%. Comparing base (109b9c4) to head (fde2343).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4254      +/-   ##
===========================================
- Coverage    99.50%   99.46%   -0.05%     
===========================================
  Files          288      289       +1     
  Lines        22079    22146      +67     
===========================================
+ Hits         21970    22027      +57     
- Misses         109      119      +10     

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

@valentinsulzer
Copy link
Member

@MehrdadBabazadeh thanks for sharing the references, can you document these as described here? https://github.com/pybamm-team/PyBaMM/blob/develop/CONTRIBUTING.md#citations

Also, you can link that git repository in the docstring of your submodel. These references will be useful in future if someone wants to know more about the model

@MehrdadBabazadeh MehrdadBabazadeh requested a review from a team as a code owner July 15, 2024 13:27
Copy link
Contributor

@kratman kratman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last thing now that #4266 was created to address set_events being misnamed.

@kratman kratman dismissed their stale review July 15, 2024 13:49

Changes were made

Copy link
Contributor

@kratman kratman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems OK now as long as the tests pass

@kratman
Copy link
Contributor

kratman commented Aug 2, 2024

@MehrdadBabazadeh Are you still working on the coverage for this?

@MehrdadBabazadeh
Copy link
Contributor Author

Yes. Ferran and me will go on the remaining issues and continue the work next week.

@brosaplanella
Copy link
Sponsor Member

The coverage issues seem to be unrelated to this PR, so that should be good to go. Waiting to merge as I don't know if we need to wait for #4311

@arjxn-py
Copy link
Member

arjxn-py commented Aug 5, 2024

I don't know if we need to wait for #4311

@brosaplanella, we can merge this one without waiting for #4311. Coverage failure seems unrelated to me too as giving failure in pybamm/util.py which is failing for old changes

@brosaplanella brosaplanella merged commit 2d3db21 into pybamm-team:develop Aug 5, 2024
25 of 26 checks passed
@brosaplanella
Copy link
Sponsor Member

@all-contributors please add @MehrdadBabazadeh for code and tests

Copy link
Contributor

@brosaplanella

I've put up a pull request to add @MehrdadBabazadeh! 🎉

@MehrdadBabazadeh
Copy link
Contributor Author

Thanks to @brosaplanella and other team members!

js1tr3 pushed a commit to js1tr3/PyBaMM that referenced this pull request Aug 12, 2024
* add diffusion element for ECM

* fix bugs

* An example to compare ECM with ECMD model

* style: pre-commit fixes

* We removed the example to compare two models.

* Added citation.

* Fixed typo.

* Added entry to change log.

* style: pre-commit fixes

* Added test

* style: pre-commit fixes

* Fixed citation error.

* Changed line 39.

* New test_default_properties definition added.

* style: pre-commit fixes

* Increased cover

* style: pre-commit fixes

---------

Co-authored-by: Ferran Brosa Planella <Ferran.Brosa-Planella@warwick.ac.uk>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com>
Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants