Repository navigation
Negative Delay #13
Description
This issue might be related to microsoft/Qcodes#44 and microsoft/Qcodes#47
upon writing a numerical simulation as an instrument several problems occurred (described in separate issues)
I want to sweep over some numerical parameter such the energy of the system and measure some transmission property. the loop i wrote is:
loop = qcodes.Loop(system.parameters["energy"][0:1:0.1], delay=0)
data = loop.run(background=False)
where system is the variable containing the numerical simulation instrument
(the background=false is part of a separate issue)
upon running this code i get the same warning as @AdriaanRol got before:
WARNING:root:negative delay -0.000041 sec
In the case of such loops (containing numerical work rather than experiments) I feel that there should be some easy/trivial way to remove this sort of delays from the loop without causing this warning.
I feel that this behavior should be default for the syntaxes:
qcodes.Loop(system.parameters["energy"][0:1:0.1], delay=0)
qcodes.Loop(system.parameters["energy"][0:1:0.1], delay=None)
qcodes.Loop(system.parameters["energy"][0:1:0.1])
I understand that this warning is very important in some loops, but that is only the case if delay>0.
For loops not containing a delay, the actual time taken will always be larger than 0 and the warning is meaningless.
Activity
alexcjohnson commentedon Mar 18, 2016
That sounds reasonable, 0 or None suppressing delay warnings. I'd rather not allow users to omit delay though - would be too easy to do accidentally, which can sometimes actually break things.
AdriaanRol commentedon Mar 18, 2016
Do you have an example of this? I would argue that delay=None should be the default option unless there is a strong argument why that is a really bad idea.
alexcjohnson commentedon Mar 18, 2016
It's not going to break things in code... I just mean like ramping certain parameters too fast can break a device.
akhmerov commentedon Mar 18, 2016
Shouldn't this rather go into the driver, as opposed to the measurement loop?
AdriaanRol commentedon Mar 18, 2016
Should that not be set in the instrument driver itself?
The way we currently operate is that we set the delay settings upon initialisation of our instrument. All this is set in an initialisation script that is very rarely modified and prevents any user mistakes on runtime.
I think the alternative you are proposing, which is setting the delay in the loop is far more error prone, I see it going wrong in two ways, the first is accidently forgetting a zero in the delay you set, the second is people copy pasting and forgetting about it altogether.
alexcjohnson commentedon Mar 18, 2016
Ah yes, it could go in the driver via an init script... that's a good idea. I think it's generally not a good idea for this to be part of the drivers themselves, because it will vary by experiment, but if the instrument itself cares requires some delay for safety then I guess it can be put in the driver. Or for simulations where you know delays are useless they can be preset to zero. How about this:
default_delayattribute that gets used if theLoopcall does not specify a delay or specifiesNoneNoneis NOT no delay, you must specifydelay=0Loopcall specifies a delay, it's an error - ie there's no global default, zero or otherwise.(old_value, new_value)for this to make sense as part of the parameter in a lot of cases (we talked about this at some point but I don't see it now).AdriaanRol commentedon Mar 18, 2016
@alexcjohnson
I would like to prevent introducing to much new complexity. I think there are two reasons why you would want to specify a delay. 1. Device/experiment safety reasons and 2. "Experiment" loop specific considerations.
For option 1 we have a perfectly fine way of specifying this in the instrument driver, the enhancement I would propose would be the ability to change this either upon initialization or after initialization of an instrument. For option 2 it belongs in the loop, however zero (0) is the sensible default if it is not specified.
Adding a new
default_delayparameter does not make sense to me. It only convolutes the amount of code to specfy otherwise trivial measurements.To summarize I propose the following:
alexcjohnson commentedon Mar 20, 2016
@AdriaanRol
you mean
sweep_stepandsweep_delay? Yes, that's true, good point. As it stands now,sweep_delayonly applies if you also have asweep_step, but we could apply it even without a step, then it's just the delay after anysetcall. Perhaps then it would be clearer if we changed the names toset_stepandset_delay?and re: changing these after initialization - yes, there's a
set_sweepmethod but in light of this discussion we likely want to rename it and let it change just what you specify to change, right now I think it requires all arguments.MerlinSmiles commentedon Apr 20, 2016
@alexcjohnson is there somehow a fix for this? Or anything I can do until there is a real fix?
These negative delays flood my notebooks and its basically unusable 😢
damazter commentedon Apr 26, 2016
Same here, printing the warning statements seems to be the major bottleneck for speed of my notebooks.
alexcjohnson commentedon Apr 26, 2016
Almost ready with a PR on this...
alexcjohnson commentedon Apr 26, 2016
@MerlinSmiles @damazter this should be resolved by microsoft/Qcodes#116 - closing, but feel free to reopen if issues persist.
MerlinSmiles commentedon Jun 5, 2016
@alexcjohnson I still see these negative delays, during my measurements. I guess I didnt see this for a long time because I removed that warning in my local copy 👼
giulioungaretti commentedon May 7, 2017
negative delays still happen and it's totally not clear what they mean IMHO.