Composite model for particle mechanics
Description
Implementing the composite model for particle mechanics: The phase in the current particle mechanics implementation is set to 'primary' by default. This is being updated to support independent phases (e.g., primary/secondary such as Gr/Si). To accommodate these changes, the associated files in full_battery_models, interface, SEI, and particle modules have been updated, along with modifications to the Chen2020 parameter set.
This is part of the work by Bonkile et al.
Fixes # (issue)
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.
- [x] 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) - [x] 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
@brosaplanella Unable to request specific reviewer in draft mode.
Hi Asher! The style tests are failing, this is because you have some defined variables/function that are not used. This is an issue it can't be automatically fixed by the pre-commit bot, so you will need to do it manually. You can find what's causing the issue if you check the failing test (https://github.com/pybamm-team/PyBaMM/actions/runs/11345431722/job/31553629641?pr=4516) and scroll down (you need to start reading at line 46).
Hi @brosaplanella, I've addressed the style issues, and now the remaining problem is with the unit tests. Although multiple tests are failing, they all stem from two types of errors:
- AttributeError: 'set' object has no attribute 'values'
- AttributeError: 'ReactionDriven' object has no attribute 'phase_name' Could you advise on how I should proceed with resolving these? Thank you!
Hi! The latter is because the ReactionDriven porosity model (reaction_driven_porosity.py) is not available for composite materials, that's why it breaks. I need to dig a bit more regarding the first issue.
BTW, it's better to not force push. I believe the issue you encountered is that the pre-commit bot made some style changes. In those cases you have to pull first and then push.
Hi! The latter is because the ReactionDriven porosity model (
reaction_driven_porosity.py) is not available for composite materials, that's why it breaks.
I made a PR that fixes this, which Sunil is already using (he requested it). @mohammedasher you can go to this branch of my fork of PyBaMM and merge the changes into your branch.
Thank you @DrSOKane; merging reaction_driven_porosity from your branch significantly reduced the number of failed tests. Now, the errors are:
- AttributeError: 'set' object has no attribute 'values'
- AttributeError: 'ReactionDriven' object has no attribute 'phase_name' Can this be re-scoped to self.phase_name??
- pybamm.expression_tree.exceptions.ModelError: Missing variable for submodel 'porosity': 'Negative total SEI thick...'
- Failed: DID NOT RAISE <class 'pybamm.expression_tree.exceptions.OptionError'>
@mohammedasher Are you still working on this?
@mohammedasher Are you still working on this?
@kratman Yes, I'm working on this...
@aabills does this overlap with what your did / are doing?
I think this is mostly closed by #4698, though its hard to tell if I got everything.
@mohammedasher Can you check and see if what you were intending to fix here works on pybamm 25.1.1?
@mohammedasher Can you check and see if what you were intending to fix here works on pybamm 25.1.1?
Hi @aabills, I think your work on #4698 addresses this PR i.e. to make particle mechanics work for composite electrodes; as a next step I wanted to include thermal model, LLI and LAM for composite electrodes. I see that the changes you have made should address the thermal part as well, can you please confirm?
Thank you.
Thermal models shouldn't be affected by the composite model because thermal model occurs at the electrode domain, while the composite model is at the particle domain, so temperature can just be broadcast to the composite parts of the models.
Thanks for your attempted contribution anyway! Sorry, we normally wouldn't fly through it when we have someone trying to contribute, but we had an urgent need to get this working.