Fix #5018: exclude internal 'start time' InputParameter from termination check

CHANGELOG.md

@@ -43,6 +43,7 @@
 - Fixed the `pybammsolvers` source distribution bundling the SUNDIALS/SuiteSparse submodule trees (~291 MB, over PyPI's per-file limit) when built from a checkout with the submodules initialised; the sdist now excludes them. ([#5623](https://github.com/pybamm-team/PyBaMM/pull/5623))
 - Fixed the `pybammsolvers` editable auto-rebuild failing to configure when the SUNDIALS/SuiteSparse submodules are absent even though the libraries were already built in `.idaklu`; the from-source bootstrap (and its submodule requirement) is now skipped once the libraries exist. ([#5623](https://github.com/pybamm-team/PyBaMM/pull/5623))
 - Fixed the Read the Docs documentation build, which broke once `pybammsolvers` became a workspace member with no published wheel during the release: RTD now fetches the SUNDIALS/SuiteSparse submodules and installs `gfortran`/`libopenblas` so the solver builds from source. Link checking was also restructured so transient external-link failures no longer block builds: Sphinx `linkcheck` is advisory, and the CI `lychee` check runs offline on PRs with a weekly external sweep that files a tracking issue. ([#5659](https://github.com/pybamm-team/PyBaMM/pull/5659))
+- Fixed `pybamm.step.current`/`voltage`/`power`/`resistance` rejecting operator-free string terminations when the step value was a Python callable. The internal `InputParameter("start time")` is now excluded from the termination parameter check. ([#5018](https://github.com/pybamm-team/PyBaMM/issues/5018))
 
 # [v26.6.2.0](https://github.com/pybamm-team/PyBaMM/tree/v26.6.2.0) - 2026-06-16
 

packages/pybamm/src/pybamm/experiment/step/base_step.py

@@ -758,9 +758,27 @@ def _parse_termination(term_str, value):
 
 
 def _check_input_params(value):
-    """Check if self.value is a function of input parameters"""
+    """Check if ``value`` depends on any user-supplied ``InputParameter``.
+
+    ``BaseStep`` injects an internal ``InputParameter("start time")``
+    into ``self.value`` whenever the step value is a Python function or
+    a drive cycle (see the assignments above to ``self.value``). That
+    placeholder is an implementation detail — it is solved-in at
+    ``simulation`` time via :attr:`Simulation._START_TIME_INPUT` and is
+    not user-controllable. Treating it as a real ``InputParameter``
+    here misclassifies every Python-function step as
+    "uses InputParameter", which then incorrectly forces an operator on
+    plain string terminations like ``"4.3 V"`` (issue #5018) and turns
+    off direction inference for ``pybamm.step.current(callable, ...)``.
+
+    Genuine user InputParameters (any other name) still trigger the
+    check, so existing behaviour for symbolic values like
+    ``pybamm.InputParameter("I_app")`` or ``I_coeff * pybamm.t`` is
+    preserved.
+    """
     leaves = value.post_order(filter=lambda node: len(node.children) == 0)
     contains_input_parameter = any(
-        isinstance(leaf, pybamm.InputParameter) for leaf in leaves
+        isinstance(leaf, pybamm.InputParameter) and leaf.name != "start time"
+        for leaf in leaves
     )
     return contains_input_parameter

packages/pybamm/tests/unit/test_experiments/test_base_step.py

@@ -133,3 +133,96 @@ def test_parse_termination_requires_operator_for_input_parameter():
         pybamm.experiment.step.base_step._parse_termination(
             "2A", pybamm.InputParameter("I_app")
         )
+
+
+def _t_dependent_current(t):
+    """A Python-function current that genuinely depends on ``t`` so that
+    ``BaseStep`` cannot fold ``value(t)`` into a scalar at construction.
+    Module-scope so steps stay picklable (used elsewhere in this file)."""
+    return -1.0 + 0.1 * np.sin(2 * np.pi * t)
+
+
+def test_check_input_params_ignores_internal_start_time():
+    """Regression for #5018. ``BaseStep`` injects
+    ``InputParameter("start time")`` into ``self.value`` whenever the
+    step is built from a Python function (or a drive cycle). That
+    placeholder is an implementation detail — it is not a user-supplied
+    ``InputParameter`` — so ``_check_input_params`` must return False
+    for a value tree that only depends on it."""
+    step = pybamm.step.current(_t_dependent_current, period="0.1 seconds")
+    # Sanity check that the internal "start time" placeholder is in the
+    # symbol tree, so the test exercises the intended code path.
+    leaves = step.value.post_order(filter=lambda n: len(n.children) == 0)
+    names = {leaf.name for leaf in leaves if isinstance(leaf, pybamm.InputParameter)}
+    assert names == {"start time"}
+    assert pybamm.experiment.step.base_step._check_input_params(step.value) is False
+
+
+def test_check_input_params_still_detects_user_input_parameter():
+    """Counterpart to the test above: ``_check_input_params`` must keep
+    returning True for value trees that mix the internal ``"start time"``
+    placeholder with a genuine user ``InputParameter``. Forces the new
+    filter to be ``name != "start time"`` rather than a blanket skip."""
+    I_app = pybamm.InputParameter("I_app")
+    # Manually build a tree that contains both leaves to make the
+    # mixing explicit.
+    value = I_app + (pybamm.t - pybamm.InputParameter("start time"))
+    assert pybamm.experiment.step.base_step._check_input_params(value) is True
+
+
+def test_python_function_step_accepts_string_termination_without_operator():
+    """End-to-end regression for #5018: ``pybamm.step.current(callable)``
+    used to raise ``ValueError: Termination must include an operator
+    when using InputParameter.`` whenever the callable depended on
+    ``t``, because ``_check_input_params`` saw the internal
+    ``InputParameter("start time")`` and assumed the user had passed
+    one. After the fix, a plain ``"4.3 V"`` (and other operator-free
+    string terminations) must be accepted again."""
+    step = pybamm.step.current(
+        _t_dependent_current,
+        period="0.05 seconds",
+        termination=["4.3 V"],
+    )
+    assert len(step.termination) == 1
+    assert step.termination[0] == pybamm.step.VoltageTermination(4.3)
+
+
+def test_python_function_step_accepts_mixed_terminations():
+    """Pins the exact shape of the OP's failing experiment in #5018:
+    a ``CustomTermination`` object together with a plain string
+    termination, around a ``t``-dependent callable current."""
+
+    def soc_cutoff(variables):
+        return variables["Discharge capacity [A.h]"] + 4.5
+
+    soc_termination = pybamm.step.CustomTermination(
+        name="SOC 90%", event_function=soc_cutoff
+    )
+    step = pybamm.step.current(
+        _t_dependent_current,
+        period="0.05 seconds",
+        termination=[soc_termination, "4.3 V"],
+    )
+    assert len(step.termination) == 2
+    assert step.termination[1] == pybamm.step.VoltageTermination(4.3)
+
+
+def test_python_function_step_infers_direction_from_value_at_zero():
+    """After #5018's fix, ``_check_input_params`` no longer flags
+    Python-function values as "depends on InputParameter", so
+    ``value_based_charge_or_discharge`` falls through to evaluating the
+    symbolic value at ``t=0`` (with the internal ``"start time"``
+    placeholder set to 0). The sign of the result then drives
+    direction inference, restoring the pre-#4826 behaviour for
+    callables. ``_t_dependent_current(0) = -1.0`` ⇒ charge."""
+    step = pybamm.step.current(_t_dependent_current, period="0.05 seconds")
+    assert step.direction == "charge"
+
+
+def test_user_input_parameter_step_still_requires_operator():
+    """Belt-and-braces non-regression: the ``InputParameter`` →
+    "must include an operator" guard introduced by #4826 must keep
+    firing when the user genuinely passes one. Without this we would
+    silently weaken the check."""
+    with pytest.raises(ValueError, match="Termination must include an operator"):
+        pybamm.step.current(pybamm.InputParameter("I_app"), termination="2.5 V")