Open systemofapwne opened 5 months ago
dnadlinger
commented
5 months ago The second call to self.setattr_argument(key,...) should internally update the type of the argument.
I think as a user I'd prefer getting a clear error message on the experiment side (exception thrown with better error message). Otherwise, doesn't this risk introducing unexpected dependencies on the order classes are instantiated?
systemofapwne
commented
5 months ago [...] Otherwise, doesn't this risk introducing unexpected dependencies on the order classes are instantiated?
It depends on, what you want to do.
In our specific case, we have sub-experiments structure like this here
# This is a "Controler" class, that controls various aspects of an experiment. Such as turning one specific part on or off. Here, it sets a detuning
class Controler(HasEnvironment):
def build(self):
self.setattr_argument("detuning_blue", processor = NumberValue(default = 1, ndecimals = 2, scale=1.0, unit="Γ"))
def init(self):
# Will use self.detuning_blue attribute in here
self.set_detuning(self.detuning_blue)
def set_detuning(self, detuning):
# Does not use self.detuning_blue in here but uses the argument passed
# The main Experimentm that works fine
class MainExperiment(EnvExperiment):
def build(self):
self.setattr_argument("some_other_attribute", processor = NumberValue(default = 1, ndecimals = 2, scale=1.0))
self.Controler = Controler(self) # <-- This will call setattr_argument() on MainExperiment instance for arguments: detuning_blue
def run(self):
self.Controler.init() # Initializes to default values. Not necessary but
# Do other stuff below
# An experiment, that should replicated ALL parameters of MainExperiment and then specificly scan over one parameter
class ScannableExperiment(EnvExperiment):
def build(self):
self.setattr_device("scheduler") # Used later in the run method below
self.Exp = MainExperiment(self) # <-- This will call setattr_argument() on ScannableExperiment instance for arguments: detuning_blue, some_other_attribute
# Now, we want to modify a specific attribute to make it scannable and still use its original name
self.setattr_argument("detuning_blue", processor = Scannable(default=NoScan(1,1), scale=1.0, unit="Γ")) # <- This line later causes an exception, since detuning_blue has been previously defined as "NumberValue" via the MainExperiment(self) call above and artiq thinks, it still is of that type, while the data of it is now a dictionary, according to the Scannable representation
def run(self):
# Loop over the scannable.
# Pseudo code here: It decomposes the detuning_blue ScannableObject into individual "NumberValue" type
# and then loops over them. Individual experiments of type "MainExperiment" are then submitted to the scheduler,
# where the arguments of detuning_blue is scanned.I have to admit, that this looks like a very special case. But it if the user specificically says "I overwrite this existing argument of type A to new type B", then the underlaying code should comply. Apparently, artiq seems not do to so.
For the time being, I have written an ugly workaround, that easily might fail in the future, if the implementation of the artiq environment changes:
class ArgReplicator(HasEnvironment):
""" Replicates arguments of an experiment on a parent experiment """
def __init__(self, exp):
self._exp = exp
self._args = {}
# HACK: We disguise ourself to be the "argument manager" in order to intercept calls to self.__argument_mgr.get() inside artiq.environment.HasEnvironment.get_argument
# NOTE: This could break in future Artiq versions, when HasEnvironment.__init__ is or the argument manager's get() method is getting refactored
# dev_mgr = exp._HasEnvironment__device_mgr
# dataset_mgr = exp._HasEnvironment__dataset_mgr
# sched_def = exp._HasEnvironment__scheduler_defaults
super().__init__(exp)
self._HasEnvironment__argument_mgr = self
def get(self, key, *args):
""" Intercept Artiq calls of artiq.environment.HasEnvironment.get_argument for later use """
# NOTE: This could break in future Artiq versions, when artiq.environment.HasEnvironment.get_argument is getting rewritten
self._args[key] = args
def setattr_argument(self, key, processor=None, group=None, tooltip=None):
""" Try to extract group and tooltip, if already was set from the experiment we derive from """
if key in self._args:
v = self._args[key]
group = group or v[1]
tooltip = tooltip or v[2]
self._args[key] = (processor, group, tooltip)
@property
def args(self):
""" Return a list of all arguments of the experiment """
return list(self._args.keys())
def __call__(self):
if not self._exp: return
for key, args in self._args.items():
self._exp.setattr_argument(key, *args)With this, we can now write our Scannable Experiment's build() method like this:
# An experiment, that should replicated ALL parameters of MainExperiment and then specificly scan over one parameter
class ScannableExperiment(EnvExperiment):
def build(self):
self.setattr_device("scheduler") # Used later in the run method below
repl = ArgReplicator(self)
self.Exp = MainExperiment(repl) # <-- This will call setattr_argument() and internally the argument manager is intercepted by the ArgReplicator. Arguments are NEVER passed to the argument manager here
repl.setattr_argument("detuning_blue", processor = Scannable(default=NoScan(1,1), scale=1.0, unit="Γ")) # Overwrites existing detuning_blue on the ArgReplicator object
repl() # Applies all arguments of the ArgReplicator on the instance of this very class here.
Since I am a fan of fixing problems rather than working around them, the argument manager should just allow changing previously defined arguments if the users choses so.
SimonRenblad
commented
5 months ago The experiment works as expected when ran via artiq_run.
I should note here (for the sake of completeness) that artiq_run only works if the "detuning_blue" argument is not set. For the minimal example above, the following code throws the same exception:
artiq_run <FILE> "detuning_blue={'value': 1, 'repetitions': 1, 'ty': 'NoScan'}"Additionally, if the minimal example was altered slightly with StringValue instead of NumberValue:
class ArgError(EnvExperiment):
def build(self):
self.setattr_argument("detuning_blue", processor = StringValue(default = "string"))
self.setattr_argument("detuning_blue", processor = Scannable(default=NoScan(1,1), scale=1.0, unit="Γ"))No exception is thrown and the experiment is run without issues. (Calls to get_argument would not return the expected value however...)
SimonRenblad
commented
4 months ago A few thoughts regarding this issue after tinkering with it for a bit.
The workaround provided works as long only as long as there is no logic in build that depends on get_argument since the processing of unprocessed arguments is deferred until the end of build. An example of this is shown below. I do not know how common such usage is, but we do expose get_argument so it should to be accounted for.
def build(self):
if self.get_argument("checked", processor=BooleanValue(default=True)):
# build logic
else:
# other build logicI also do not think that suppressing errors caused by processor.process() is a good solution, as it may lead to some confusing behavior and misleading error messages for easy mistakes.
Example: Entering an invalid string to a NumberValue would cause the message "Supplied argument(s) not queried in experiment".
One potential solution is to take the deferred concept from @systemofapwne's ArgReplicator and make it a part of the API. It may look something like this:
def build(self):
self.defer_processing("detuning_blue") # key is marked for deferral
# these 2 calls no longer process the unprocessed arg, `self.detuning_blue` is set to default value
self.setattr_argument("detuning_blue", processor = NumberValue(default = 1, ndecimals = 2, scale=1.0, unit="Γ"))
self.setattr_argument("detuning_blue", processor = Scannable(default=NoScan(1,1), scale=1.0, unit="Γ"))
self.set_deferred() # "detuning_blue" is processed by `Scannable` and set as attribute + kernel invariantThis would allow the usage of sub experiments as desired without having to change their interface, as defer_processing and set_deferred can be called by the top-level experiment. Obviously better names would be given for these methods as well...
Thoughts on this @systemofapwne?
Bug Report
One-Line Summary
Calling
self.setattr_argument(key,...)for the same key but different type raises a TypeError on the masterIssue Details
Steps to Reproduce
Expected Behavior
The second call to
self.setattr_argument(key,...)should internally update the type of the argument.Actual (undesired) Behavior
The master raises a TypeError
Your System
Additional notes
The experiment works as expected when ran via
artiq_run.