labscript-suite-temp / labscript

The labscript Python library provides a translation from simple Python code to complex hardware instructions. The library is used to construct a "connection table" containing information about what hardware is being used and how it is interconnected. Devices described in this connection table can then have their outputs set by using a range of functions, including arbitrary ramps.
BSD 2-Clause "Simplified" License
0 stars 0 forks source link

Missing WAIT instruction(s) #47

Closed philipstarkey closed 6 years ago

philipstarkey commented 6 years ago

Original report (archived issue) by Russell Anderson (Bitbucket: rpanderson, GitHub: rpanderson).


The quantisation of change times via quantise_to_pseudoclock (pull request #14) has introduced a bug to waits. Wait instructions are not always appended to the clock instructions, as:

This bug arises in the following (using 32f1c4d739488b9cf5e1ae3ab13e52f38a0a17aa) for various values of t_wait:

#!python

from __future__ import division
from labscript import ClockLine, DigitalOut, WaitMonitor, AnalogIn, start, stop, wait
from labscript_devices.PulseBlaster import PulseBlaster
from labscript_devices.NI_PCIe_6363 import NI_PCIe_6363

PulseBlaster(name='pulseblaster_0', board_number=0)
ClockLine(name='pulseblaster_0_ni_clock',
          pseudoclock=pulseblaster_0.pseudoclock, connection='flag 0')
NI_PCIe_6363(name='ni_pcie_6363_0', parent_device=pulseblaster_0_ni_clock,
             clock_terminal='/ni_pcie_6363_0/PFI0', acquisition_rate=1e4, 
             MAX_name='ni_pcie_6363_0')
WaitMonitor(name='wait_monitor', parent_device=ni_pcie_6363_0, connection='port0/line3',
            acquisition_device=ni_pcie_6363_0, acquisition_connection='ctr0',
            timeout_device=ni_pcie_6363_0, timeout_connection='port1/line1')

start()
t = 0
t_wait = 4.5/ni_pcie_6363_0.clock_limit
t_wait_quantised = ni_pcie_6363_0.quantise_to_pseudoclock([t_wait])[0]
print('t_wait  = {:.10f} s'.format(t_wait))
print('quantised = {:.10f} s'.format(t_wait_quantised))
print('difference = {:.3e} s'.format(t_wait_quantised - t_wait))
t += t_wait
t += wait(label='wait_test', t=t, timeout=0.1)
t += 1e-3
stop(t)

The bug can be detected by adding a print statement before/after clock.append('WAIT') in labscript.py, or by appending the following to the above labscript (after stop(t)):

#!python

if t_wait_quantised in pulseblaster_0.trigger_times[1:]:
    print('WAIT instruction added')
else:
    print('WAIT instruction missed')

It seems to be that non-integer multiples of 1/ni_pcie_6363_0.clock_limit reliably result in the missing WAIT instruction. A potential solution is to quantise the trigger times before checking each element of all_change_times for membership of self.parent_device.trigger_times[1:]:

#!python
quantised_trigger_times = self.quantise_to_pseudoclock(self.parent_device.trigger_times)
for i, time in enumerate(all_change_times):
    if time in quantised_trigger_times[1:]:
       clock.append('WAIT')

... or check for proximity of the differently conditioned times rather than equality.

#!python
for i, time in enumerate(all_change_times):
    if min(abs(time - self.parent_device.trigger_times[1:])) <= self.pseudoclock_device.clock_resolution/2:
       clock.append('WAIT')
philipstarkey commented 6 years ago

Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).


labscript 3 (in progress, to be resumed post-thesis-submission) has a solution to this by quantising correctly based on the time since the wait preceding a given instruction.

To do similar here, one might want to modify quantise_to_pseudoclock() to partition the change times based on which wait preceded them, and then quantise them in terms of the time since the last wait, rather than the time since t=0.

The wait times themselves, when quantised to how long its been since themselves, will remain unchanged, which is what should resolve the root cause of this issue.

philipstarkey commented 6 years ago

Original comment by Russell Anderson (Bitbucket: rpanderson, GitHub: rpanderson).


philipstarkey commented 6 years ago

Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: pstarkey).


While I agree that modifying the code to quantise relative to each trigger point (as if t=0 each trigger time) is the best solution to resolving the issue, I suspect it's more practical to leave that until the next major version of labscript.

I actually think the bug might just be that we quantise the stop time and don't quantise the trigger times here. I'm not sure if there would be a lingering issue with the fact that the outputs offset their instructions times using pre-quantised trigger times (and then quantise after the subtraction) but I suspect that the rounding to 0.1ns we do everywhere probably prevents issues with that (by removing floating point rounding errors).

Russ, do you want to try adding self.trigger_times = self.quantise_to_pseudoclock(self.trigger_times) at the bottom of PseudoclockDevice.offset_instructions_from_trigger to see if it fixes it?

philipstarkey commented 6 years ago

Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).


Absolutely happy with the quickest solution that resolves immediate problems, if this solves it!

philipstarkey commented 6 years ago

Original comment by Russell Anderson (Bitbucket: rpanderson, GitHub: rpanderson).


... try adding self.trigger_times = self.quantise_to_pseudoclock(self.trigger_times) at the bottom of PseudoclockDevice.offset_instructions_from_trigger to see if it fixes it?

This fixed it.

philipstarkey commented 6 years ago

Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).


Resolves issue #47, where WAIT instructions were not issued to a clock.

→ \<\<cset a897716e2e021b70c2b60ec25521f0e5af102e15>>