Repository navigation
Fix based on review - #2
Merged
NicolaCourtier merged 78 commits intoOct 13, 2026
Merged
NicolaCourtier merged 78 commits into
NicolaCourtier merged 78 commits into
Conversation
…eam#5755) * Use true node spacing for finite-volume node-to-edge shifts `FiniteVolume.shift`'s "node to edge" matrices hard-coded weights that assume uniformly spaced nodes: `[0.5, 0.5]` on interior edges and `[1.5, -0.5]` / `[-0.5, 1.5]` on the two exterior edges. `boundary_value_or_flux` extrapolates from the true node positions, so on a non-uniform mesh the two disagree. That breaks mass conservation wherever a flux is imposed as a gradient divided by a surface value, as the Fickian particle does, because the flux the gradient produces multiplies by the node-to-edge diffusivity while the boundary condition divided by `surf(D_eff)`. The resulting bias acts in the same direction every step, so it accumulates: with a concentration-dependent particle diffusivity on an `Exponential1DSubMesh`, `ORegan2022` gains 26% of its particle lithium over three 1C cycles. Weight interior edges by the true distances and share one true-position extrapolation for the exterior edges between the arithmetic and harmonic means. Both reduce to the old values on a uniform mesh, so uniform-mesh results are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Add CHANGELOG entry for pybamm-team#5755 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…y with 2 updates (pybamm-team#5757) Signed-off-by: dependabot[bot] <support@github.com>
…els (pybamm-team#5745) (pybamm-team#5747) * Keep t_plus * i_e / F inside the electrolyte flux in the Basic* models (pybamm-team#5745) * Fix sodium BasicDFN electrolyte conservation
* fix(discretisation): carry domain on a primary-broadcast Vector A Vector produced by a primary broadcast dropped its domain during discretisation, leaving downstream operations (including DiffSL export) with an empty domain. Carry the broadcast's domain onto the Vector. * feat(diffsl): support Interpolant nodes in DiffSLExport Export a pybamm.Interpolant to DiffSL's native interp1d over the table data (declared as constant tensors), so models with interpolated parameters (OCP, diffusivity lookup tables, ...) can be exported. This is piecewise-linear with endpoint clamping, matching numpy.interp and pybamm's own linear interpolant, and stays compact for any table size. 2D interpolants use successive 1D interpolation (interp1d along the inner axis, linear blend along the outer) until DiffSL gains an interp2d. Covered by unit tests (1D/2D export shape) and an integration test that solves the exported model and matches an independent pybamm CasadiSolver solve. --------- Co-authored-by: Martin Robinson <martinjrobins@gmail.com>
…y with 2 updates (pybamm-team#5760) Signed-off-by: dependabot[bot] <support@github.com>
…ble name (pybamm-team#5759) * Fix duplicated prefix in minimum particle surface concentration name An implicit string concatenation in base_particle.py repeated its first line, so the dimensional minimum particle surface concentration was only reachable as "Minimum negative particle Minimum negative particle surface concentration [mol.m-3]" and the intended name resolved to nothing. * Generalize the variable-name test to catch repeated words Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* chore: bump pinned CasADi from 3.7.2 to 3.8.1 CasADi 3.8 ships cp311-abi3 wheels on every platform, so one wheel covers 3.11 and every later CPython. 3.7.x publishes version-specific wheels only and stops at cp314, which blocks Python 3.15. Two migration points: * CasADi 3.8 changes the result type of numpy functions applied to CasADi values (casadi#2959) and warns about it. PyBaMM's Function nodes depend on the legacy types, and the new mode returns ArrayInterfaceMX rather than MX, so pin the legacy behaviour with setNumpyMode(-1) at import. * CasADi 3.8 no longer dispatches scipy's erf ufunc onto symbolic values, which broke the generic pybamm.Function(special.erf, ...) path. Map it to casadi.erf explicitly. pybamm.erf already overrode _casadi_evaluate and was unaffected. Also bump the auditwheel exclude to libcasadi.so.3.8. Verified locally on macOS arm64 / CPython 3.14: solver builds, 57 solver tests, 4353 pybamm unit tests, and 496 integration tests all pass. NOT done here: pybamm-team/casadi-vcpkg-registry has no 3.8.1 port, so the Windows vcpkg baseline in vcpkg-configuration.json still points at 3.7.2. Windows wheels need that registry bump before this can merge. * ci: point the vcpkg casadi registry at the 3.8.1 port Windows solver wheels build CasADi from the pybamm-team casadi-vcpkg registry, whose baseline still resolved to the 3.7.2 port. Move it to the 3.0.0 port entry (CasADi 3.8.1). TEMPORARY: baseline/reference point at the unmerged branch update-casadi-3.8.1 (casadi-vcpkg-registry#7) so Windows CI can validate the port. Repoint both at the post-merge main SHA before this PR merges. This rotates the vcpkg binary-cache key, so the first Windows run rebuilds the ports from source. * fix: keep the auditwheel exclude on libcasadi.so.3.7 CasADi 3.8.1 still ships libcasadi.so.3.7 -- the soname does not track the release version -- so excluding libcasadi.so.3.8 left the real dependency unexcluded and auditwheel failed to locate it: ValueError: Cannot repair wheel, because required library "libcasadi.so.3.7" could not be located The macOS and Windows repair commands match on an unversioned "libcasadi", so they were unaffected. * ci: repoint the vcpkg casadi registry at merged main casadi-vcpkg-registry#7 is merged, so the baseline/reference no longer need to track the unmerged branch.
…ybamm-team#5766) Co-authored-by: Nishmith121 <nishmith121@github.com>
* fix: correct half-cell electrolyte lithium diagnostics * docs: link half-cell fix changelog entry * fix: serialise electrolyte integral
Signed-off-by: dependabot[bot] <support@github.com>
…m-team#5773) Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com> Signed-off-by: dependabot[bot] <support@github.com>
…amm-team#5775) * Report whole-cell lithium in mol and fix a_j label in BasicDFN2D The lithium integrals over x and z were in mol per metre of width; scale them by the electrode width L_y so they are in mol. Rename the volumetric a_j variable from "Current density [A.m-2]" to the full DFN's "Sum of volumetric interfacial current densities [A.m-3]". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Add PR link to CHANGELOG entry Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Subscript "cool" was not encapsulated correctly.
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Fix typo in docstring: occured to occurred * Fix typos in Serialise docstrings: seperate to separate * Fix typo in solver error message: occured to occurred * Fix typos in test comment: propegated/sucessfully
pybamm-team#5776) * Use the local active material fraction for the BasicDFN2D surface area The surface area used the electrode-averaged active material volume fraction while the solid lithium and conductivity used the position-dependent one, so a graded loading failed to discretise (x grading) or to solve (z grading). Adds a lithium conservation test with uniform and +/-10% graded loadings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Add PR link to CHANGELOG entry Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Robert Timms <43040151+rtimms@users.noreply.github.com>
* improve composite coupling * improved sparsity * jacobian spy helpers * improve spy and style * idaklu jac unit tests * Update CHANGELOG.md * Scope the x-averaged electrolyte concentration state to SPMe The full conductivity submodel never reads the x-average, so on DFN it was only an extra algebraic equation. Also document that get_jacobian_sparsity reports the pattern of J - cj*M, and only call tight_layout in spy when it created the figure.
…#5774) * Stop flooring cos(theta) in the over-relaxed TPFA weight FiniteVolumeUnstructured floored cos(theta) at 0.05 (an OpenFOAM convention), capping the two-point weight alpha = 1/cos(theta) at 20. Kuhn tetrahedra of a thin pouch-cell slab have internal faces about 89.9 degrees from their centroid line (cos(theta) ~ 1e-3), so the cap under-weighted the implicit two-point part and the explicit least-squares cross term gave the discrete diffusion operator eigenvalues with positive real part. A tetrahedral DFN on the default pouch geometry grew an electrolyte mode with a 5 s e-folding time and IDAKLU failed. With the exact weight the operator is stable there and the tetrahedral DFN matches hexahedra to about 1 mV. Meshes whose faces never reached the floor are unchanged. The floor also kept boundary faces with cos(theta) <= 0 finite; those now raise the same GeometryError that inverted internal faces already raise. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Add changelog entry for the unfloored TPFA weight Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Drop the boundary-face GeometryError guard Keep this PR to removing the cos(theta) floor. Boundary faces only reach cos(theta) <= 0 on inverted cells, so the guard is unrelated validation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* feat: add VTK plotting for unstructured meshes
Add VTKQuickPlot, QuickPlot/dynamic_plot hooks, the vtk optional extra,
and headless OSMesa CI setup for GIF export. Extracted from the
unstructured finite-volume stack.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(plotting): draw each VTK panel on its own variable's mesh
Every panel was built from the FIRST spatial variable's mesh, so a
3-domain variable (e.g. electrolyte concentration) plotted alongside
a 5-domain one was painted onto the larger grid: all values shifted
by the leading domains' cell count, rendering electrolyte data on
current-collector tabs. Build each panel's grid from that variable's
own mesh, and make _set_cell/point_scalars raise on a length mismatch
so this bug class errors instead of drawing a wrong picture.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Sample unstructured variables for plotting on the plotting side
The regular visualisation grid, mid-plane slices, quiver sampling and
slice-plane positions used to live on the unstructured processed variables;
the spatial-method PR now ships them as pure point interpolators. Add
pybamm.plotting.unstructured_plot_grid with plot_grid, default_slice_positions,
midplane_slices and quiver_data, and have QuickPlot own the grid and slice
positions per plotted variable (the slice sliders update QuickPlot state).
Meshes expose `vertices`, not `nodes`; fix plot_vtk and its test stand-ins.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Use vtkViewport.AddViewProp; VTK 9.7 removed the AddActor2D alias
The CI matrix installs extras unlocked (prebuilt-solver-wheel mode uses
uv pip install), so it picked up VTK 9.7.0, where the legacy AddActor2D
name no longer exists. AddViewProp is the underlying method in every
supported VTK. Lock VTK 9.7.0 so local environments test the same version.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Build the NaN-as-white colormap with with_extremes
matplotlib 3.11 (which the unlocked CI matrix installs) emits a
PendingDeprecationWarning for Colormap.set_bad, and the test suite treats
warnings as errors. with_extremes(bad=...) has been available since 3.4.
Lock matplotlib 3.11.1 so local runs see what CI sees.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat: add unstructured 2D/3D DFN battery models
Add BasicDFN2DUnstructured and BasicDFN3DUnstructured on top of the
meshing, spatial-method, and plotting stack, plus Max/Min reduction and
domain-size helpers needed by those models.
Co-authored-by: Cursor <cursoragent@cursor.com>
* style: match export/strategy whitespace from full branch
Co-authored-by: Cursor <cursoragent@cursor.com>
* Cover the VTK dynamic_plot dispatch and remaining plotting branches
Tests for dynamic_plot(backend="vtk"), the on-screen render window, the
matplotlib-less lookup-table fallback, QuickPlot's 3D "tight" limits path
and the wireframe helper's 3D guard. The interactive-only lines in
VTKQuickPlot.dynamic_plot (interactor.Start and the render inside the
slider callback) cannot run headless and are marked no cover, as
QuickPlot already does for plt.show.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Don't assert on-screen rendering state; headless CI returns an OSMesa window
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Make VTK panel labels legible: three ticks per axis, plain scalar-bar numbers
VTK's automatic major ticks piled a dozen labels onto short or stretched
axes ("0.060.080.100.12") and the through-cell axis became a smear under
the 100x display stretch. Set three explicit labels per axis, print the
range of any axis thinner than 5% of the largest in its title instead of
ticks, drop the "(x10^-6)" label scaling, and format scalar-bar values
with %.4g so "1.27e+03" is no longer clipped to "1.27e+" and "303." loses
its dangling point. Slightly smaller axis and panel-title fonts.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Label a thin axis at its two ends rather than hiding its ticks
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Reject non-unstructured spatial variables in VTKQuickPlot
Only unstructured-mesh scalars and 0D time series are plottable; vector
fields and structured spatial variables used to fall through to the 0D
chart path, crashing or silently plotting mesh node 0.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Read node-centred mesh coordinates through one accessor
ScikitFemSubMesh3D stores coordinates as nodes, not vertices, so the
FEM path of VTKQuickPlot only worked against test fakes. The node-data
fixture now mirrors the real mesh.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Forward dynamic_plot arguments unchanged to the chosen backend
The VTK branch re-passed output_variables by keyword next to *args, so
the positional form raised TypeError; unknown backends now raise
OptionError instead of silently using matplotlib.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Place VTK slices and cube axes on each panel's own mesh
Slice position and axis ranges came from the first variable's mesh, so a
panel on a sub-domain mesh was cut and labelled against the wrong
extents. 2D meshes now map their (x, z) columns explicitly instead of
indexing a missing third column.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Give every nox entry point the headless VTK setup
The OSMesa packages and default window class were wired only into the
reusable workflow, so the periodic matrix ran the unit suite without
them. The env default now lives in noxfile.py for headless Linux, and
the periodic workflow installs the same system packages.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Let VTK's factory choose the render window; declare pillow
Off-screen Linux no longer hard-codes the OSMesa window class, which
crashed desktops without libOSMesa; headless hosts select it through
VTK_DEFAULT_OPENGL_WINDOW instead. save_gif's PIL import is now an
optional-dependency import backed by the vtk extra.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Share QuickPlot's unstructured rendering between plot and slider_update
The 2D quiver, unstructured pcolormesh and 3D slice blocks were pasted
into both methods; they now live in one helper each. The pcolormesh
helper also replaces the previous frame's artists instead of stacking
them, and the fixed-limits scan uses a single comprehension pair.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Draw 3D scalar slices and slice sliders in the display unit
The plot_surface panels and the y/z slice sliders used raw metres with
unit-less labels while every other panel honoured spatial_unit.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Transfer mesh and scalar arrays to VTK with numpy_support
Points, connectivity and per-frame scalar updates went through
per-element Python loops; they are now single vectorised copies, and the
cell/point scalar setters share one implementation.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Bring the VTK plotting module in line with repo conventions
Log through pybamm.logger, raise OptionError/ShapeError/GeometryError
instead of ValueError, import optional dependencies through
import_optional_dependency, keep inline comments to two lines, add the
future-annotations import and type hints on the public functions.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Count surface points as inside and cache 3D containment masks
contains_points_3d compared the winding number against exactly 2*pi, the
value a point on a face subtends, so points on the bounding-box faces were
classified pseudo-randomly and every default 3D slice came back with NaN
speckle along its outline. Use a small positive threshold so surface
points count as inside, as in 2D, and vectorise the winding-number sum in
bounded chunks.
contains_points now remembers the mask per query-point array, so a slider
redrawing the same display grid no longer recomputes the containment test
on every frame.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Fix QuickPlot colour limits and 3D guards for unstructured variables
- Reduce quiver magnitudes with nanmax: samples outside a non-rectangular
domain are NaN, which made np.max NaN and collapsed the colour scale to
[0, 1].
- Keep user-specified variable_limits for vector fields instead of
discarding them, and apply them to the quiver colour normalisation.
- Update the 3D colorbar in slider_update when the limits are per-frame,
matching the 2D branch.
- Take 'fixed' limits from the raw cell values for every unstructured FVM
variable, not just 3D, instead of interpolating the 200x200 display grid
over every time step.
- Raise NotImplementedError pointing at VTKQuickPlot for 3D variables that
are not unstructured FVM, instead of an AttributeError deep in
_get_spatial_var.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Validate VTKQuickPlot inputs and make its colour range NaN-safe
- Unwrap solutions with QuickPlot.preprocess_solutions so a list of
simulations (as passed by BatchStudy.plot) works, and raise OptionError
for several solutions instead of silently plotting the first.
- Raise OptionError for an empty output_variables list instead of a
ZeroDivisionError in the panel layout.
- Compute the lookup-table range from the finite values only, so one NaN
cell no longer blanks the whole panel, and raise when nothing is finite.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Mark the contains_points SHA-1 cache key as non-security use
Bandit (via Codacy) flags bare hashlib.sha1; the digest only keys a
per-mesh cache of query grids, so usedforsecurity=False is accurate.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Cover the contains_points cache eviction branch
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Validate VTKQuickPlot options and read the stored variable history
- reject scale values that are not 'auto', None or one factor per axis,
and slice fractions outside [0, 1], with OptionError instead of numpy
IndexError/ValueError
- keep the slice plane 1e-6 of the extent inside the mesh so fractions
0 and 1 no longer cut nothing and leave an empty panel
- use the (points x times) _entries_raw the variable already holds
instead of rebuilding it through the time interpolator
- raise OptionError for an all-NaN variable, drop the unused mesh
attribute, document interpolate_time
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Keep QuickPlot 3D panels stable across slider updates
Slider updates replace each panel's artists instead of calling
ax.clear(), so the limits, locators and labels set by plot() survive.
The slice sliders now carry across variables on different meshes as a
fraction of each variable's own extent instead of writing one absolute
position into every variable.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Drop banner comments flagged by the comment-slop hook
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Give BasicDFN3DUnstructured an element_type argument
Mirror the 2D model so the 3D model can be built on tetrahedra as well
as the hexahedral default, and correct the docstring, which claimed
tetrahedral elements while the generator produced hexahedra.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Cover the unstructured DFN defaults and Reduction shape in unit tests
Build both unstructured models through Simulation for every element
type so the default geometry, spatial methods, submesh types and var_pts
run under the coverage job, and check that Max/Min clear domains and
report a scalar shape. Drop the per-test numpy imports in the
integration file, which already imports numpy at module level.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Drop section banners flagged by the comment-slop hook
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Shrink contains_points_3d chunk to bound temporaries at ~20 MB
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Default quiver_data slice positions to the mesh mid-planes
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Drop forced equal aspect on unstructured 2D panels
Thin electrode cross-sections collapsed to a sliver; other 2D QuickPlot
panels stretch to the subplot, so match them.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Evaluate 0D chart data in one vectorised call
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Factor the VTK panel viewport helper and drop a duplicate name map
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Share VTK 3d cameras per grid bounds instead of across all panels
A panel on a differently placed mesh was framed by the first panel's
camera and drawn off-centre.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Reserve layout room for the 3D slice sliders in QuickPlot
The time slider moves up to make space for two slice sliders, but plot()
still reserved only the single-slider strip, so panels overlapped it.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Reject VTKQuickPlot options for unknown variables or several slice axes
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Make BasicDFN3DUnstructured a subclass of BasicDFN2DUnstructured
The 3D model was a near-verbatim copy of the 2D file. The 2D model now builds
its transverse spatial variables, position-dependent parameter inputs,
zero-flux boundary sides, default geometry and default var_pts from class
attributes, and the 3D model only overrides those attributes. As a result the
3D porosity and active-material FunctionParameters gain the missing
"Horizontal distance (y) [m]" input, and the 3D default mesh is resolved
comparably to the 2D one (10 through-cell points per domain, 5 x 5 transverse)
instead of 5 x 3 x 3.
Both modules now start with `from __future__ import annotations` and the
constructors are type-hinted. Solutions of the 2D quad/triangle and 3D
hexahedron models are unchanged to the last bit.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Conserve lithium in the unstructured DFN electrolyte balance
The unstructured models used the non-conservative (1 - t_plus) a_j / F source
that pybamm-team#5745 removed from the other basic DFNs. Since the unstructured
divergence only accepts div(D * grad(u)) products, -div(N_e) with
N_e = -D_e grad(c_e) + t_plus i_e / F is expanded into three such terms. With
ORegan2022 (concentration-dependent t_plus) the total lithium inventory now
stays constant to 1e-15 relative instead of drifting by 1.8e-2.
Also expose the electrolyte current density i_e and the electrolyte flux N_e
under the variable names that previously held a_j and D_e_eff, and use the
surface-stoichiometry-dependent electrode conductivity like BasicDFN2D so
parameter sets such as ORegan2022 process.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Test the unstructured DFN models like the other basic models
TestBasicDFN2DUnstructured and TestBasicDFN3DUnstructured now inherit
BaseBasicModelTest, so both models run through a three-step Experiment, and
TestElectrolyteConservation checks both against ORegan2022 using the models'
own total lithium variables. The 2D-vs-structured and 3D-vs-2D voltage
comparisons pass an explicit coarse mesh instead of relying on the default
var_pts, and the 3D-vs-2D tolerance tightens from 5 mV to 0.1 mV since the
3D model is uniform in y.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Default VTKQuickPlot to the model's plottable quick-plot variables
Replace the unused _is_unstructured_spatial_variable helper with
_variable_kind and use it both to classify requested variables and to
filter default_quick_plot_variables, instead of plotting the model's
first variable key ("Time [s]" for every battery model).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Reject unknown VTKQuickPlot panel option keys and plot_type values
A misspelled key or plot_type (e.g. "Slice") used to fall through to
the default 3D pipeline without any error.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Read spatial variable history through the public entries property
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Make VTK 0D chart ranges NaN-safe and fill chart tables in one call
The chart axis range now comes from the finite samples, matching the
spatial panels, and the time/value columns are transferred with
numpy_to_vtk instead of a per-value InsertNextValue loop.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Honour fixed colour limits for quiver panels and draw wireframes once
- variable_limits="fixed" (the default) now spans the vector magnitude
over every time and cell instead of silently behaving as "tight"
- the static 2D mesh wireframe is drawn once per figure (zorder above
the QuadMesh) rather than rebuilt on every slider update
- the 3D error message only points at VTKQuickPlot for node-centred
unstructured variables, which is what it accepts
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Require VTK >= 9.3.1, the first release whose numpy_support works with NumPy 2
pybamm needs numpy>=2.0.0, so the previous >=9.0.0 bound admitted VTK
wheels whose numpy bridge fails at import.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Mention the QuickPlot unstructured support and vtk backend in the changelog
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Flatten grouped quick-plot defaults and reject empty VTK panel options
A model's `default_quick_plot_variables` may group several names onto one
QuickPlot axis, so `_default_output_variables` was indexing the solution with a
list and dying with `TypeError: unhashable type: 'list'` — e.g. any
`VTKQuickPlot(solution)` on a Thevenin, lead-acid or `basic_splitOCVR`
solution. Flatten the groups first; VTK draws one panel per variable anyway.
An empty per-variable options list passed every other validation check but left
the variable with no panel, and `dynamic_plot` then divided by a zero panel
column count. Reject it where the other malformed options are rejected.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Drop the unreachable unstructured branch and list the new plotting modules
`_get_spatial_var` returns early for every unstructured variable through the
display-grid lookup above, so the unstructured classes in the `second`
dimension's isinstance check could never be reached — and neither class carries
the `second_dim_pts` the branch reads. Leave `ProcessedVariable2DFVM`, which is
the only class that gets there.
`plot_vtk` and `unstructured_plot_grid` were missing from the plotting
package's `__all__`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Cover the zero-magnitude fallback for quiver colour limits
Codecov's one patch miss on this PR: a vector field whose magnitude is zero
everywhere leaves `float(np.nanmax(magnitude))` at 0, and the (0, 0) range would
paint every arrow the same colour, so the limits fall back to (0, 1). Give
`_triangle_vector_solution` a `values` argument and assert the fallback; the
test fails if the guard is removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Move the VTK changelog entry to Unreleased and share one slider update path.
The bullet had landed under the released v26.8.0.0 section. The time slider updated each panel twice and read cell data through a private helper the public call already covers.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Fix 3D plotting controls and magnitudes
Co-authored-by: Cursor <cursoragent@cursor.com>
* Label VTK frames with the snapped time that is drawn
Without interpolate_time the panels show the nearest stored time, but the
time text and chart markers used the raw slider value.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Step notebook slice sliders by the mesh extent
ipywidgets' default step of 0.1 display units left a metre-unit slice slider
snapping between its two ends.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Point VTK x/y slice cameras along the plane normal
The slice camera only moved the axis_idx component of ResetCamera's
default +z position, so x- and y-slices were viewed ~59 degrees off
their normal. Place the camera at the focal point offset along the
normal only.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Label VTK slice axes with the slice's own extent
Slice panels drew their cube axes around the cut geometry but labelled
them with the whole mesh's range, so a slice covering part of a
non-box domain showed wrong coordinates. Derive the physical ranges
from the drawn bounds divided by the panel scale.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep surface points inside contains_points_3d on off-axis meshes
A point exactly on a boundary triangle makes that triangle's solid
angle +-2*pi (+-pi on an edge) with the sign set by round-off. On
axis-aligned faces the round-off happened to vanish, but on a rotated
mesh ~half the surface points came back outside, so plots showed NaN
gaps along the boundary. Inflate the boundary by 1e-9 of the mesh span
along the vertex normals, keeping the surface closed, and classify with
the 2*pi midpoint again.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Merge the slice-camera comments into two lines
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep VTK scalar and chart data in float64
Casting to float32 collapsed fields with a small spread on a large offset
(e.g. 298.15 K + 2e-5 K) onto a couple of colours while the colour bar,
computed in float64, showed the full range.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Drop matplotlib 3D support from QuickPlot
VTKQuickPlot is the 3D path. QuickPlot keeps 2D unstructured scalar and
vector-field plotting and now raises for any 3D variable, pointing
unstructured ones to VTKQuickPlot. Removes the mid-plane slice panels,
3D quiver panels, the slice sliders, and their sampling helpers.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Restore the 2D-only multi-solution error assertion
QuickPlot's multiple-solution error is 2D-only again after dropping
matplotlib 3D support, so the test matches main's message.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Report whole-cell lithium in mol and fix a_j label in unstructured DFN
The 2D model integrated lithium over x and z only (mol per metre of width);
scale by the unresolved length L_y so both models report mol. Rename the
volumetric a_j variable from "Current density [A.m-2]" to the full DFN's
"Sum of volumetric interfacial current densities [A.m-3]".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Use the local active material fraction for the unstructured DFN surface area
The surface area used the electrode-averaged active material volume fraction
while the solid lithium and conductivity used the position-dependent one, so a
graded loading reacted as if uniform and total lithium drifted (~0.2% over
20 min). The conservation test now also runs with a +/-10% graded loading.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Drop the Max/Min Reduction and domain_size changes from the unstructured DFN PR
Neither unstructured model uses them and the models solve without them,
so they belong in their own PR once something needs them.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Simplify the unstructured DFN axis bookkeeping
List only the resolved axis letters and look up each axis's
FunctionParameter label and boundary sides in one table. Drop the
direction tags (the unstructured method reads the axis from the name),
the separate z_2d/z_3d geometry variables, and the spatial variables
in model.variables; geometry and var_pts are keyed by plain strings.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Write the unstructured DFN geometry out explicitly
Replace the axis table and nested helpers with a single
_three_dimensional flag, explicit spatial-variable lists per domain and
inline integrals, sides and geometry.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Merge the unstructured DFN models into BasicDFNUnstructured
Replace BasicDFN2DUnstructured and BasicDFN3DUnstructured with one
BasicDFNUnstructured(dimension=2|3, element_type=None). The element type
defaults to quad in 2D and hexahedron in 3D; other dimensions raise
OptionError.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Select the unstructured DFN dimension with a 'mesh dimensionality' option
BasicDFNUnstructured now takes (options=None, name=...) like the other
basic models. The new 'mesh dimensionality' option (1 default, 2 or 3)
sets the electrode/separator mesh dimension; only BasicDFNUnstructured
accepts 2 or 3 and it defaults to 2. The element_type argument is gone:
simplex elements are chosen through Simulation(submesh_types=...).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Use the existing 'dimensionality' option for the unstructured DFN
'dimensionality' already counts the directions resolved besides x
(1 = z, 2 = y and z), so it selects the x-z or x-y-z mesh directly.
Drop the 'mesh dimensionality' option and its base-class check; the
model defaults to 1 and rejects 0 and 3.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Solve the unstructured DFN for every element type in the unit test
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Name the interfacial current density correctly in the 2D basic DFN models
"Negative/Positive electrode current density [A.m-2]" is the solid-phase
current density elsewhere in PyBaMM; these models stored j under it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Include L_y in the unstructured DFN conditioning scale in 3D
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Add "Voltage expression [V]" to the unstructured DFN
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Build the unstructured DFN integration-test models inside the test
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Restore the blank line in pybamm/__init__.py
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Say tetrahedra are not supported by the unstructured DFN
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ybamm-team#5789) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* Implement Mayur2024 Added Mayur2024 parameter file for composite degradation (Si/Gr anode) and tested. * Removed redundant folder * style: pre-commit fixes * fix: rename Mayur2024 to Bonkile2024 and fix citation wiring Addresses PR review comments on the composite Si/graphite parameter set: - Rename the parameter set from Mayur2024 to Bonkile2024 (surname+year, matching every other set's convention) before release makes it a breaking change. Renamed the module, and updated the pyproject.toml entry point and the lithium_ion __init__.py __all__ list (placed alphabetically, dropped the trailing comma before the closing bracket). - Register the actual source paper as a citation: added a Bonkile2024 BibTeX entry to CITATIONS.bib (alphabetically between Beeckman1990 and BrosaPlanella2021), put "Bonkile2024" first in the parameter set's citations list, and changed the get_parameter_values() docstring's plain-prose attribution to :footcite:t:`Bonkile2024` so it renders like the neighbouring references. - Fix the invalid "Ai2020" citation key (no such entry in CITATIONS.bib) to "Ai2019", both in the citations list and in a docstring footcite reference -- this was breaking pybamm.print_citations() for every remaining unknown citation, and breaking the docs build. - Register "Yang2017", which was already cited in prose but missing from the citations list. - Fix an accidental implicit string concatenation that split "Electrolyte conductivity [S.m-1]" across two literals for no reason. Verified every :footcite:t: key in the file resolves against CITATIONS.bib and is present in the citations list (checked programmatically in both directions). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add Bonkile2024 to param_to_model test and changelog - Adds a `Bonkile2024` entry to test_parameter_values_with_model covering the model options it needs: dual particle phases with a current-sigmoid/single OCP split, solvent-diffusion-limited SEI, partially reversible lithium plating, swelling-and-cracking/ swelling-only particle mechanics, and stress-driven LAM. - Adds a Features changelog entry under Unreleased for PR pybamm-team#5694. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015UqRkqrpsTfNkfwzuf68CD * Clear comment-slop hits in Bonkile2024 and EntryPoint docstrings Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: rtimms <rob@ionworks.com>
…curity-updates group across 1 directory (pybamm-team#5833) Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com> Signed-off-by: dependabot[bot] <support@github.com>
…ybamm-team#5835) * feat: anisotropic coefficients in FiniteVolumeUnstructured div(D grad u) div_D_grad now accepts a VectorField (one value per axis, a diagonal tensor, as FiniteVolume2D does) or a rank-2 TensorField (full tensor), each component a cell field. Each face uses its normal conductivity n.K.n from the owner and neighbour cells, combined by the same distance-weighted harmonic mean as a scalar D, and n.K.n of the owner on boundary faces. Identically-zero constant components are skipped. Exact when K's principal axes are aligned with the faces; the tangential part of K n is not included. Checks: hex box with K = diag(1, 10, 100) reproduces the scalar operator with each axis's k to 1e-15, and linear fields to 6e-15. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: anisotropic div(K grad u) in FiniteVolumeUnstructured Isotropic VectorField / TensorField match the scalar operator on triangle, quad, tet and hex meshes; a diagonal tensor uses each axis's conductivity (interior, Dirichlet and Neumann faces); on axis-aligned faces a full tensor reduces to its diagonal (the documented limitation), while a rotated tensor differs on a tet mesh; zero components are skipped; secondary-domain repeats; shape and zero-coefficient errors; pybamm.div(K * pybamm.grad(u)) dispatches to div_D_grad. The zero check now looks through broadcasts and treats an unevaluable component as non-zero. Replaces the test that a VectorField coefficient raises. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: changelog PR number for pybamm-team#5835 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * style: pre-commit fixes * fix: satisfy comment-slop and cover state-dependent anisotropic components Drops the unreachable NotImplementedError branch, adds a state-dependent VectorField test, and trims comments flagged by the style job. The comment-slop hook diffs against HEAD locally, so it was checked against the base ref instead. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…n a direction (pybamm-team#5838) * Reject FiniteVolume2D meshes with a single node in either direction Node-to-edge shifts and boundary extrapolation in FiniteVolume2D need at least two nodes per direction, so a 2D submesh with one node (e.g. BasicDFN2D with z_2d = 1) failed deep in harmonic_mean with an opaque scipy ValueError. build() now raises a DiscretisationError up front; ghost-cell submeshes are exempt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Add PR number to CHANGELOG entry Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci: retrigger cancelled Read the Docs build Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
… nodes (pybamm-team#5839) * Compute FiniteVolume2D extrapolation spacings only when needed Boundary extrapolation read the second node spacing in both directions unconditionally, so any 2D mesh with fewer than 3 nodes in either direction raised IndexError. Spacings are now looked up lazily and a DiscretisationError is raised when the mesh is too coarse for the requested extrapolation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Add PR link to CHANGELOG entry Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: drop one-node coarse-mesh cases now rejected by pybamm-team#5838 FiniteVolume2D raises on meshes with a single node in a direction (pybamm-team#5838), before extrapolation runs, so these two cases can no longer reach the extrapolation check. pybamm-team#5838's own tests cover the one-node rejection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…m#5849) * fix: log NonlinearSolver Newton iterations at DEBUG level StandaloneNewtonSolver never attached a SolverLog to its NonlinearSolver, so per-iteration messages were dropped. Accept an optional logger in solve/solve_batch and pass pybamm.logger.debug when DEBUG is enabled. * test: regression test for NonlinearSolver debug logging * docs: link PR in changelog
… per zone (pybamm-team#5815) Co-authored-by: tleo <tleo@silanano.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: mleot <140573653+mleot@users.noreply.github.com>
…-team#5837) * fix(idaklu): run a single solver outside any OpenMP parallel region With one solver the solve still ran inside a one-thread parallel region, so OpenMP code the model calls during the solve (a CasADi external function, for example) ran nested at level 1. It then took the nested-level ICVs, so under OMP_NUM_THREADS=8,1 its regions got one thread instead of eight. A single solver now runs directly on the calling thread; several solvers keep the self-scheduled team. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: changelog for the single-solver OpenMP fix Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: link the changelog entry to pybamm-team#5837 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(idaklu): a single solve runs outside any OpenMP region The solver's logger runs on the calling thread mid-solve, so it can read omp_get_level() from the loaded OpenMP runtime. It reads 1 on main and 0 with the fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(idaklu): satisfy ruff in the OpenMP runtime lookup Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(idaklu): multiple solvers still run in an OpenMP region Companion to the single-solve test: with num_solvers=2 the calling thread's logger reads omp_get_level() == 1, so the multi-solver path is unchanged and the level probe can see a nonzero level. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…_time schedule (pybamm-team#5817) * fix: give repeated experiment steps their own start/end times Steps are deduplicated by repr, so repeated steps shared one object and _set_next_start_time left every repeat with the first occurrence's times. Each position now gets a shallow copy. Fixes pybamm-team#5809 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: add changelog entry for pybamm-team#5817 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ads (pybamm-team#5785) * fix: processed-variable ordering, 3D unrolling and faster computed reads Return unsorted non-Hermite time queries in query order, unroll computed 3D variables time-major, and stack mixed scalar and vector inputs. Build computed variables' xarray lazily, read 0D ones with np.interp; add Solution.sensitivity_names. * fix: lay out as_computed() rows as the solver returns them as_computed() flattened each time point in C order, the reverse of the F-ordered CasADi output that unroll_2D/unroll_3D expect, so 2D and 3D variables round-tripped with their spatial axes reordered. It now flattens the kernel's raw observation, before per-class reordering. initialise_3D_scikit_fem read a 2+1D model's current-collector nodes y-fastest, but the mesh numbers them z-fastest, so x-y-z variables from output_variables had y and z swapped. The as_computed tests now use spatially varying data, which the constant data could not catch. * fix: convert 0D time integrals in as_computed() as_computed() reshaped a time integral's entries as if they kept a time axis. A 0D integral is already summed to one value, so it raised a ValueError; scikit-fem integrals are already reordered, so 2D and 3D ones came back scrambled. A 0D integral now passes its summed value with time_indep=True, as an output_variables solve stores it, and spatial time integrals raise NotImplementedError. The 3D-only output_variables test becomes a matrix over every layout ProcessedVariableComputed supports and every time-integral method, each checked against an output_variables solve. A guard fails when a new ProcessedVariable subclass or time-integral method has no case. * fix: join output_variables solutions at a shared boundary Solution.__add__ drops the second solution's first time point when it repeats the first's last, but ProcessedVariableComputed._concat kept that sample's data and sensitivities, so the joined variable held one more value than time points and raised on read. The join now drops them too, and takes the joined variable's times from its segments, because Solution.from_sub_solutions passes the final solution to every join. A time integral from output_variables or as_computed() lost its time independence in the join and could not be read. The variable now carries its ProcessedVariableTimeIntegral: an ExplicitTimeIntegral joins into its integral over both segments, counting the initial condition once. A DiscreteTimeSum, or an expression of a time integral, has no value over the join, since every segment samples its end points; the join still succeeds, and reading the variable raises NotImplementedError. The output_variables matrix now joins every layout and time-integral method through Solution.__add__ at a shared boundary, checked against a full-state join. New tests cover sensitivities at a shared boundary, __add__ and from_sub_solutions over three segments, and an experiment with a DiscreteTimeSum output variable. * docs: say a joined time integral leaves out any gap between segments ProcessedVariableComputed._concat sums each segment's own integral, so nothing is integrated between segments that do not share a boundary. The docstring now says so, and a test covers the gap for a join of two output_variables solves and of a full-state solve with one. * test: read a variable from a solve with mixed scalar and vector inputs The all_inputs_stacked test only checked the stacked array. It now also reads a variable from an IDAKLUSolver solve with a scalar and a vector input, which raised a ValueError before the stacking fix. * docs: regenerate the bpx notebook's parameter search output The stored output of parameter_values.search("Negative electrode") had one line edited by hand to the new diffusivity name, but that name no longer matches the search, and the output predates the current search format. It is now the output of running the cell. * docs: say the np.interp fast path also takes an empty or tuple t The comment said every query the fast path takes keeps xarray's errors, but an empty or tuple t, which xarray rejected, now returns values. * refactor: stack Solution inputs with flatten_inputs Solution.all_inputs_stacked re-implemented base_solver's flatten_inputs, which handles mixed scalar and vector inputs and an empty dict the same way. It now uses the helper. solution.py now imports base_solver while pybamm is still initialising, so base_solver defers its annotations; on Python < 3.14 its pybamm.Solution annotation otherwise fails at class creation. * docs: trim the changelog entries and comments Each changelog entry is now one line, leaving the detail to the PR, and the performance entry moves from Features to Optimizations. Four two-line comments become one line, and a test comment that restated the test's name is removed. * docs: say how a joined time integral treats a gap between solutions A full-state join integrates an ExplicitTimeIntegral across a gap between the solutions, while a join involving output_variables sums each solution's integral. Solution.__add__ now says so. * fix: read and join computed variables pickled by v26.9 _unjoinable, _xr_interp_args and time_integral were set only in __init__, so a ProcessedVariableComputed loaded from a v26.9 pickle raised AttributeError on its first read or join. Class-level defaults cover it; a new test strips them from a pickled variable, before and after its first read. * docs: say the joined-solutions fix leaves out sparse variables
… and first_state (pybamm-team#5794) * fix: leave the initial condition out of time-integral sensitivities postfix_sensitivities summed the sensitivities with postfix_sum, which adds the time integral's initial condition. The initial condition does not depend on the inputs, so every sensitivity of ExplicitTimeIntegral(c, 5) came back 5 too large, on both the full-state and output_variables paths. The sum over time is now kept apart from the initial condition, which only the value adds. A new test checks the sensitivity against its analytic value on both paths, and the output_variables join test now uses a nonzero initial condition, which the joined value counts once and its sensitivity not at all. * fix: store every entry of sparse output variables IDAKLUSolver returns only the structural nonzeros of an output variable, such as the zero-flux boundary edges of "Electrolyte current density [A.m-2]", while as_computed() data holds every entry. ProcessedVariableComputed inferred one layout for a joined variable from its first CasADi function and filled every segment from the first segment's data, so joining two such solutions, or the steps of an experiment, raised an IndexError, and joining one to a full-state solution raised a ValueError. The solver now scatters the nonzeros into every entry, and _unroll_nnz is gone. The output_variables matrix gains a 1D and a 2D sparse case, and its join test also joins two output_variables solutions. Its comparisons take an atol, since the full-state path rounds values within about 1e-16 of zero to zero. * fix: trim joined sensitivities and sum time-integral ones over all segments Adding solutions solved with sensitivities dropped the repeated time point at a shared boundary from the states but not from their sensitivities, so reading the sensitivities of any variable raised a CasADi RuntimeError. _merge_sensitivities now drops that time point's rows, in __add__ and from_sub_solutions alike. A time integral's value sums over every segment at once, but its sensitivities were summed per segment and stacked, one row per segment. They now go through a single postfix over all segments. * fix: differentiate a time integral's post-sum expression at the integral postfix_sensitivities evaluated the derivative of a post-sum expression, such as ExplicitTimeIntegral(c, 0) ** 2, at the expression's own value rather than at the integral, so the square gave 2 I^2 dI/da instead of 2 I dI/da. It now takes the summed variable at each time point, as postfix does, and forms the integral itself; both callers pass those raw values. test_explicit_time_integral and test_discrete_data_sum encoded the wrong derivative and are corrected. At the discrete test's a = 2 the model reproduces its data, and the square root of a zero sum has no derivative, so that one sensitivity assertion is skipped. The joined time-integral sensitivity test gains a post-sum case. * fix: take first_state sensitivities from the first time point first_state sliced the last n_states rows of the first segment's sensitivities, which belong to its last time point, so it carried the sensitivities of the wrong state. It now takes the first rows, as the states themselves do. * fix: keep an output_variables solve's initial state on its Solution first_state of a solve that returns only output variables read the model's y0full when first accessed. That vector is the solver's input: it carries the initial sensitivities after the states, holds the algebraic states from before consistent initialization (a DFN's potentials were off by up to 0.08 V), and is overwritten by the model's next solve. In an experiment whose cycle repeats a step, the cycle's first state and every "Change in ..." summary variable came from the last repeat, halving "Change in throughput capacity [A.h]". pybammsolvers now returns y_init alongside y_term when it saves only output variables: the states at t0 after consistent initialization, from which the outputs at t0 are computed. IDAKLUSolver keeps it on the Solution, and __add__, from_sub_solutions, copy and reduce_solution carry it. It ships in the unreleased pybammsolvers 0.11.0, which pybamm already requires. Solution gains a _y0 slot, which a solution pickled before it existed loads as None. * refactor: scatter sparse output variables at their flat indices The fill of a sparse output variable indexed the dense entries by the sparsity's row indices, which are the flat indices only because every output variable is a column vector. It now uses find(), the flat indices for any shape, and reads the sparsity from sparsity_out() instead of evaluating the variable at zero. * fix: take sensitivities only with respect to the sensitivity inputs An output variable's parameter Jacobian, which pybammsolvers uses to compute its sensitivities during an output_variables solve, was taken with respect to every input, but the solver reads one column per sensitivity input. With calculate_sensitivities=["b"] and inputs a and b, the solver took the derivative with respect to a, returned it as "a", left "b" empty, and wrote past the end of its buffer for the column of b. The Jacobian is now taken with respect to the sensitivity inputs, in their order, and the output variables' sensitivities are named by them. The post-sum expression of a time integral was differentiated with respect to every input too, so its (1 x inputs) derivative broadcast against the (1 x sensitivities) chain-rule term: "b" came back as the derivative with respect to a. postfix_sensitivities now takes the sensitivity names, keeps the other inputs as values, and differentiates with respect to those names only. * fix: keep the state sensitivities of an output_variables solve An output_variables solve filled the states' sensitivities from sol.yS, which then holds the outputs' sensitivities as (n_t, n_outputs, n_p): indexing it by parameter picked a time point. With more sensitivity inputs than time points the solve raised an IndexError; otherwise first_state and last_state held one entry per output, of the wrong quantity, and solver.step started the next step from them, raising "y0 has wrong number of cols" or, when the counts matched, giving wrong sensitivities (du/da = 0.23 for an exact 0.86). pybammsolvers now returns yS_init and yS_term alongside y_init and y_term: the states' sensitivities at the first and last time points, one row per sensitivity parameter. IDAKLUSolver keeps them on the Solution for first_state and last_state, and __add__, from_sub_solutions, copy and reduce_solution carry them as they carry y_init and y_event. The states' sensitivities over time are empty, as the states are. Solution gains _y0_sensitivities and _y_event_sensitivities slots, which a solution pickled before they existed loads as None. BaseSolver._check_restart_sensitivities rejected every sensitivity restart from an output_variables solve, whose step boundary had no state sensitivities to seed from. It now rejects only a solve without them, and the step test that expected the rejection checks the restarted sensitivities instead. * docs: trim the changelog entries and comments Each changelog entry is now one line, leaving the detail to the PR, and eleven two-line comments become one line. * docs: remove pybammsolvers version changelog entry * test: keep the event sensitivities of a trailing last_state join from_sub_solutions takes _y_event_sensitivities from the last kept segment, not sols[-1], matching __add__'s single-sample short-circuit, which keeps self's. The comment above it covered only closest_event_idx, so it now says why the event sensitivities follow it too. Simulation joins a trailing single-sample duplicate when every step of a cycle is infeasible: it carries last_state over, which holds no event sensitivities of its own. Taking them from sols[-1] would drop them, so last_state.sensitivities would come from the outputs' rows and a restart would raise. A new test joins a solve with its last_state through both __add__ and from_sub_solutions and checks them.
…ybamm-team#5786) * refactor: put solution observation behind an ObservationBackend seam A Solution reads variables through one ObservationBackend and a ProcessedVariable through one VariableObserver (CasADi only); OutputAssembly owns the output_variables layout and keys its sensitivities by sensitivity parameter. Solution carries its backend in an _observation slot, which a solution pickled before it existed loads as CASADI_OBSERVATION. * test: mark the new pickle round trips nosec, as main's tests do * refactor: read each segment through its own model's observer cache A Solution carried an observation backend, the module-level CASADI_OBSERVATION, which first_state, last_state, +, from_sub_solutions and copy sliced and joined. The backend held no state, so every slice and join returned it unchanged, while the CasADi leaves it compiled were cached on each model. A Solution now holds no observation state. build_variable reads each segment through its own model's ObserverCache, which replaces model._variables_casadi and keeps its rules: keyed by variable name, copied by new_copy, and skipping time integrals. A model pickled before the cache existed builds one on first read. ObservationBackend, CasadiObservation, CASADI_OBSERVATION and join_observations are removed with their tests. * perf: convert each time integral to CasADi once per model ObserverCache skipped time integrals: it cached only the CasADi leaf, so a hit lost the integral and would have read its integrand. Every read of a time integral therefore converted it again for each segment, 20 times for a 20-step experiment, and again for every solve of the model. The cache now keeps a leaf with the expression it evaluates and its time integral. The integral's analysis depends only on its expression, and nothing changes it after conversion, so solutions of one model can share it. Reading an ExplicitTimeIntegral from a 20-step experiment drops from 1.9 ms to 0.6 ms on first read and from 1.5 ms to 0.01 ms on later solves of the model, with identical values. * docs: trim the changelog entries and comments Each changelog entry is now one line, leaving the detail to the PR, and the performance entry moves from Features to Optimizations. Eight two-line comments become one line, and four that restated the code they moved with are removed. VariableObserver holds its leaves rather than owning them: the model's ObserverCache shares them. * test: read an output variable with no structural nonzeros The solver returns no rows for such a variable, so its payload is (n_t, 0). Covers it as the only output and alongside a regular one. * docs: say a cached observer leaf keeps its first solve's inputs and options
…MD (pybamm-team#5864) * perf(pybammsolvers): order KLU factorisation with AMD instead of COLAMD Co-Authored-By: Claude <noreply@anthropic.com> * docs: add PR number to changelog Co-Authored-By: Claude <noreply@anthropic.com> * docs: shorten changelog entry Co-Authored-By: Claude <noreply@anthropic.com> * test: loosen integration solver rtol to 1e-6 Co-Authored-By: Claude <noreply@anthropic.com> * test: rtol 1e-7 for integration solves, 1e-4 for serialisation round-trip check Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Robert Timms <43040151+rtimms@users.noreply.github.com>
…t-up (pybamm-team#5880) Co-authored-by: Marc Berliner <marc@ionworks.com>
* Add model zoo policy * Rob's comments * Apply review suggestions: zoo PR checklist wording, model.toml as maintainer source of truth Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: rtimms <rob@ionworks.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Robert Timms <43040151+rtimms@users.noreply.github.com>
…team#5869) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>
* Require BSD-3-Clause for in-tree model zoo models Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Add PR link to zoo changelog Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Make the zoo license rule its own unwaivable contract check A skip_contract waiver of 'packaging' would otherwise bypass it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
) * fix: average BasicDFN2D voltage over the positive face Voltage was phi_s_p at the top-right corner, so it depended on the z mesh whenever properties vary with z. Use the z-average of the right-boundary value, consistent with the uniform-current Neumann BC there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: link CHANGELOG entry to pybamm-team#5865 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: pin BasicDFN2D voltage to the corner value for uniform properties Mesh independence alone would not catch a wrong normalisation. Also say why the face average is the right voltage. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: rtimms <rob@ionworks.com>
…amm-team#5805) * fix: sum the phases' surface area to volume ratios for the total The total surface area to volume ratio of a multi-phase electrode (pybamm-team#5165) was sum(eps_k * R_k**2) / (3 * sum(R_k**3)). The surface area per unit electrode volume is the sum over phases of particle number density n_k = eps_k / (4/3 pi R_k**3) times particle surface 4 pi R_k**2, that is sum(3 * eps_k / R_k), the sum of the per-phase ratios the model already defines. The old expression took a ratio of sums, weighting each phase by its volume fraction instead of its number density, and divided by 3 where it should multiply: for Chen2020's graphite split into two identical halves it gave 21 331 m-1 instead of 383 959 m-1 (pybamm-team#5802). The total sets the double-layer charging under "surface form": "differential", so that split differed from the one-phase electrode by 7e-5 in voltage at 1C; it now agrees to ~1e-9 for SPM, SPMe and DFN. The new test checks the total and the voltage for all three models; it fails without the fix. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs: link the surface-area fix's CHANGELOG entry to its PR Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test: unit tests for the total surface area to volume ratio of composite electrodes Lifted from pybamm-team#5803 by @shaurya20335. Checks two distinct phases against 3 eps_1 / R_1 + 3 eps_2 / R_2, and that splitting the Chen2020 negative electrode into two identical phases keeps the single-phase total. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: Shaurya Shukla <shauryabhushanshukla@gmail.com> Co-authored-by: rtimms <rob@ionworks.com>
Move the Unreleased entries under v26.10.0.0 and bump CITATION.cff. Move the pybamm-team#5781, pybamm-team#5784, pybamm-team#5813 and pybamm-team#5817 entries out of v26.9.0.0, which was tagged before they merged, and add the missing pybammsolvers>=0.11.0 breaking change. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ity fixes (pybamm-team#5882) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…-team#5872) * fix: give Marcus kinetics the right sign and consistent units In the Marcus rate law the oxidation rate goes with (lambda - e*eta)^2 and the reduction rate with (lambda + e*eta)^2. The two were swapped, so the current was negative for a positive overpotential. The reorganization energy, in eV, was also used as a bare number next to RT in J/mol, so the current depended on the unit system, and models with Marcus kinetics failed to solve. Nondimensionalise lambda and eta with F/RT, as MarcusHushChidsey does, and put the terms the right way round. Add a unit test against the closed-form rate law and an integration test that solves SPM, SPMe, DFN and Newman-Tobias with Marcus kinetics. Fixes pybamm-team#5870 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * add changelog entry for pybamm-team#5872 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* feat: add Python 3.15 support to pybamm and pybammsolvers Raise requires-python to <3.16 in pybamm, pybammsolvers and pybamm-model-zoo, add the 3.15 trove classifier, and re-lock the workspace over the widened range. The zoo must move too: uv locks a workspace to the intersection of its members' ranges. CI installs from uv.lock, so the locked packages with no cp315 wheels (numpy, scipy, pandas, matplotlib, contourpy, kiwisolver, cffi, markupsafe, pydantic-core, rpds-py, watchfiles, memray, pywinpty) are upgraded; otherwise every 3.15 leg builds numpy and scipy from source. The jax and vtk extras skip 3.15: no jaxlib inside the jax<0.9.0 pin has cp315 wheels, and vtk publishes neither cp315 wheels nor an sdist. CI: cibuildwheel already targets cp315, so only the cp315t skip is added (CasADi has no free-threaded build; its cp311-abi3 wheels cover 3.15). 3.15 becomes the max endpoint of the gated unit-test matrix on Linux x86/arm and macOS and of the advisory Windows job, with 3.14 moving to the nightly sweep alongside the other interior versions. setup-python allows prereleases until 3.15.0 is published. * fix: lock xarray 2026.9.0 for numpy 2.5 numpy 2.5 deprecates generic-unit timedeltas, and xarray before 2026.04.0 builds one at import, so the model zoo suite (whose filterwarnings is a bare "error") failed to load its conftest. * ci: drop setup-python prereleases now that 3.15.0 is out 3.15.0 is in the setup-python manifest, so the 3.15 legs resolve to the final release without allow-prereleases. * feat: allow jax up to 0.11 so the jax extra installs on Python 3.15 jaxlib publishes cp315 wheels only from 0.11.1, so widen the jax extra to <0.12.0 and drop its python_version < '3.15' marker. JAX 0.11 needs Python 3.12, so uv forks the lock: 3.11 gets jax 0.10.2 and 3.12-3.15 get 0.11.2. JAX 0.11 removed APIs the solvers used: - The BDF solver's closure conversion relied on jax.core.get_aval, lu.transformation and pe.trace_to_jaxpr_dynamic. Use the public jax.closure_convert (also what jax.experimental.ode uses) and a plain closure for ravel_first_arg. Drop the unused _check_arg, which called the removed core.valid_jaxtype. - IDAKLU-JAX's JVP used lax.zeros_like_array; use jnp.zeros_like. Newer JAX also hands a primitive's eager implementation a scalar time as a 0-d np.ndarray rather than np.float64, which ProcessedVariable indexed with t_observe[0]. Treat a 0-d array like a scalar. The jax tests pass on jax 0.7.0, 0.8.3, 0.10.2 and 0.11.2.
…ad of being added (pybamm-team#5886) * Add strict=True to ParameterValues.update so unknown keys raise (pybamm-team#5874) With strict=True every key must already exist; all keys are checked before any is written, and the KeyError lists the closest existing names from FuzzyDict.get_best_matches. The default is unchanged and still adds new parameters. * Run the strict check after list values are expanded check_parameter_values expands a list value into indexed keys, so the check on raw keys rejected a valid strict update of an existing list parameter. The check now runs on the expanded keys, still before any write. A name with no close match is reported without an empty match list. * Reject an unknown name with an empty list under strict=True An empty list expands to no indexed keys, so the strict check never saw the name. The name itself, or its first indexed key, must now exist. --------- Co-authored-by: Saltwater Energy Solutions <266172925+saltwater-energy-solutions@users.noreply.github.com>
updates: - [github.com/astral-sh/ruff-pre-commit: v0.16.10 → v0.17.0](astral-sh/ruff-pre-commit@v0.16.10...v0.17.0) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
NicolaCourtier
merged commit Oct 13, 2026
98455d5
into
NicolaCourtier:step-cycle-numbers
39 of 41 checks passed
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
17 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
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
Fixes # (issue)
Type of change
Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #)
Important checks:
Please confirm the following before marking the PR as ready for review:
nox -s pre-commitnox -s testsnox -s doctests