From 47e87fd0d022d48feebc3dd4907a7a0a8cb9909f Mon Sep 17 00:00:00 2001 From: Alexander Bills Date: Fri, 31 Jul 2026 16:02:06 -0700 Subject: [PATCH] deprecate: replace Magnitude with Component Emit DeprecationWarning from Magnitude and migrate internal 2D call sites to Component. Teach FiniteVolume2D edge-averaging to read Component indices so the replacement works for flux products. Co-authored-by: Cursor --- CHANGELOG.md | 4 ++ .../pybamm/expression_tree/unary_operators.py | 15 ++++++- .../lithium_ion/basic_dfn_2d.py | 4 +- .../spatial_methods/finite_volume_2d.py | 16 +++++++- .../test_vector_field_and_magnitude.py | 40 ++++++++++++++----- .../test_solvers/test_processed_variable.py | 4 +- .../test_2d_finite_volume_integration.py | 2 +- .../test_finite_volume_2d.py | 4 +- 8 files changed, 68 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2eb6934c2e..539a32e415 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ - Generalised `VectorField` to N components and added `Component`/`Norm` operators for multi-dimensional vector fields. - Added unstructured mesh support (`UnstructuredSubMesh`, generators, and interface coupling) for arbitrary 2D/3D domains. +## Breaking changes + +- `pybamm.Magnitude` is deprecated; use `pybamm.Component(symbol, 0)` / `pybamm.Component(symbol, 1)` for the lr/tb components of a `VectorField`. + ## Bug fixes - `BatchStudy.solve` no longer ignores its `solver` argument: previously the loop over study inputs shadowed it, so a caller-supplied solver was silently dropped. A solver from `BatchStudy(solvers=...)` still takes precedence. ([#5677](https://github.com/pybamm-team/PyBaMM/pull/5677)) diff --git a/packages/pybamm/src/pybamm/expression_tree/unary_operators.py b/packages/pybamm/src/pybamm/expression_tree/unary_operators.py index be3f2df585..d70cafae4b 100644 --- a/packages/pybamm/src/pybamm/expression_tree/unary_operators.py +++ b/packages/pybamm/src/pybamm/expression_tree/unary_operators.py @@ -3,6 +3,8 @@ # from __future__ import annotations +import warnings + import casadi import numpy as np import numpy.typing as npt @@ -1486,10 +1488,21 @@ def _unary_new_copy(self, child, perform_simplifications=True): class Magnitude(UnaryOperator): """ - A node in the expression tree representing the magnitude of a vector field. + Extract a directional component from a 2D :class:`VectorField`. + + .. deprecated:: + Use :class:`Component` with index ``0`` (``"lr"``) or ``1`` (``"tb"``) + instead. ``Magnitude`` will be removed in a future release. """ def __init__(self, child, direction): + warnings.warn( + "pybamm.Magnitude is deprecated and will be removed in a future " + "release. Use pybamm.Component(symbol, 0) for the lr/x component " + "or pybamm.Component(symbol, 1) for the tb/z component.", + DeprecationWarning, + stacklevel=2, + ) super().__init__("magnitude" + f"({direction})", child) self.direction = direction diff --git a/packages/pybamm/src/pybamm/models/full_battery_models/lithium_ion/basic_dfn_2d.py b/packages/pybamm/src/pybamm/models/full_battery_models/lithium_ion/basic_dfn_2d.py index 94baa22ca8..489b066073 100644 --- a/packages/pybamm/src/pybamm/models/full_battery_models/lithium_ion/basic_dfn_2d.py +++ b/packages/pybamm/src/pybamm/models/full_battery_models/lithium_ion/basic_dfn_2d.py @@ -364,8 +364,8 @@ def __init__(self, name="Doyle-Fuller-Newman model"): "Negative electrode ocp [V]": self.param.n.prim.U(sto_surf_n, T), "Positive electrode current density [A.m-2]": j_p, "Negative electrode current density [A.m-2]": j_n, - "Electrolyte flux X-component [mol.m-2.s-1]": pybamm.Magnitude(N_e, "lr"), - "Electrolyte flux Z-component [mol.m-2.s-1]": pybamm.Magnitude(N_e, "tb"), + "Electrolyte flux X-component [mol.m-2.s-1]": pybamm.Component(N_e, 0), + "Electrolyte flux Z-component [mol.m-2.s-1]": pybamm.Component(N_e, 1), "Positive solid lithium [mol]": solid_lithium_positive, "Negative solid lithium [mol]": solid_lithium_negative, "Total solid lithium [mol]": total_solid_lithium, diff --git a/packages/pybamm/src/pybamm/spatial_methods/finite_volume_2d.py b/packages/pybamm/src/pybamm/spatial_methods/finite_volume_2d.py index 475ddec514..b77b49f603 100644 --- a/packages/pybamm/src/pybamm/spatial_methods/finite_volume_2d.py +++ b/packages/pybamm/src/pybamm/spatial_methods/finite_volume_2d.py @@ -59,6 +59,18 @@ class FiniteVolume2D(pybamm.SpatialMethod): def __init__(self, options=None): super().__init__(options) + @staticmethod + def _edge_direction(symbol): + """lr/tb direction for edge-averaging binary-op children.""" + if isinstance(symbol, pybamm.Component): + try: + return ("lr", "tb")[symbol.index] + except IndexError as err: + raise ValueError( + f"Component index {symbol.index} is not a 2D lr/tb direction" + ) from err + return symbol.direction + def build(self, mesh): super().build(mesh) @@ -2169,7 +2181,7 @@ def process_binary_operators(self, bin_op, left, right, disc_left, disc_right): # binary operator represents a flux) elif left_evaluates_on_edges and not right_evaluates_on_edges: method = "arithmetic" - direction = left.direction + direction = self._edge_direction(left) disc_right = self.node_to_edge( disc_right, method=method, direction=direction ) @@ -2179,7 +2191,7 @@ def process_binary_operators(self, bin_op, left, right, disc_left, disc_right): # binary operator represents a flux) elif right_evaluates_on_edges and not left_evaluates_on_edges: method = "arithmetic" - direction = right.direction + direction = self._edge_direction(right) disc_left = self.node_to_edge(disc_left, method=method, direction=direction) # Return new binary operator with appropriate class diff --git a/packages/pybamm/tests/unit/test_expression_tree/test_vector_field_and_magnitude.py b/packages/pybamm/tests/unit/test_expression_tree/test_vector_field_and_magnitude.py index f1f798775a..f164b7dc08 100644 --- a/packages/pybamm/tests/unit/test_expression_tree/test_vector_field_and_magnitude.py +++ b/packages/pybamm/tests/unit/test_expression_tree/test_vector_field_and_magnitude.py @@ -4,7 +4,7 @@ class TestVectorFieldAndMagnitude: - def test_vector_field_and_magnitude(self, mesh_2d): + def test_vector_field_and_component(self, mesh_2d): mesh = mesh_2d spatial_methods = { "macroscale": pybamm.FiniteVolume2D(), @@ -17,18 +17,18 @@ def test_vector_field_and_magnitude(self, mesh_2d): one = pybamm.Constant(1, "one") vf_plus_one = vector_field + one one_plus_vf = one + vector_field - magnitude_lr = pybamm.Magnitude(vector_field, "lr") - magnitude_tb = pybamm.Magnitude(vector_field, "tb") + component_lr = pybamm.Component(vector_field, 0) + component_tb = pybamm.Component(vector_field, 1) negative_vf = -vector_field vf_processed = disc.process_symbol(vector_field) vf_plus_one_processed = disc.process_symbol(vf_plus_one) one_plus_vf_processed = disc.process_symbol(one_plus_vf) - magnitude_lr_processed = disc.process_symbol(magnitude_lr) - magnitude_tb_processed = disc.process_symbol(magnitude_tb) + component_lr_processed = disc.process_symbol(component_lr) + component_tb_processed = disc.process_symbol(component_tb) negative_vf_processed = disc.process_symbol(negative_vf) - assert magnitude_lr_processed.evaluate() == 1 - assert magnitude_tb_processed.evaluate() == 2 + assert component_lr_processed.evaluate() == 1 + assert component_tb_processed.evaluate() == 2 assert vf_plus_one_processed == pybamm.VectorField( pybamm.Scalar(2), pybamm.Scalar(3) ) @@ -42,8 +42,8 @@ def test_vector_field_and_magnitude(self, mesh_2d): ) assert vf_processed == pybamm.VectorField(pybamm.Scalar(1), pybamm.Scalar(2)) - with pytest.raises(ValueError, match=r"applied to a vector field"): - disc.process_symbol(pybamm.Magnitude(pybamm.Scalar(1), "lr")) + with pytest.raises(ValueError, match=r"applied to a VectorField"): + disc.process_symbol(pybamm.Component(pybamm.Scalar(1), 0)) assert negative_vf_processed == pybamm.VectorField( pybamm.Scalar(-1), pybamm.Scalar(-2) @@ -60,7 +60,25 @@ def test_vector_field_and_magnitude(self, mesh_2d): with pytest.raises(ValueError, match=r"must either"): vf_evaluates_on_edges.evaluates_on_edges("primary") - assert magnitude_lr.new_copy([vector_field]) == magnitude_lr + assert component_lr.new_copy([vector_field]) == component_lr + + def test_magnitude_is_deprecated(self, mesh_2d): + mesh = mesh_2d + disc = pybamm.Discretisation(mesh, {"macroscale": pybamm.FiniteVolume2D()}) + vector_field = pybamm.VectorField(pybamm.Scalar(1), pybamm.Scalar(2)) + + with pytest.warns(DeprecationWarning, match="Magnitude is deprecated"): + magnitude_lr = pybamm.Magnitude(vector_field, "lr") + with pytest.warns(DeprecationWarning, match="Magnitude is deprecated"): + magnitude_tb = pybamm.Magnitude(vector_field, "tb") + + assert disc.process_symbol(magnitude_lr).evaluate() == 1 + assert disc.process_symbol(magnitude_tb).evaluate() == 2 + + with pytest.raises(ValueError, match=r"applied to a vector field"): + with pytest.warns(DeprecationWarning, match="Magnitude is deprecated"): + disc.process_symbol(pybamm.Magnitude(pybamm.Scalar(1), "lr")) with pytest.raises(ValueError, match=r"Invalid direction"): - disc.process_symbol(pybamm.Magnitude(vector_field, "asdf")) + with pytest.warns(DeprecationWarning, match="Magnitude is deprecated"): + disc.process_symbol(pybamm.Magnitude(vector_field, "asdf")) diff --git a/packages/pybamm/tests/unit/test_solvers/test_processed_variable.py b/packages/pybamm/tests/unit/test_solvers/test_processed_variable.py index 82c359cb9d..f9fae7512d 100644 --- a/packages/pybamm/tests/unit/test_solvers/test_processed_variable.py +++ b/packages/pybamm/tests/unit/test_solvers/test_processed_variable.py @@ -405,7 +405,7 @@ def test_processed_variable_2D_fvm(self, mesh_2d): y_sol.reshape(processed_var.entries.shape), ) - var_edges_tb = pybamm.Magnitude(pybamm.grad(var), "tb") + var_edges_tb = pybamm.Component(pybamm.grad(var), 1) disc.bcs = { var: { "left": (pybamm.Scalar(0), "Dirichlet"), @@ -433,7 +433,7 @@ def test_processed_variable_2D_fvm(self, mesh_2d): ), ) - var_edges_lr = pybamm.Magnitude(pybamm.grad(var), "lr") + var_edges_lr = pybamm.Component(pybamm.grad(var), 0) disc.bcs = { var: { "left": (pybamm.Scalar(0), "Dirichlet"), diff --git a/packages/pybamm/tests/unit/test_spatial_methods/test_finite_volume_2d/test_2d_finite_volume_integration.py b/packages/pybamm/tests/unit/test_spatial_methods/test_finite_volume_2d/test_2d_finite_volume_integration.py index 2ba72e88a4..05770f75f3 100644 --- a/packages/pybamm/tests/unit/test_spatial_methods/test_finite_volume_2d/test_2d_finite_volume_integration.py +++ b/packages/pybamm/tests/unit/test_spatial_methods/test_finite_volume_2d/test_2d_finite_volume_integration.py @@ -256,7 +256,7 @@ def test_boundary_integral(self, mesh_2d): } k = pybamm.VectorField(pybamm.Scalar(1 + 1e-5), pybamm.Scalar(1 + 1e-5)) disc.bcs = bcs - symbol = pybamm.Magnitude(k * pybamm.Gradient(var) / pybamm.Scalar(-5), "tb") + symbol = pybamm.Component(k * pybamm.Gradient(var) / pybamm.Scalar(-5), 1) boundary_integral_left = pybamm.BoundaryIntegral(symbol, region="bottom") # Fix this test boundary_integral_left_disc = disc.process_symbol(boundary_integral_left) diff --git a/packages/pybamm/tests/unit/test_spatial_methods/test_finite_volume_2d/test_finite_volume_2d.py b/packages/pybamm/tests/unit/test_spatial_methods/test_finite_volume_2d/test_finite_volume_2d.py index 437cf349d7..122cee2267 100644 --- a/packages/pybamm/tests/unit/test_spatial_methods/test_finite_volume_2d/test_finite_volume_2d.py +++ b/packages/pybamm/tests/unit/test_spatial_methods/test_finite_volume_2d/test_finite_volume_2d.py @@ -310,8 +310,8 @@ def test_process_binary_operators(self, mesh_2d): eqn_disc.evaluate(None, TB.flatten()) for eqn in [ - var * pybamm.Magnitude(pybamm.grad(var), "lr"), - pybamm.Magnitude(pybamm.grad(var), "lr") * var, + var * pybamm.Component(pybamm.grad(var), 0), + pybamm.Component(pybamm.grad(var), 0) * var, ]: eqn_disc = disc.process_symbol(eqn) eqn_disc.evaluate(None, LR.flatten())