Skip to content

Composite release#2256

Merged
valentinsulzer merged 92 commits intodevelopfrom
composite-release-base
Aug 25, 2022
Merged

Composite release#2256
valentinsulzer merged 92 commits intodevelopfrom
composite-release-base

Conversation

@valentinsulzer
Copy link
Copy Markdown
Member

Description

Adds silicon/graphite composite model, using the two-phase changes from #2133
Adds current sigmoid OCP submodel

Fixes #1433
Fixes #2052

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

Simon O'Kane and others added 30 commits July 7, 2020 15:21
@review-notebook-app
Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@valentinsulzer valentinsulzer marked this pull request as draft August 24, 2022 07:15
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 24, 2022

Codecov Report

Merging #2256 (5a63f4c) into develop (4a8a221) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##           develop    #2256    +/-   ##
=========================================
  Coverage    99.38%   99.39%            
=========================================
  Files          359      365     +6     
  Lines        19821    20011   +190     
=========================================
+ Hits         19699    19889   +190     
  Misses         122      122            
Impacted Files Coverage Δ
...ectrolyte_exchange_current_density_Dualfoil1998.py 100.00% <ø> (ø)
...0_electrolyte_exchange_current_density_Chen2020.py 100.00% <ø> (ø)
..._electrolyte_exchange_current_density_Ecker2015.py 100.00% <ø> (ø)
...te_electrolyte_exchange_current_density_Kim2011.py 100.00% <ø> (ø)
...0_electrolyte_exchange_current_density_Chen2020.py 100.00% <ø> (ø)
...ectrolyte_exchange_current_density_Ramadass2004.py 100.00% <ø> (ø)
..._electrolyte_exchange_current_density_PeymanMPM.py 100.00% <ø> (ø)
...ectrolyte_exchange_current_density_Dualfoil1998.py 100.00% <ø> (ø)
...ctrolyte_exchange_current_density_kashkooli2017.py 100.00% <ø> (ø)
..._electrolyte_exchange_current_density_Ecker2015.py 100.00% <ø> (ø)
... and 19 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@valentinsulzer valentinsulzer marked this pull request as ready for review August 24, 2022 14:37
Copy link
Copy Markdown
Member

@brosaplanella brosaplanella left a comment

Choose a reason for hiding this comment

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

Looks good! The new models should be added to the docs though.

Comment thread examples/scripts/compare_lithium_ion_two_phase.py Outdated

return (
m_ref * arrhenius * c_e ** 0.5 * c_s_surf ** 0.5 * (c_s_max - c_s_surf) ** 0.5
m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How come some ** don't have spaces and some do? (note this applies to quite a few files)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I have no idea. Maybe we should have github actions run black on all pushes to avoid things like this (@priyanshuone6 can you set that up?)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is interesting that flake8 is not complaining though

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should black run on all the files or only the pushed commits?

@valentinsulzer
Copy link
Copy Markdown
Member Author

I've added the OCP models to the docs

@valentinsulzer valentinsulzer merged commit 0dbfa62 into develop Aug 25, 2022
@valentinsulzer valentinsulzer deleted the composite-release-base branch August 25, 2022 10:50
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.

Multiple phases in particle submodel Composite anode of graphite and silicon

5 participants