Repository navigation
Conversation
`"heat of mixing": "true"` could not be used with a composite electrode.
`BaseThermal._heat_of_mixing` read the single-phase variable names
("Negative particle radius [m]", "X-averaged negative particle
concentration [mol.m-3]") and took its material properties from
`param.n.prim` / `param.p.prim` alone, so any model built with
`"particle phases": "2"` failed at build time with
ModelError: Missing variable for submodel 'thermal':
'Negative particle radius [m]'.
Richardson's term is per-particle-population, so the fix is to evaluate it
per phase and add the contributions, which is what the reaction, reversible
and hysteresis heat sources in the same class already do. The phase loop
reuses the `phase_names = ["primary ", "secondary "]` idiom used there.
The single-phase path is untouched: it takes the `num_phases == 1` branch,
which builds the same expression from the same unprefixed variable names as
before. A DFN with Chen2020 and "heat of mixing": "true" returns voltage,
temperature and "Volume-averaged heat of mixing [W.m-3]" bit-identical to
the previous implementation.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The composite heat-of-mixing tests only built the model. Extend the graphite/graphite heat-source comparison from #5770 so that splitting one graphite phase into two identical halves must also reproduce the one-phase "Volume-averaged heat of mixing [W.m-3]": - run it with "heat of mixing": "true", for isothermal (heat source only) and lumped thermal models, comparing the temperature rise for the latter - give Chen2020 the Ai2020 entropic changes; with zero dU/dT the reversible heat and the temperature dependence of U inside the heat of mixing were compared as zero against zero - solve at rtol 1e-6: at the default rtol the ~300 K temperature state drifts by ~0.03 K, which is 1% of the rise and failed SPM + lumped A heat of mixing that sums only the primary phase fails all four cases. Add a Si/graphite smoke test (Chen2020_composite, lumped) checking the heat of mixing stays positive and heats the cell. It supplies the per-layer "Negative electrode density [kg.m-3]" the set lacks and swaps the cubic Interpolant graphite OCP for the analytic Chen2020 fit, whose derivative the solver can convert. Also look up the electrode and phase parameters through param.domain_params and options.phases instead of getattr. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Parametrise the graphite/graphite heat-source comparison over each value of the "thermal" option, for SPM and DFN: - isothermal (heat source only) and lumped, on Chen2020 with the Ai2020 entropic changes, as before - x-full, which resolves the temperature in x, so the heat of mixing reads the local electrode temperature instead of the x-averaged one - x-lumped with 1+1D current collectors The pouch cases use Marquis2019, which ships the tab and heat transfer parameters, a non-zero entropic change and an analytic, temperature dependent particle diffusivity. A heat of mixing that sums only the primary phase fails all eight cases. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Composite models default to "surface form": "algebraic" while one-phase models default to "false", so the comparison was between two electrolyte formulations. Pin "algebraic" for both, which makes it like with like and lets SPMe join SPM and DFN: with the defaults SPMe differs by 1.4e-3 in voltage from the formulation change alone. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…xing pybamm had no check that its heat sources conserve energy. The reaction and hysteresis heats sum to a j (phi_s - phi_e - U_eq) at the particle surface, so the stored chemical energy is -F * int U_eq dc over the particles, and integrating by parts leaves exactly the heat of mixing taken with U_eq. The new test integrates a 1C SPM discharge and checks that the fall in that stored energy equals electrical work plus heat to 0.5% of the heat, for a one-phase cell and for a composite positive electrode whose second phase has current-sigmoid hysteresis with branches tilted away from its OCP. The SPM keeps the electrolyte uniform and an isothermal model with zero entropic change keeps temperature out of the stored energy, so the particles are the only store. Residuals are about -0.1%. The heat of mixing is ~45% of the heat, and two plausible mistakes fail: summing only the primary phase (+19%) and taking the heat of mixing from the hysteresis-weighted branch instead of U_eq (-1.4%). The heat-of-mixing docstring now says it must share U_eq with the hysteresis heat. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The heat of mixing is the dissipation of the particle flux, -N . grad(mu) = F D |grad c|^2 dU_eq/dc, so it needs the D the flux uses. It re-evaluated the base `phase_param.D(c, T)` instead, which ignores the "current sigmoid" diffusivity branches and the stress-induced diffusion factor. Read the particle's "effective diffusivity" variable, x-averaged for the x-averaged models, as the particle submodels publish it. With lithiation and delithiation diffusivities of D/3 and 3D on one phase the heat sources missed the energy balance by 33% of the heat. The energy-balance test now gives the composite positive's hysteretic phase a current-sigmoid diffusivity (D/2, 2D): the old code fails it at -18%, the new code passes at -0.2%. A uniform particle profile publishes no diffusivity and has no gradient, so its heat of mixing is now zero outright. That also makes a DFN with "particle": "uniform profile" and heat of mixing build; it raised a shape error before. Default options give bit-identical voltage, temperature and heat of mixing for SPM, SPMe and DFN on Chen2020 and Marquis2019. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Apply the review suggestion: build the phase-name prefixes from options.phases, with "" for a one-phase electrode, and sum over them, so the one-phase case needs no branch of its own. One-phase results are bit-identical. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Merging main put them under the released v26.9.0.0 section. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lynomial # Conflicts: # CHANGELOG.md # packages/pybamm/tests/integration/test_models/test_full_battery_models/test_lithium_ion/test_compare_outputs_two_phase.py
The polynomial profiles reconstruct c on the nodes, so grad(c) does not discretise, and dU/dc of the reconstructed c silently evaluates to zero. Use D (dc/dr)^2 dU/dc = -N dU/dr with the closed-form flux. Closes #5876. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4 of 5 tasks
Contributor
|
Bencher Report| Project | PyBaMM |
| Branch | fix/heat-of-mixing-polynomial |
| Testbed | bare-metal |
🐰 View full continuous benchmarking report in Bencher
⚠️ WARNING: Truncated view!The full continuous benchmarking report exceeds the maximum length allowed on this platform.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5935 +/- ##
=======================================
Coverage 98.23% 98.24%
=======================================
Files 350 350
Lines 36104 36101 -3
=======================================
- Hits 35468 35466 -2
+ Misses 636 635 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lynomial # Conflicts: # CHANGELOG.md
This branch has not been deployed
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
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
Fixes #5876.
Note
Depends on #5769. This branch is #5769 with
mainmerged in, so until that lands the diff also shows its commits. Only3a185f88dbelongs to this PR.With
"particle": "quadratic profile"or"quartic profile", the particle concentration is a polynomial inrreconstructed on the mesh nodes, not a finite-volume state. The heat of mixing tookgrad(c)of it, which fails to discretise (ShapeError, 21 vs 19 rows for the SPM). Getting past the shape error is not enough:U(c / c_max).diff(c)also evaluates to zero whencis a compound expression, so the heat of mixing would silently be 0.For these profiles the heat of mixing now uses
D (dc/dr)^2 dU/dc = -N dU/drwith the profile's closed-form flux
N = -D dc/dr, and differentiates the OCP alongr. Fickian diffusion is unchanged, and the uniform profile is still zero (from #5769).Check
Chen2020, 1C, lumped thermal,
"Heat of mixing [W]"time-averaged over 100–3000 s, compared with Fickian diffusion:test_heat_of_mixing_polynomial_profilechecks the heat of mixing is positive and matches Fickian diffusion to 15% (quadratic) and 5% (quartic) for the SPM and DFN. It fails onmain.Type of change
Bug fix, with an entry under
## Bug fixesin CHANGELOG.md.Important checks:
🤖 Generated with Claude Code