Repository: pybamm-team/PyBaMM (python)

Issue:
Use InputParameter in pybamm experiment
### Description

Allow using input parameters in a pybamm experiment, e.g. 
`pybamm.step.current(pybamm.InputParameter("I_app"), termination="2.5V")`

### Motivation

Firstly, this allows faster sweeping over different conditions

More importantly, we would be able to set up simulations to re-use steps instead of re-discretizing each step. Currently, it does avoid re-discretizing steps if they're exactly the same, but there are some edge cases where it incorrectly thinks steps are different. It also rediscretizes steps even if they have the same structure but different values, which should not be necessary (e.g. "Discharge at 1C until 2.5V" and "Charge at 2C until 4.2V" have the same model structure and should not need to be discretized twice).



### Possible Implementation

Create relevant input parameters when building the simulation experiment, then pass in the appropriate values as inputs at solve time

### Additional context

_No response_

Pull request description:
Added InputParameter support in PyBamm experiments
# Description

Added InputParameter support in experiments, now you can pass them something like this:
```python
current_step = pybamm.step.current(1, input_parameters=pybamm.InputParameter("I_app"), termination="2.5 V")
experiment = pybamm.Experiment(
    input_parameters={"I_app": 1},
    termination="2.5 V",
    operating_conditions=[current_step],
)
```

Fixes #4799 

## Type of change

