FaradayInstitution / BPX

BPX schema in pydantic and JSON schema format, and parsers
MIT License
25 stars 12 forks source link

Move units from key name to nested JSON structure #56

Open ejfdickinson opened 3 months ago

ejfdickinson commented 3 months ago

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 commented 3 months ago

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.