Skip to content

Fix PotentialPair1plus1D/2plus1D normalisation for parallel electrodes (#5467)#5478

Open
kamal20122012 wants to merge 2 commits intopybamm-team:mainfrom
kamal20122012:fix/potential-pair-parallel-electrodes-5467
Open

Fix PotentialPair1plus1D/2plus1D normalisation for parallel electrodes (#5467)#5478
kamal20122012 wants to merge 2 commits intopybamm-team:mainfrom
kamal20122012:fix/potential-pair-parallel-electrodes-5467

Conversation

@kamal20122012
Copy link
Copy Markdown

Summary

  • The 1+1D and 2+1D PotentialPair current-collector submodels apply a Neumann BC at the positive tab of a single modelled electrode pair, but the boundary condition was computed from the total cell current (applied_current_density * A_cc) rather than the per-electrode-pair current. Because A_cc already includes n_electrodes_parallel, the gradient of phi_s_cp at the tab was overstated by that factor whenever more than one electrode pair was connected in parallel — voltage drops in the current collector were inflated, the lower voltage cutoff was hit prematurely, and the reported discharge capacity was approximately 1 / n_electrodes_parallel of the correct value.
  • Fix: divide by n_electrodes_parallel so the BC uses the current per modelled electrode pair. No effect for the default n=1 case.
  • Added an integration regression test that compares discharge capacity between an equivalent wide single-electrode configuration and a two-parallel-electrode configuration with the same total current-collector area; capacities now match.

Fixes #5467

Test plan

  • Reproduction script from the issue: parallel config now reports 40.84 A·h (matches wide single-electrode case) instead of 22.36 A·h.
  • pytest tests/integration/test_models/test_full_battery_models/test_lithium_ion/test_dfn.py::TestDFN::test_potential_pair_1plus1D_parallel_electrodes (passes; verified to fail on pre-fix code)
  • pytest tests/unit/test_models/test_full_battery_models/test_lithium_ion -k "1plus1D or 2plus1D or potential_pair" — 50 passed
  • pre-commit run --files <changed files> — all hooks pass (ruff-check, ruff-format, blacken-docs, file-checks, sp-repo-review)

The 1+1D and 2+1D potential-pair current-collector submodels apply a
Neumann boundary condition at the positive tab of a single modelled
electrode pair. The boundary condition was computed from the total cell
current (applied_current_density * A_cc) instead of the per-electrode
current. Because A_cc already includes the
"Number of electrodes connected in parallel to make a cell" factor, this
overstated the gradient of phi_s_cp at the tab by that factor when more
than one electrode pair was connected in parallel: voltage drops in the
current collector were inflated, the lower voltage cutoff was hit
prematurely, and the reported discharge capacity was approximately
1/n_electrodes_parallel of the correct value.

Divide by n_electrodes_parallel so the BC uses the current per
electrode pair. No effect for the default n=1 case.

Adds an integration regression test that compares discharge capacity
between an equivalent wide single-electrode configuration and a
two-parallel-electrode configuration; capacities now match.

Fixes pybamm-team#5467
@kamal20122012 kamal20122012 requested a review from a team as a code owner April 30, 2026 07:37
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.15%. Comparing base (19c2d57) to head (8db311f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5478   +/-   ##
=======================================
  Coverage   98.15%   98.15%           
=======================================
  Files         338      338           
  Lines       31115    31122    +7     
=======================================
+ Hits        30542    30549    +7     
  Misses        573      573           

☔ 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.

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.

[Bug]: PotentialPair1plus1D not normalising correctly parallel cells

1 participant