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

Max step size option #2253

Open
rtimms opened this issue Aug 23, 2022 · 13 comments · May be fixed by #4673
Open

Max step size option #2253

rtimms opened this issue Aug 23, 2022 · 13 comments · May be fixed by #4673
Assignees
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours feature priority: medium To be resolved if time allows

Comments

@rtimms
Copy link
Contributor

rtimms commented Aug 23, 2022

Add a max_step parameter to BaseSolver and pass it to the solvers in the appropriate way. Then for drive cycle simulations we should set max_step to the step size in the data.

@rtimms
Copy link
Contributor Author

rtimms commented Aug 23, 2022

Or at least do that in our drive cycle examples

@rtimms rtimms added feature difficulty: easy A good issue for someone new. Can be done in a few hours labels Nov 1, 2022
@jaskiratsingh2000
Copy link

Hey @rtimms What are you excatly expecting out of this? Where can I find this? Thanks!

@rtimms
Copy link
Contributor Author

rtimms commented Feb 20, 2023

I wanted to add a max_step parameter to BaseSolver which controls the maximum step size the solver can take. You'll need to look at the docs for the each of the solvers we use as they may all have different interfaces. E.g. for scipy integrate you can directly pass max_step as an extra option. We should probably follow their syntax so that the default is inf, i.e., the step size is not bounded and determined solely by the solver.

Note this is different from dt_max used in the CasadiSolver to set the step size for event checking. I propose we rename dt_max to dt_event.

This might be more like medium rather than easy.

@jaskiratsingh2000
Copy link

I see. Can you provide a basic example for the same because I'm not getting where other parameters are gettiing called in https://github.com/pybamm-team/PyBaMM/blob/develop/pybamm/models/submodels/base_submodel.py

@Saransh-cpp
Copy link
Member

I think you are looking at the wrong class (based on the link you attached). BaseSolver -

class BaseSolver(object):

@jaskiratsingh2000
Copy link

@rtimms Any update on this?

@rtimms
Copy link
Contributor Author

rtimms commented Feb 23, 2023

What do you need an update on? I think you were looking at the wrong class, as suggested by @Saransh-cpp .

@jaskiratsingh2000
Copy link

@rtimms I was expecting the example on above. That would be helpful if you can provide it.

@arjxn-py
Copy link
Member

arjxn-py commented Mar 7, 2023

Hi there @rtimms I had gone through the scipy integrate to try to understand what exacty could be done, but I don't find any parameter like max_step there. Would you like to suggest me anything in regards?

@rtimms rtimms added the priority: medium To be resolved if time allows label May 15, 2023
@rtimms
Copy link
Contributor Author

rtimms commented May 16, 2023

max_step is in the **options part of the scipy docs

@medha-14
Copy link
Contributor

Could I work on this issue if no one else is currently assigned to it?

@agriyakhetarpal
Copy link
Member

Could I work on this issue if no one else is currently assigned to it?

Yes – @arjxn-py, if you're not working on #3106 anymore, could @medha-14 try to take over? @medha-14, you can pull that branch from Arjun's fork so that authorship of the previous work is preserved.

@arjxn-py
Copy link
Member

Hi @medha-14, thank you for extending your interest. Please feel free to take over the PR.
Sorry this one fell through the cracks.

@medha-14 medha-14 linked a pull request Dec 13, 2024 that will close this issue
8 tasks
@arjxn-py arjxn-py assigned medha-14 and unassigned arjxn-py Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours feature priority: medium To be resolved if time allows
Projects
None yet
6 participants