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

Add "temperature" argument to experiment #2358

Closed
valentinsulzer opened this issue Oct 13, 2022 · 9 comments · Fixed by #2518
Closed

Add "temperature" argument to experiment #2358

valentinsulzer opened this issue Oct 13, 2022 · 9 comments · Fixed by #2518
Assignees
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours

Comments

@valentinsulzer
Copy link
Member

Simulation should then update the parameter values ("Ambient temperature [K]" and "Initial temperature [K]", not "Reference temperature [K]"), with the given temperature

@valentinsulzer valentinsulzer added difficulty: easy A good issue for someone new. Can be done in a few hours hacktoberfest labels Oct 13, 2022
@Niiola
Copy link

Niiola commented Oct 13, 2022

can i get more explanation on this project and also i would love to be assigned this task

@valentinsulzer
Copy link
Member Author

  1. In the Experiment (https://github.com/pybamm-team/PyBaMM/blob/develop/pybamm/experiments/experiment.py) class, add a new attribute temperature which is None by default but can be provided
  2. In the Simulation (https://github.com/pybamm-team/PyBaMM/blob/develop/pybamm/simulation.py) class __init__, if experiment.temperature is not None, update ambient and initial temperature in the parameter values with the given temperature.

@Scottmar93
Copy link
Contributor

I also think temperature control is super useful to have in the experiment class.

@tinosulzer what's your view on the complexity of allowing the ambient temperature to be controllable on the step level? e.g.

   experiment = pybamm.Experiment(
    [
        (
            "Discharge at C/10 for 10 hours or until 3.3 V at 25oC",
            "Rest for 1 hour at 35oC",
            "Charge at 100 A until 4.1 V at 35oC (1 second period)",
            "Hold at 4.1 V until 5 A at 35oC (1 seconds period)",
            "Rest for 1 hour at 25oC",
        ),
    ]
)

This would give the experiment class a similar level of control as most battery cyclers.

@valentinsulzer
Copy link
Member Author

This would be super easy, the hardest part would probably be reading the string to extract the temperature, e.g. adding a new field "T" for the operating conditions. Once that is done, it's just a matter of updating the parameter values for each step before processing the model here

@Scottmar93
Copy link
Contributor

Nice! That is great to hear.

Is this something worth doing within this issue or separately? Can perhaps be done separately with this issue setting a global temperature and any temperature set on a step overwriting it.

@valentinsulzer
Copy link
Member Author

I don't mind, whichever you prefer

@valentinsulzer
Copy link
Member Author

Feel free to take over this issue too unless @Niiola has made progress?

@Scottmar93
Copy link
Contributor

Happy to do as @Niiola wishes 😄

@Niiola
Copy link

Niiola commented Nov 20, 2022 via email

@Scottmar93 Scottmar93 self-assigned this Nov 20, 2022
Scottmar93 added a commit that referenced this issue Dec 4, 2022
Scottmar93 added a commit that referenced this issue Dec 4, 2022
Scottmar93 added a commit that referenced this issue Jan 8, 2023
Scottmar93 added a commit that referenced this issue Jan 8, 2023
Scottmar93 added a commit that referenced this issue Jan 8, 2023
Scottmar93 added a commit that referenced this issue Jan 10, 2023
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants