Porosity change with particle size distributions

[DrSOKane]

Description

Porosity change due to SEI and lithium plating was previously done in terms of thickness. This had the advantage of being able to exclude the initial SEI thickness from the porosity change, but the drawback of rendering porosity change incompatible with particle size distributions.

By deleting the correction for initial SEI thickness, I was able to refactor the porosity change in terms of concentration. To achieve this, I modified the BaseSEI class so that the SEI concentration is always a size-averaged variable, as is already done with plating. This is physically justified because the SEI growth rate is a function of the surface potential difference, SEI thickness and various input parameters, none of which vary in the particle size domain.

Type of change

Removal of the correction for initial thickness makes this a breaking change.

Important checks:

Please confirm the following before marking the PR as ready for review:

• No style issues: nox -s pre-commit
• All tests pass: nox -s tests
• The documentation builds: nox -s doctests
• Code is commented for hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.24%. Comparing base (bee697c) to head (889ee8b).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5417      +/-   ##
==========================================
- Coverage   98.24%   98.24%   -0.01%     
==========================================
  Files         330      330              
  Lines       30072    30070       -2     
==========================================
- Hits        29544    29542       -2     
  Misses        528      528              

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

Refactors reaction-driven porosity change (SEI + lithium plating) to be compatible with particle-size distributions by expressing deposited volume in terms of concentrations and partial molar volumes, and updates SEI concentration handling accordingly.

Changes:

• Update reaction-driven porosity change to use SEI/plating concentrations (size-averaged) instead of thickness-based expressions.
• Adjust BaseSEI so SEI concentration is always size-averaged when a particle-size distribution is enabled.
• Remove option/test restrictions that previously rejected porosity-change + particle-size distribution combinations; document behavior changes in the changelog.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/pybamm/models/submodels/porosity/reaction_driven_porosity.py	Switch porosity change computation from thickness-based to concentration × partial-molar-volume.
src/pybamm/models/submodels/interface/sei/base_sei.py	Make SEI concentration size-averaged when size distributions are active.
src/pybamm/models/full_battery_models/base_battery_model.py	Remove option guards that blocked porosity-change with particle-size distributions.
tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_mpm.py	Remove tests asserting “not implemented” for distributions + porosity-change combinations.
tests/unit/test_models/test_full_battery_models/test_base_battery_model.py	Remove option-validation test that enforced an OptionError for distributions + plating porosity change.
CHANGELOG.md	Add feature/bugfix/breaking-change entries describing the new behavior.

Comments suppressed due to low confidence (2)

src/pybamm/models/submodels/porosity/reaction_driven_porosity.py:77

• delta_eps_k is added to epsilon_init, but SEI/plated/dead lithium occupy pore volume and should reduce porosity. Previously this term was negative (-a_k * L_tot); with concentrations it should likely subtract the deposited volume fraction (e.g., use a negative sign) so porosity decreases as c_sei_* / c_pl_* increase.

                    delta_eps_k += V_bar_sei * c_sei_tot_k + V_bar_Li * c_pl_tot_k

            domain_param = self.param.domain_params[domain.split()[0]]
            eps_k = domain_param.epsilon_init + delta_eps_k
            eps_dict[domain] = eps_k

tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_mpm.py:90

• This change removes the regression test that asserted plating+porosity-change is incompatible with particle-size distributions, but it doesn’t add a new test that positively verifies the newly supported combination works (e.g. building an MPM with particle size: distribution and lithium plating porosity change: true and checking well-posedness). Adding such a test would help prevent future regressions.

    def test_nonspherical_particle_not_implemented(self):
        options = {"particle shape": "user"}
        with pytest.raises(NotImplementedError):
            pybamm.lithium_ion.MPM(options)

    def test_msmr(self):
        options = {
            "open-circuit potential": "MSMR",
            "particle": "MSMR",
            "number of MSMR reactions": ("6", "4"),
            "intercalation kinetics": "MSMR",
        }
        model = pybamm.lithium_ion.MPM(options)
        model.check_well_posedness()

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The test that enforced an OptionError for particle size: distribution + lithium plating porosity change: true was removed, but there isn’t an added test confirming this configuration is now valid (e.g. model can be instantiated/built). Consider adding a positive test case here to lock in the new supported behavior.

Suggested change

	# valid combination of particle size distribution and lithium plating porosity change
	pybamm.BaseBatteryModel(
	{
	"particle size": "distribution",
	"lithium plating": "reversible",
	"lithium plating porosity change": "true",
	}
	)

[Copilot]

The module header / class docstring still describes porosity change “as a multiple of SEI/plating thicknesses”, but the implementation now uses concentrations and partial molar volumes. Consider updating those docstrings/comments to match the new approach to avoid misleading documentation.

[Copilot]

Lithium metal partial molar volume is already available on the parameter set (self.param.V_bar_Li) and is used that way elsewhere (e.g. lithium plating submodels). Creating a new pybamm.Parameter(...) here duplicates the symbol and makes the code less consistent; prefer reusing self.param.V_bar_Li.

Suggested change

	V_bar_Li = pybamm.Parameter(
	"Lithium metal partial molar volume [m3.mol-1]"
	)
	c_sei_k = variables[
	f"{Domain} {phase_name}SEI concentration [mol.m-3]"
	V_bar_Li = self.param.V_bar_Li
	c_sei_k = variables[
	f"{Domain} {phase_name}SEI concentration [mol.m-3]"
	f"{Domain} {phase_name}SEI concentration [mol.m-3]"