Warning on negative delay in IVVI driver about qcodes HOT 9 CLOSED

I tried suppressing them a bit in my current experiment but a snippet of the form below does not work. Presumably because it is comming from logging.warning (qcodes.utils.helpers line 85) and not from warning.warn. If anyone knows the equivalent command to suppress a specific warning from logging please let me know.

import warnings
with warnings.catch_warnings():
    warnings.filterwarnings("ignore")
    IVVI.dac1.set(100)

from qcodes.

Ah right - the solution I proposed before was to allow sweep_delay=None (or perhaps 0?) then it just goes as fast as it can with no warnings. What do you think of that? It would be easy to add - just have to allow one of those values in set_sweep and then omit the delays in _validate_and_sweep and _validate_and_sweep_async when there's no delay

And if you do want to suppress warnings, it looks like you can do (unfortunately doesn't seem that there's a built-in context manager for this...):

root_logger = logging.getLogger()
log_level = root_logger.level
root_logger.setLevel(max(log_level, logging.ERROR))
# code to ignore warnings in
root_logger.setLevel(log_level)

We should probably start scoping our logging to qc_logger = logging.getLogger('qcodes') rather than implicitly using the root logger - setLevel on the root logger will still apply to our logger, but users can do more interesting things if we scope our log messages.

from qcodes.

I think having sweep-delay = None is not the desired solution, we definately do want this to be slowed down for chip safety reasons and not vary wildly whenever the software feels like it.

I think that changing the scope of the logger is also not the way to solve this. It does make the problem go away but it is not a sustainable solution. I do not want to suppress warnings in general ( I get a lot of useful ones all the time), I only want to suppress this specific warning in when calling this specific command. That's where the with construction is nice, it is just that it does not work here (or I have not found it's equivalent).

I think that scoping qcodes error messages is a great idea but also one that requires a bit of thought. I think that for now using the default (and well described) logging levels makes sense. Making a custom logger at some point may give some added value but I think we want to save that for another day(/issue).

from qcodes.

ok... so you want to make sure there's some delay, but you're not comfortable increasing the delay such that it's always limited by software? What about a min_sweep_delay parameter, that will make sure each loop iteration takes at least that long but not complain about extra time?

from qcodes.

@alexcjohnson , even though that would indeed solve the problem I am not a big fan of adding another parameter looking at how complex the parameter class already is.

I would instead opt for either, finding some way to manually suppress the warning, or changing the default behaviour of sweep delay to be what you describe for min_sweep_delay.

What do you think?

from qcodes.

I don't think we should make a practice of suppressing warnings, we should code such that they are only emitted when the user should really be warned about something. Even here this could suppress behavior you would consider dangerous (though I guess we should change this so each delay resets the start time): what if the first delay was really long, then after that the delays were all super short but they fit within the accumulated negative delay time.

We could do something like make a few different plugin classes that just do setting - the default one set_method=SimpleSetter() just doing what _validate_and_set does (the parameter itself defining the underlying validate and _set methods, and the setter class just using it), then another set_method=SteppedSetter(sweep_step, sweep_delay, max_val_age, warn_on_extra_delay=True) that sweeps (and set warn_on_extra_delay=False to get the behavior you want, or this could even be a third setter class). I'm not sure that would really be easier to use, but at least it segments the argument list a little.

from qcodes.

@AdriaanRol a bit more followup after chatting with @alan-geller - you're saying that the 50ms time it takes to execute one set is enough to make the stepping safe, but you don't want to waste time waiting ANY LONGER than that (these warnings would all presumably all go away if you set the sweep_delay to 60ms, unless the IVVI has a 50ms internal clock cycle or something...). What if the 50ms changes? What if someone upgrades the hardware, or figures out a better way to run the comm channel, and suddenly it's only 1ms per set? Then you start blowing up devices if you're just relying on the comm channel to limit the sweep rate. Conversely, what if something goes wrong and it's suddenly taking 500ms per set? You'll never know about it (until you start wondering why your sweeps are taking so long) if the warnings are suppressed.

So I think when we're making stepped setters, we should really make it explicit what both the minimum (target) and maximum step times are. In this case I'd imagine something like 30ms and 60ms to allow for some potential speedup to help you without sacrificing safety, while continuing to monitor for glitches. That could be with another argument to StandardParameter (both sweep_delay and max_sweep_delay perhaps, and max defaults to the same as sweep_delay?) or to an extra object like I suggest above (but I've convinced myself anyway that warn=False is a bad idea) or a different form for the sweep_delay argument (a 2-tuple (min, max)?) but however we do it we need to find a way to provide 2 times.

from qcodes.

@alexcjohnson
I like the idea of having both a min and max delay. If we then only throw a warning there is no risk of ignoring warnings that should not be ignored. 👍

In the future we may also want to be able to change these values as it depends not only on the instrument driver but also on the sample/usage of the instrument. I guess that would be a separate issue but definitely something we want to think about.

from qcodes.

closed by #47

from qcodes.