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

"Adding" solutions is slow (casadi.horzcat) #1329

Closed
rtimms opened this issue Jan 18, 2021 · 2 comments · Fixed by #1331
Closed

"Adding" solutions is slow (casadi.horzcat) #1329

rtimms opened this issue Jan 18, 2021 · 2 comments · Fixed by #1331
Assignees

Comments

@rtimms
Copy link
Contributor

rtimms commented Jan 18, 2021

"Adding" solutions can take up a large portion of the overall solution time. The time is spend calling casadi.horzcat.

E.g. in the below example casadi.horzcat takes about 9% of the overall time.

import pybamm
pybamm.set_logging_level("INFO")
options = {
    "sei": "solvent-diffusion limited",
    "sei film resistance": "distributed",
    "sei porosity change": "true",
}
model = pybamm.lithium_ion.DFN(options=options)
params = pybamm.ParameterValues(chemistry=pybamm.parameter_sets.Chen2020)
params.update(
    {
        "Reference temperature [K]": 278.15,
        "Ambient temperature [K]": 278.15,
        "Initial temperature [K]": 278.15,
        "Upper voltage cut-off [V]": 4.21,
    }
)
exp = pybamm.Experiment(
    ["Hold at 4.2V until C/100", "Rest for 4 hours (3 minute period)"]
    + [
        "Discharge at 1C until 2.5V",
        "Charge at 0.3C until 4.2V (3 minute period)",
        "Hold at 4.2V until C/100 (3 minute period)",
    ]
    * 50
)
sim = pybamm.Simulation(model=model, parameter_values=params, experiment=exp)
sim.solve()
@valentinsulzer
Copy link
Member

Should be fixed, together with #1276 , by reformatting Solution to create a processed variable from its sub-solutions, without first creating solution.y

@valentinsulzer valentinsulzer self-assigned this Jan 18, 2021
valentinsulzer added a commit that referenced this issue Jan 20, 2021
valentinsulzer added a commit that referenced this issue Jan 20, 2021
valentinsulzer added a commit that referenced this issue Jan 20, 2021
valentinsulzer added a commit that referenced this issue Jan 20, 2021
@valentinsulzer
Copy link
Member

This was a great find @rtimms - new results look promising!

On develop, this experiment took 7:28
On new branch, this experiment took 5:35

valentinsulzer added a commit that referenced this issue Jan 21, 2021
valentinsulzer added a commit that referenced this issue Jan 21, 2021
#1329 reformat Solution to avoid concatenating y
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants