[Bug]: Solve time returns wrong value if not all cycles are saved

[brosaplanella]

PyBaMM Version

22.10

Python Version

3.9

Describe the bug

The solution.solve_time.value returns different values depending on whether all cycles are saved or only one. I suspect it only returns the time for the saved cycles.

Steps to Reproduce

import pybamm

model = pybamm.lithium_ion.SPM()

experiment = pybamm.Experiment(
    [
        "Discharge at 1C until 2.5 V",
        "Charge at C/3 until 4.2 V",
        "Hold at 4.2 V until C/20",
    ] * 20
)

param = pybamm.ParameterValues("Chen2020")

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

sim.solve()
print(sim.solution.solve_time.value)

sim.solve(save_at_cycles=[1])
print(sim.solution.solve_time.value)

Relevant log output

2.047022205000019
0.0252866289999929

[brosaplanella]

I didn't address this as part of #3101, as it is a different issue. Basically, the total time is computed when adding the solutions together (each solution has its own solving time, which also gets added when adding two solutions). The problem is that when a solution is not saved, the solve time is not saved and thus not added. An easy way here would be to add a solve_time variable to EmptySolution and check that when we do not save a cycle we store it as an EmptySolution (rather than as a None).

A further improvement would be to allow EmptySolution to also store a y (which would be the initial y of the cycle) so we can get rid of all_first_states.

Note: bumping it to priority:medium as I think it would be good to fix that if there is time.

[RuiheLi]

I am interested in this one as I also found similar problems, especially when EmptySolution is presented, and that will further affect the calculation of throughput capacity as well. May I work on this one?

[RuiheLi]

I am wondering instead of adding something to EmptySolution to make it actually not empty, will it be a good idea to add another variable, so apart from solve_time , we have solve_time_tot to do the accumulation? @brosaplanella

[brosaplanella]

I don't remember the exact details as I haven't checked this for a while, but according to my previous comment the easiest way should be to:

Basically, the total time is computed when adding the solutions together (each solution has its own solving time, which also gets added when adding two solutions). The problem is that when a solution is not saved, the solve time is not saved and thus not added. An easy way here would be to add a solve_time variable to EmptySolution and check that when we do not save a cycle we store it as an EmptySolution (rather than as a None).

Maybe this is a bit naïve, but might be worth checking if this fix works first, before trying more complex stuff.