UCLALabRAD / LaSER-Project

Effort to add functionality to the EGGS-Campbell repository to allow for the simulation of hardware in experiments conducted by the Campbell and Hudson labs.
0 stars 0 forks source link

Possible parametervault issue: inability to get/set parameters more than one level down in registry's Parameter Vault directory #2

Open lando224 opened 2 years ago

lando224 commented 2 years ago

Other functionality might be screwed up too; as an example get_parameter_names of a top level directory may give names of subdirectories 2 levels down... will investigate (edit: this was true)

lando224 commented 2 years ago

saveParameters and loadparameters seem to have correct logic while get and set I believe do not

lando224 commented 2 years ago

Yeah, this was actually an issue. Changed so now supports collections that are more than one level below the parameter vault directory. Currently a collection contains all parameters that are keys in the corresponding directory OR a subdirectory (subcollection). Uses dot notation with subcollection names (example: subcoll.subsubcoll.subsubparam) to avoid name clashes and so client can get/set parameters for any subcollection easily.

lando224 commented 2 years ago

Need to do more research about whether this is a good approach based on how PV is generally used. Also doesn't maintain backwards comp since '.' probably now not permitted in parameter name, so may need a different approach.

lando224 commented 2 years ago

Note: need to make sure scriptscanner plays nicely with new dot notation.

lando224 commented 2 years ago

Fixed. Will close this issue. Summary: getParameter,setParameter,get_parameternames all used incorrect logic. Each incorrectly assumed collection had no subcollections (whereas load and save_parameters did not assume this). So if dir structure was topleveldir->subdir->param, there was no way to get or set param, and get_parameters would give subdir. Now, topleveldir is a collection with parameter subdir.param, and [topleveldir,subdir] is a collection with parameter param. In experiment file, should use same . notation for attribute of pv (example: self.p.toplevel.subdir.param)

lando224 commented 2 years ago

Reopening because I was thinking about this a bit more. Right now you can set parameters lower than the top level which is great. But I wonder if the subcollection thing should go. So for example in the above case topleveldir is still a collection with parameter subdir.param but there would be no collection [toplevel, subdir]. The issue with the current way is that in actual experiments when you set the required params, they still have to have the entire path there, just split between the collection and the parameter however you want. Which is fine, but then you don't get to magically use self.parameters differently, because you still have to have the whole path separated by dots when you grab a parameter from this treedict. We could change it so its stored in the treedict with the parameter name starting from the lowest level in the collection, but the issue here is name clashes. need to discuss.

lando224 commented 2 years ago

Idea: can use * to import everything from a collection. This could justify the use of subcollections.

lando224 commented 1 year ago

Going to scrap subcollections logic for now