[Bug]: Unable to save time data from thevenin model solution

[raghuramshankar]

PyBaMM Version

23.5

Python Version

3.9.17

Describe the bug

Unable to save time data when simulating an equivalent circuit model and saving the solution externally. More specifically, Time [h] is expected as a field available in the solution object, but it throws an error when trying to access the same.

Seems to work fine for SPMe and DFN solution objects.

Steps to Reproduce

Running run_ecm.py and adding code to save the solution:

# %%
import pybamm

pybamm.set_logging_level("INFO")

model = pybamm.equivalent_circuit.Thevenin()

experiment = pybamm.Experiment(
    [
        (
            "Discharge at C/10 for 10 hours or until 3.3 V",
            "Rest for 30 minutes",
            "Rest for 2 hours",
            "Charge at 100 A until 4.1 V",
            "Hold at 4.1 V until 5 A",
            "Rest for 30 minutes",
            "Rest for 1 hour",
        ),
    ]
)

sim = pybamm.Simulation(model, experiment=experiment)

# save solution
solution = sim.solve()
quick_plot = pybamm.QuickPlot(solution)
quick_plot.dynamic_plot()

print(solution.data.keys())

solution.save_data(
    "data.csv", ["Time [h]", "Current [A]", "Voltage [V]"], to_format="csv"
)

Relevant log output

KeyError                                  Traceback (most recent call last)
File ~/Developer/PyBaMM/pybamm/util.py:58, in FuzzyDict.__getitem__(self, key)
     57 try:
---> 58     return super().__getitem__(key)
     59 except KeyError:

KeyError: 'Time [h]'

During handling of the above exception, another exception occurred:

KeyError                                  Traceback (most recent call last)
/Users/raghuramshankar/Developer/PyBaMM/examples/scripts/run_ecm.py in line 31
     27 quick_plot.dynamic_plot()
     29 print(solution.data.keys())
---> 31 solution.save_data(
     32     "data.csv", ["Time [h]", "Current [A]", "Voltage [V]"], to_format="csv"
     33 )

File ~/Developer/PyBaMM/pybamm/solvers/solution.py:634, in Solution.save_data(self, filename, variables, to_format, short_names)
    603 def save_data(
    604     self, filename=None, variables=None, to_format="pickle", short_names=None
    605 ):
    606     """
    607     Save solution data only (raw arrays)
...
     92             f"'{key}' not found. Use the dimensional version '{k}' instead."
     93         )
---> 94 raise KeyError(f"'{key}' not found. Best matches are {best_matches}")

KeyError: "'Time [h]' not found. Best matches are ['Resistance [Ohm]', 'R1 [Ohm]', 'R0 [Ohm]']"

[rtimms]

This is because in the current implementation Thevenin subclasses pybamm.BaseModel and not pybamm.BaseBattery Model. Right now, pybamm.BaseBattery Model contains a lot of physics-based model specific stuff, so in the long run we should make this class more general.

For now we should add a set_standard_output_variables method to Thevenin (like this one) to add some standard outputs. Or make a pybamm.BaseEquivalentCircuitModel if we anticipate adding more ECMs.