Please add a line in the relevant section of [CHANGELOG.md](https://github.com/pybamm-team/PyBaMM/blob/develop/CHANGELOG.md) to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.

- [x] New feature (non-breaking change which adds functionality)
- [ ] Optimization (back-end change that speeds up the code)
- [ ] Bug fix (non-breaking change which fixes an issue)

# Key checklist:

- [x] No style issues: `$ pre-commit run` (or `$ nox -s pre-commit`) (see [CONTRIBUTING.md](https://github.com/pybamm-team/PyBaMM/blob/develop/CONTRIBUTING.md#installing-and-using-pre-commit) for how to set this up to run automatically when committing locally, in just two lines of code)
- [x] All tests pass: `$ python -m pytest` (or `$ nox -s tests`)
- [x] The documentation builds: `$ python -m pytest --doctest-plus src` (or `$ nox -s doctests`)

You can run integration tests, unit tests, and doctests together at once, using `$ nox -s quick`.

## Further checks:

- [ ] Code is commented, particularly in hard-to-understand areas
- [x] Tests added that prove fix is effective or that feature works


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b266bd180..4bf1fe7f9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@
 - Creates a 'calc_esoh' property in battery models ([#4825](https://github.com/pybamm-team/PyBaMM/pull/4825))
 - Added 'get_summary_variables' to return dictionary of computed summary variables ([#4824](https://github.com/pybamm-team/PyBaMM/pull/4824))
 - Added support for particle size distributions combined with particle mechanics. ([#4807](https://github.com/pybamm-team/PyBaMM/pull/4807))
+- Added InputParameter support in PyBamm experiments ([#4826](https://github.com/pybamm-team/PyBaMM/pull/4826))
 
 ## Breaking changes
 
diff --git a/src/pybamm/experiment/step/base_step.py b/src/pybamm/experiment/step/base_step.py
index e6dc1fd88..0895cdfaa 100644
--- a/src/pybamm/experiment/step/base_step.py
+++ b/src/pybamm/experiment/step/base_step.py
@@ -79,9 +79,9 @@ class BaseStep:
                 f"Invalid direction: {direction}. Must be one of {potential_directions}"
             )
         self.input_duration = duration
-        self.input_duration = duration
         self.input_value = value
         self.skip_ok = skip_ok
+
         # Check if drive cycle
         is_drive_cycle = isinstance(value, np.ndarray)
         is_python_function = callable(value)
@@ -152,13 +152,6 @@ class BaseStep:
             self.value = value
             self.period = _convert_time_to_seconds(period)
 
-        if (
-            hasattr(self, "calculate_charge_or_discharge")
-            and self.calculate_charge_or_discharge
-        ):
-            direction = self.value_based_charge_or_discharge()
-        self.direction = direction
-
         self.repr_args, self.hash_args = self.record_tags(
             value,
             duration,
@@ -179,10 +172,20 @@ class BaseStep:
             termination = [termination]
         self.termination = []
         for term in termination:
+            term_obj = None
             if isinstance(term, str):
-                term = _convert_electric(term)
-            term = _read_termination(term)
-            self.termination.append(term)
+                operator, typ, val = _parse_termination(term, self.value)
+                term_obj = _read_termination((operator, typ, val))
+            else:
+                term_obj = _read_termination(term)
+            self.termination.append(term_obj)
+
+        if (
+            hasattr(self, "calculate_charge_or_discharge")
+            and self.calculate_charge_or_discharge
+        ):
+            direction = self.value_based_charge_or_discharge()
+        self.direction = direction
 
         self.temperature = _convert_temperature_to_kelvin(temperature)
 
@@ -389,9 +392,11 @@ class BaseStep:
     def value_based_charge_or_discharge(self):
         """
         Determine whether the step is a charge or discharge step based on the value of the
-        step
+        step. If an operator is provided, the step direction is not used, so we return None.
         """
         if isinstance(self.value, pybamm.Symbol):
+            if _check_input_params(self.value):
+                return None
             inpt = {"start time": 0}
             init_curr = self.value.evaluate(t=0, inputs=inpt).flatten()[0]
         else:
@@ -582,3 +587,32 @@ def _convert_electric(value_string):
             f"units must be 'A', 'V', 'W', 'Ohm', or 'C'. For example: {_examples}"
         ) from error
     return typ, value
+
+
+def _parse_termination(term_str, value):
+    """Parse a termination string into its components"""
+    term_str = term_str.strip()
+    operator = None
+    remaining = term_str
+    # Check if the string starts with '<' or '>'
+    if term_str and term_str[0] in ("<", ">"):
+        operator = term_str[0]
+        remaining = term_str[1:].strip()
+    remaining = remaining.replace(" ", "")
+    typ, val = _convert_electric(remaining)
+    if (
+        isinstance(value, pybamm.Symbol) and _check_input_params(value)
+    ) and operator is None:
+        raise ValueError(
+            "Termination must include an operator when using InputParameter."
+        )
+    return operator, typ, val
+
+
+def _check_input_params(value):
+    """Check if self.value is a function of input parameters"""
+    leaves = value.post_order(filter=lambda node: len(node.children) == 0)
+    contains_input_parameter = any(
+        isinstance(leaf, pybamm.InputParameter) for leaf in leaves
+    )
+    return contains_input_parameter
diff --git a/src/pybamm/experiment/step/step_termination.py b/src/pybamm/experiment/step/step_termination.py
index 06ec5359b..dd4a9f2ed 100644
--- a/src/pybamm/experiment/step/step_termination.py
+++ b/src/pybamm/experiment/step/step_termination.py
@@ -199,9 +199,9 @@ class CustomTermination(BaseTermination):
         return pybamm.Event(self.name, self.event_function(variables))
 
 
-def _read_termination(termination):
+def _read_termination(termination, operator=None):
     if isinstance(termination, tuple):
-        typ, value = termination
+        op, typ, value = termination
     else:
         return termination
 
@@ -210,4 +210,4 @@ def _read_termination(termination):
         "voltage": VoltageTermination,
         "C-rate": CRateTermination,
     }[typ]
-    return termination_class(value)
+    return termination_class(value, operator=op)


Test patch:
diff --git a/tests/integration/test_experiments.py b/tests/integration/test_experiments.py
index e31f09818..687ac5fab 100644
--- a/tests/integration/test_experiments.py
+++ b/tests/integration/test_experiments.py
@@ -89,7 +89,7 @@ class TestExperiments:
     def test_drive_cycle(self):
         drive_cycle = np.array([np.arange(100), 5 * np.ones(100)]).T
         c_step = pybamm.step.current(
-            value=drive_cycle, duration=100, termination=["4.00 V"]
+            value=drive_cycle, duration=100, termination=["< 4.00 V"]
         )
         experiment = pybamm.Experiment(
             [
diff --git a/tests/unit/test_experiments/test_experiment.py b/tests/unit/test_experiments/test_experiment.py
index 2eda78e1e..57f1fac2e 100644
--- a/tests/unit/test_experiments/test_experiment.py
+++ b/tests/unit/test_experiments/test_experiment.py
@@ -5,6 +5,7 @@
 from datetime import datetime
 import pybamm
 import pytest
+import numpy as np
 
 
 class TestExperiment:
@@ -214,3 +215,69 @@ class TestExperiment:
 
         # TODO: once #3176 is completed, the test should pass for
         # operating_conditions_steps (or equivalent) as well
+
+    def test_simulation_solve_updates_input_parameters(self):
+        model = pybamm.lithium_ion.SPM()
+
+        step = pybamm.step.current(
+            pybamm.InputParameter("I_app"),
+            termination="< 2.5 V",
+        )
+        experiment = pybamm.Experiment([step])
+
+        sim = pybamm.Simulation(model, experiment=experiment)
+
+        sim.solve(inputs={"I_app": 1})
+        solution = sim.solution
+
+        current = solution["Current [A]"].entries
+
+        assert np.allclose(current, 1, atol=1e-3)
+
+    def test_current_step_raises_error_without_operator_with_input_parameters(self):
+        pybamm.lithium_ion.SPM()
+        with pytest.raises(
+            ValueError,
+            match="Termination must include an operator when using InputParameter.",
+        ):
+            pybamm.step.current(pybamm.InputParameter("I_app"), termination="2.5 V")
+
+    def test_value_function_with_input_parameter(self):
+        I_coeff = pybamm.InputParameter("I_coeff")
+        t = pybamm.t
+        expr = I_coeff * t
+        step = pybamm.step.current(expr, termination="< 2.5V")
+
+        direction = step.value_based_charge_or_discharge()
+        assert direction is None, (
+            "Expected direction to be None when the expression depends on an InputParameter."
+        )
+
+    def test_symbolic_current_step(self):
+        model = pybamm.lithium_ion.SPM()
+        expr = 2.5 + 0 * pybamm.t
+
+        step = pybamm.step.current(expr, duration=3600)
+        experiment = pybamm.Experiment([step])
+
+        sim = pybamm.Simulation(model, experiment=experiment)
+        sim.solve([0, 3600])
+
+        solution = sim.solution
+        voltage = solution["Current [A]"].entries
+
+        np.testing.assert_allclose(voltage[-1], 2.5, atol=0.1)
+
+    def test_voltage_without_directions(self):
+        model = pybamm.lithium_ion.SPM()
+
+        step = pybamm.step.voltage(2.5, termination="2.5 V")
+        experiment = pybamm.Experiment([step])
+
+        sim = pybamm.Simulation(model, experiment=experiment)
+
+        sim.solve()
+        solution = sim.solution
+
+        voltage = solution["Terminal voltage [V]"].entries
+        assert np.allclose(voltage, 2.5, atol=1e-3)
diff --git a/tests/unit/test_experiments/test_simulation_with_experiment.py b/tests/unit/test_experiments/test_simulation_with_experiment.py
index dfc24ac3e..060fc9109 100644
--- a/tests/unit/test_experiments/test_simulation_with_experiment.py
+++ b/tests/unit/test_experiments/test_simulation_with_experiment.py
@@ -340,7 +340,7 @@ class TestSimulationExperiment:
                 (
                     pybamm.step.current(drive_cycle, temperature="35oC"),
                     pybamm.step.voltage(drive_cycle),
-                    pybamm.step.power(drive_cycle, termination="3 V"),
+                    pybamm.step.power(drive_cycle, termination="< 3 V"),
                 )
             ],
         )