pybamm-team / PyBaMM

Fast and flexible physics-based battery models in Python
https://www.pybamm.org/
BSD 3-Clause "New" or "Revised" License
1.11k stars 547 forks source link

Pass array of times to `step` #3524

Closed rtimms closed 8 months ago

rtimms commented 1 year ago

When you call solver.step you pass in dt, the time time increment you want to step forward in. You can also pass npts to get the solution back at a linearly spaced array of times between the initial time, t0, and t0+dt. It would be good to instead be able to pass an array of times, t_eval, like you pass to solve, so you can get step solutions returned at non-linearly spaced times.

The motivation for this is that right now you can't pass a starting_solution to Simualtion.solve if you aren't using an experiment, so if you want to chain solutions together you have to call Simulation.step instead.

An alternative is that we allow starting solutions without experiments in Simulation.solve.

The whole logic for step and solve is pretty clunky both in Simulation and BaseSolver, but that's a separate issue...

prady0t commented 12 months ago

Hey I would like to work on this.

prady0t commented 11 months ago

Can you please give examples of how to use step() function to get solutions which I can use for debugging? I'm quite new here and I cant find it in project doc.

rtimms commented 11 months ago

Sorry for the slow reply. Take a look here.

prady0t commented 11 months ago

Thanks 😄

prady0t commented 11 months ago

Maybe if we change step function to something like:

    def step(
        self,
        old_solution,
        model,
        dt=0,
        non_linear_time=None,
        npts=2,
        inputs=None,
        save=True,
    )

Here non_linear_time is an array of time (example: [0,10,15,17,20]). We can assign dt = non_linear_time[i] - non_linear_time[i-1] inside a loop and make changes to the function accordingly. Is this a correct solution?

arjxn-py commented 11 months ago

Here non_linear_time is an array of time (example: [0,10,15,17,20]).

Hi @prady0t, theoretically it looks promising, though it would be nicer to open a PR to keep things moving & also would be easier for us to suggest changes there 🙂

prady0t commented 11 months ago

Sure will do!

prady0t commented 11 months ago

Here non_linear_time is an array of time (example: [0,10,15,17,20]).

Hi @prady0t, theoretically it looks promising, though it would be nicer to open a PR to keep things moving & also would be easier for us to suggest changes there 🙂

Made a PR. Please have a look.