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

[Roadmap area] Parameter refactor #3909

Open
brosaplanella opened this issue Mar 20, 2024 · 3 comments
Open

[Roadmap area] Parameter refactor #3909

brosaplanella opened this issue Mar 20, 2024 · 3 comments
Assignees

Comments

@brosaplanella
Copy link
Member

Refactor parameter values to use the BPX format by default.
Using nested dictionaries allows cleaner definition of negative / positive and also different components of the battery.
This is a big breaking change so we should add a conversion class from legacy parameter sets to new ones, and legacy parameter sets should still work for a while.

Originally posted by @tinosulzer in #3839 (comment)

@rtimms
Copy link
Contributor

rtimms commented Mar 20, 2024

FYI we (Ionworks) have a FUSE internship open for applications that is related to this, so we can support this over the summer

See https://www.linkedin.com/jobs/view/3851436140/

@valentinsulzer
Copy link
Member

I think the main points to discuss are:

  1. What to call the new parameters (stick close to pybamm or switch to bpx). Does the latest version of BPX allow us to specify all parameters, including miscellaneous ones? If so, I suggest we align ourselves completely with that, even if we don't totally agree with all the parameter naming choices. My only change would be that our specification should be python-based instead of json-based to allow for any arbitrary python functions / interpolants.
  2. How to handle string names for the nested dictionaries (e.g. pybamm.Parameter("Negative::Particle size [m]"))
  3. How to do the transition slowly / in a backwards compatible way

@valentinsulzer
Copy link
Member

Also, when we do this, it would be great to do a deep review of existing parameter sets, including removing parameters that are not provided in the original reference (e.g SEI parameters for a performance-only model). See #3919 (review)

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

No branches or pull requests

5 participants