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

ParameterValues.create_from_bpx - support BPX v0.4 #3679

Closed
ejfdickinson opened this issue Jan 3, 2024 · 4 comments · Fixed by #3414
Closed

ParameterValues.create_from_bpx - support BPX v0.4 #3679

ejfdickinson opened this issue Jan 3, 2024 · 4 comments · Fixed by #3414
Labels

Comments

@ejfdickinson
Copy link

ejfdickinson commented Jan 3, 2024

Description

Add support to ParameterValues.create_from_bpx() for new BPX 0.4 features.

  • Import blended electrode parameters according to specification.
  • Consider whether PyBaMM could define a standard for importing user-defined parameters for model extensions. As an example, this might include a read from BPX for the necessary branches for current sigmoid hysteresis from user-defined BPX FloatFunctionTable features.

Motivation

No response

Possible Implementation

A simple PyBaMM-internal standard might be that the contents of the BPX user-defined parameters node is read directly into the ParameterValues object. Then users could add any PyBaMM-compatible parameter names to the BPX, under this node.

Since the content of a BPX file implies the type of model that should be used (DFN vs SPM, blended electrode or not, hysteresis or not), could create_from_BPX optionally return a dict of appropriate model options? Or else another utility function could do this?

Additional context

No response

@rtimms
Copy link
Contributor

rtimms commented Jan 10, 2024

See #3414

@rtimms
Copy link
Contributor

rtimms commented Jan 10, 2024

This was working when I made the draft PR. I need to check it works with the actual v.0.4.0 release. Will try to get to it this week so it can be included in the final 24.1 release of PyBaMM (tagging @Saransh-cpp)

@rtimms
Copy link
Contributor

rtimms commented Jan 10, 2024

A simple PyBaMM-internal standard might be that the contents of the BPX user-defined parameters node is read directly into the ParameterValues object.

This is the implementation in the draft PR

Since the content of a BPX file implies the type of model that should be used (DFN vs SPM, blended electrode or not, hysteresis or not), could create_from_BPX optionally return a dict of appropriate model options? Or else another utility function could do this?

Yes, but this might require us to be strict about how/where people give this in the BPX file. E.g. if hysteresis is given as a user-defined parameter, should we expect users to explicitly say this in the description somewhere? I wouldn't like to try and infer the model option from user-defined parameters.

@ejfdickinson
Copy link
Author

Thanks @rtimms - I think you are right to discard the second suggestion, and leave this to the user. Probably it should be documented that ParameterValues.create_from_bpx() only creates the pybamm.ParameterValues object, and does not embed aspects of the BPX standard that oblige a certain model, such as DFN vs SPM.

I advise that this issue can be closed if the draft PR #3414 is tested and merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants