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

Move units from key name to nested JSON structure #56

Open
ejfdickinson opened this issue Jun 19, 2024 · 1 comment
Open

Move units from key name to nested JSON structure #56

ejfdickinson opened this issue Jun 19, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@ejfdickinson
Copy link
Collaborator

Simon Clark (SINTEF) wrote:

I’m wondering if including the units as part of the key name is wise? A quantity always has two parts: a value and a unit, and for flexibility it is helpful to express it that way. For example [below] which allows you a little more flexibility in supporting alternative units or unit conversions. It does add an extra level, but I like the clarity of it. It also brings in the option to add source information for where parameters came from (see point on creator metadata below). If the unit is part of the key name, then you need to either (a) already know what the unit is or (b) parse it out from the string, which adds a level of ambiguity / effort.

Example:

Replace:

"Thickness [m]": 10e-6

with

"Thickness": { 
  "value": 10E-6,
  "unit": "m"
} 
@ejfdickinson
Copy link
Collaborator Author

ejfdickinson commented Jun 19, 2024

I think it's quite healthy that we mandate base SI units and that conversions must take place outside BPX if desired. I also think this suggestion might harm human-readability. Perhaps to be considered as part of a general further separation of BPX from PyBaMM key notation.

@ejfdickinson ejfdickinson added the enhancement New feature or request label Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant