You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently we're always converting keyword values to their default unit.
This is not needed for linting, only when converting to JSON or YAML and may even lead to problems when the XML schema lists internal_cp2k as the default unit, see #54.
It might be a good idea to delay the actual conversion to a time when it is actually needed and/or avoid it completely by changing the JSON/YAML format to yield a string containing the original unit.
The reason for this is that for serialization formats like json and yaml we yield values in the CP2K default unit since we do not export any unit specification.
It's a bit unfortunate that CP2K defines the internal units as abstract internal_cp2k rather than effective unit, especially for things where there would be a physical unit behind it (other than relative tolerances where this seems to be used otherwise).
This makes external linting of input files impossible since we could only validate such values by hardcoding the effective internal unit of those parameters within cp2k-input-tools.
I guess the proper way forward is to:
store an explicitly given unit for a quantity with a default unit of internal_cp2k and only validate it, do not attempt to convert the value from or to internal_cp2k.
when converting to a format (json or yaml) where the unit would be lost, either yield an error if the unit was explicitly given (since we can't convert it and the recipient of the json/yaml wouldn't know which unit it is), or yield the value as a string with the unit either suffixed (1.23 angstrom) or prefixed as in the CP2K input [angstrom] 1.23.
This would also delay unit conversion to the point when it becomes necessary (for linting it is not), rather than just attempting it all the time.
The intermediate solution: if the default unit is internal_cp2k, store as is, if there is an explicit unit given, raise an error since we can't convert to internal_cp2k.
Currently we're always converting keyword values to their default unit.
This is not needed for linting, only when converting to JSON or YAML and may even lead to problems when the XML schema lists
internal_cp2k
as the default unit, see #54.It might be a good idea to delay the actual conversion to a time when it is actually needed and/or avoid it completely by changing the JSON/YAML format to yield a string containing the original unit.
The parser tries to normalize values here:
cp2k-input-tools/cp2k_input_tools/keyword_helpers.py
Lines 150 to 154 in 10aaa86
The reason for this is that for serialization formats like json and yaml we yield values in the CP2K default unit since we do not export any unit specification.
It's a bit unfortunate that CP2K defines the internal units as abstract
internal_cp2k
rather than effective unit, especially for things where there would be a physical unit behind it (other than relative tolerances where this seems to be used otherwise).This makes external linting of input files impossible since we could only validate such values by hardcoding the effective internal unit of those parameters within cp2k-input-tools.
I guess the proper way forward is to:
internal_cp2k
and only validate it, do not attempt to convert the value from or tointernal_cp2k
.1.23 angstrom
) or prefixed as in the CP2K input[angstrom] 1.23
.This would also delay unit conversion to the point when it becomes necessary (for linting it is not), rather than just attempting it all the time.
The intermediate solution: if the default unit is
internal_cp2k
, store as is, if there is an explicit unit given, raise an error since we can't convert tointernal_cp2k
.Originally posted by @dev-zero in #54 (comment)
The text was updated successfully, but these errors were encountered: