Make "Contact resistance [Ohm]" a function of temperature#5604
Merged
rtimms merged 6 commits intoJun 16, 2026
Conversation
Convert the scalar "Contact resistance [Ohm]" parameter into a FunctionParameter of the volume-averaged cell temperature, which is available for all thermal options. Existing scalar parameter values keep working unchanged (auto-wrapped as constants). Also reframe the documentation: despite the legacy name, this is a lumped series resistance rather than a true contact resistance (FaradayInstitution/BPX#130). The name is kept to avoid a breaking change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reword the docstrings and notebook to explain that "Contact resistance [Ohm]" really represents the total series resistance of the cell (contact resistances between layers, tab/weld/lead resistances), keeping the parameter name. Drop the BPX link and breaking-change motivation from the docstrings (kept in the changelog). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5604 +/- ##
==========================================
- Coverage 98.10% 98.10% -0.01%
==========================================
Files 339 339
Lines 31779 31808 +29
==========================================
+ Hits 31178 31204 +26
- Misses 601 604 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
Note on the failing Read the Docs check: this is not caused by this PR. Locally, all docs build steps pass on this branch:
The RTD failure is the strict |
aabills
previously approved these changes
Jun 15, 2026
aabills
left a comment
aabills
left a comment
Contributor
There was a problem hiding this comment.
overall fine but imo the docs are written a bit incorrectly
| "$$\n", | ||
| "\n", | ||
| "Here $i_k$ is the current, $\\phi_k$ the potential, $a_k$ the surface area to volume ratio, $j_k$ the interfacial current density, $\\eta_k$ the overpotential, $U$ the open-circuit potential, $R_{cr}$ is the contact resistance, and $V_{cell}$ is the total cell volume. The averaged heat source term $\\bar{Q}$ is computed by taking the volume-average of $Q$.\n", | ||
| "Here $i_k$ is the current, $\\phi_k$ the potential, $a_k$ the surface area to volume ratio, $j_k$ the interfacial current density, $\\eta_k$ the overpotential, $U$ the open-circuit potential, $R_{cr}$ is the \"Contact resistance [Ohm]\" (which really represents the total series resistance of the cell, e.g. contact resistances between layers, and tab, weld and lead resistances, not only the contact resistance), and $V_{cell}$ is the total cell volume. The averaged heat source term $\\bar{Q}$ is computed by taking the volume-average of $Q$.\n", |
Contributor
There was a problem hiding this comment.
doesn't the "total series resistance of the cell" if read literally also include electrolyte resistance and butler-volmer?
Reword "total series resistance of the cell" to "additional series resistance not already captured by the electrochemical model" so the description does not read as also including electrolyte/Butler-Volmer contributions. Fix typos in the notebook, trim verbose option docstring, and remove the misleading "lumped" qualifier on the default option. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BradyPlanden
approved these changes
Jun 16, 2026
This was referenced Jun 16, 2026
Closed
BradyPlanden
added a commit
that referenced
this pull request
Jun 16, 2026
…monorepo-merge Brings in 9 commits from origin/main since the last sync (0b3f7b2..ae2e620): - #5616 BPX State "Initial state-of-charge" now applied (was ignored) - #5604 "Contact resistance [Ohm]" can be a function of temperature - #5610/#5612 fix incomplete BPX import for interpolated-table OCP branches - #5609 preserve custom-model discretisation recipe across serialise/load - #5613 linkcheck/lychee ignores for bot-blocked URLs - #5611 add agent-guidance files (AGENTS.md + symlinks) - #5614/#5615 security bumps (bleach 6.4.0, starlette 1.3.1, tornado 6.5.7) - #5618 v26.6.2.0 release (changelog + CITATION.cff) Resolution notes: - Merge auto-resolved with zero textual conflicts; rename detection mapped all src/* -> packages/pybamm/src/* , tests/* -> packages/pybamm/tests/* and CITATION.cff -> packages/pybamm/CITATION.cff. Each relocated file verified byte-identical to origin/main. - CHANGELOG.md: the auto-merge buried the unreleased monorepo #5512 "Features" entry inside the shipped [v26.6.2.0] block. Moved it back under [Unreleased]; the file now equals origin/main plus only that one preserved entry. - docs/conf.py and uv.lock are true 3-way unions: main's edits (linkcheck ignores; bleach/starlette/tornado bumps) combined with the monorepo's changes (bibtex path; workspace lock structure intact). - New agent files (AGENTS.md and the CLAUDE.md/GEMINI.md/QWEN.md/.cursorrules/ copilot-instructions symlinks) added at the repo root, symlinks preserved. NOTE: their content describes the pre-monorepo src/ and tests/ layout and will need reconciling to packages/pybamm/* as a follow-up.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The
"contact resistance"option adds a series resistance to the terminal voltage (and corresponding Ohmic heating). This PR makes the"Contact resistance [Ohm]"parameter aFunctionParameterof the volume-averaged cell temperature (a scalar), instead of a temperature-independent scalar.It also reframes the documentation: as noted in BPX#130, this is really a lumped series resistance, not a true contact resistance. Renaming the parameter would be a painful breaking change, so the name
"Contact resistance [Ohm]"is kept and the docs are updated to clarify what it represents.Details
ElectricalParameters.R_contactis now a methodR_contact(self, T)returningpybamm.FunctionParameter("Contact resistance [Ohm]", {"Temperature [K]": T}), matching the idiom used for other temperature-dependent parameters.base_electrode.py(voltage drop) andthermal/lumped.py(I²R heating) pass the volume-averaged cell temperature."Volume-averaged cell temperature [K]"is a fundamental variable registered by every thermal submodel (isothermal, lumped, x-lumped, x-full).Backward compatibility
Fully backward compatible. Existing parameter sets that supply a scalar (e.g.
"Contact resistance [Ohm]": 0) keep working — PyBaMM auto-wraps a numeric value as a constantFunctionParameter. No parameter-set files were changed.Testing
R(T)across isothermal/lumped/x-full: matches the constant case at the reference temperature (isothermal) and produces a larger overpotential as the cell heats (lumped/x-full).🤖 Generated with Claude Code