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

Refactor Simulation.solve #3176

Open
2 of 4 tasks
brosaplanella opened this issue Jul 21, 2023 · 2 comments
Open
2 of 4 tasks

Refactor Simulation.solve #3176

brosaplanella opened this issue Jul 21, 2023 · 2 comments
Assignees
Labels
difficulty: hard Will take several weeks in-progress Assigned in the core dev monthly meeting priority: medium To be resolved if time allows

Comments

@brosaplanella
Copy link
Member

brosaplanella commented Jul 21, 2023

As part of #3101 I was looking at the Simulation class and noticed that the messy part is basically the solve method (and in particular, the experiment part). In my mind, the bigger goal here is to keep Simulation class (without experiments) to be compatible with any model (even models beyond battery).

I think that just simply refactoring the whole experiment part as a separate function would help a lot (maybe with an extra check of whether a given model supports experiments or not). The other main point I would like to discuss here is whether we deprecate the "drive cycle" mode, given that now this can easily be done with the experiment steps.

TLDR What I propose here is:

  • Get rid of the "drive cycle" mode in the Simulation (already supported via experiment).
  • Refactor the code that solves the simulation for experiment.
  • Ensure the "without experiment" mode works with any model (even those custom defined).
  • As part of this, refactor experiment to better handle different start/end times for identical steps.
@brosaplanella
Copy link
Member Author

I have noticed that this would probably require defining self.callbacks and self.logs. I am not familiar on how they work at all, so would there be any issue with this?

@brosaplanella brosaplanella added difficulty: hard Will take several weeks priority: medium To be resolved if time allows labels Jul 27, 2023
@brosaplanella
Copy link
Member Author

I have noticed that this would probably require defining self.callbacks and self.logs. I am not familiar on how they work at all, so would there be any issue with this?

After discussion in the developers meeting that should be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: hard Will take several weeks in-progress Assigned in the core dev monthly meeting priority: medium To be resolved if time allows
Projects
Status: In Progress
Status: In Progress
Development

Successfully merging a pull request may close this issue.

1 participant