jordens
commented
5 years ago I don't like that and I don't see a clear benefit from unifying argument hierarchy, dataset hierarchy, python object/attribute, and HDF5 hierarchy. They are all different things.
- We are not tied to HDF5 as the storage format. It could be something else and we hardly make significant use of it. I don't see HDF5 path separators as an indicator for what's good or required. We also don't map dataset hierarchy or argument hierarchy to HDF5 hierarchy at all.
- Breaking Python variable naming scheme with the dataset hierarchy separator is intentional. Otherwise you end up with monstrous variable names like the one you propose:
self.globals__DDS__Ba_493__frequency). Instead retrieve the hierarchical datasets parametrically in host code and store them as idiomatic variable names, maybe something along the lines of (from memory, untested):
class Servo(HasEnvironment):
def build(self, name):
self.gain = self.get_dataset("servo.{}.gain".format(name))
self.y = 0.
@kernel
def update(self, x):
self.y += self.gain*x
class Exp(EnvExperiment):
def build(self):
self.a = Servo(self, name="a")
self.b = Servo(self, name="b")
@kernel
def run(self):
self.a.update(1.3)
self.b.update(3.)You could even automate that prefix scheme to save some typing.
- Snake case is the dominant style for attribute and variables in Python, HDF5, file names, influxdb names, etc. Using double underscore for something else inside variable names makes that unreadable and also hard to parse both mentally and computationally.
- Visual grouping of arguments is separate again as there is limited use in nesting deeply. Not allowing deep nesting was a conscious decision. Also argument groups don't map to attribute or variable names at all (but you can make them parametric in the same style as the example above).
- Retrieving the entire dataset db and shoe-horning into attributes is not recommended. I haven't felt the need to do that. I don't know why you think that's a good idea.
TheCakeIsAPi
dhslichter
dnadlinger
ARTIQ Feature Request
Problem this request addresses
The group delimiter is '.' for datasets and allows multiple levels. It is '/' for hdf5 and allows multiple levels. For arguments it must be specified with the 'group=' parameter and only allows one level deep. In the namespace no delimiters are allowed because both '.' and '/' break the python variable naming scheme, and getattr is not allowed in the kernel so you can't call it from the safety of a string, e.g. getattr('globals.DDS.Ba_493.frequency').
We are mapping our experimental parameters between all 4 of these. My stopgap solution has been to use '.' in datasets, to automatically convert '.' to '__' (double-underscore) when pulling this into the namespace, to automatically use the 2nd group ('DDS' in the example above) for argument grouping, and to allow datasets in the hdf5 file to keep the '.' and thereby remain (sadly) ungrouped.
Describe the solution you'd like
Assuming double-underscore is a reasonable python-safe delimiter: Change the group delimiter to double-underscore for datasets. Use double-underscore as an automatic group delimiter for multi-level grouping in arguments. Automatically convert the delimiter to '/' to preserve grouping in the HDF5 hierarchy.
Additional context
I've created a get_dataset_db method, patterned after get_device_db, which allows me to scrape the dataset list and automatically import anything that starts with 'globals.' which is how I'm storing experiment parameters. Without this I would be specifying every parameter name in code and this issue would be less relevant, although it would still be nice to have consistency and to preserve grouping in the hdf5 files.