Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass array of times to step #3524

Closed
rtimms opened this issue Nov 15, 2023 · 8 comments · Fixed by #3627
Closed

Pass array of times to step #3524

rtimms opened this issue Nov 15, 2023 · 8 comments · Fixed by #3627
Assignees
Labels
difficulty: medium Will take a few days priority: medium To be resolved if time allows

Comments

@rtimms
Copy link
Contributor

rtimms commented Nov 15, 2023

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...

@rtimms rtimms added difficulty: medium Will take a few days priority: medium To be resolved if time allows labels Nov 15, 2023
@prady0t
Copy link
Contributor

prady0t commented Nov 22, 2023

Hey I would like to work on this.

@prady0t
Copy link
Contributor

prady0t commented Nov 25, 2023

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
Copy link
Contributor Author

rtimms commented Dec 7, 2023

Sorry for the slow reply. Take a look here.

@prady0t
Copy link
Contributor

prady0t commented Dec 7, 2023

Thanks 😄

@prady0t
Copy link
Contributor

prady0t commented Dec 16, 2023

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
Copy link
Member

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
Copy link
Contributor

prady0t commented Dec 16, 2023

Sure will do!

@prady0t
Copy link
Contributor

prady0t commented Dec 17, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: medium Will take a few days priority: medium To be resolved if time allows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants