Merry Christmas bug: Incorrect comparison of time intervals between clocklines for error checking#85

[spie...@umd.edu]

Dear All: Firstly happy holidays!

With that said I've just posted issue report in labscript "https://github.com/labscript-suite/labscript/issues/85" that really is a different face to some other issues.  

To save the trouble of clicking just once I will paste it below, but really, it will render better on gitub.

----------------

There are several issue reports (#53 and #51) that seem to relate to the same thing, which result from from a commit that I will touch on below. I will describe our issue here before describing the fix.

Emine and I were debugging an issue in the lab where we were pretriggering a novatech to get its update to occur at a desired time. This fails when any device on the same pulseblaster is clocked before the required 110 us delay between novatech updates, even if the are on different pseudoclock lines.

Here is the relevant code:

# Check that no two instructions are too close together: for i, t in enumerate(change_time_list[:-1]): dt = change_time_list[i+1] - t if dt < 1.0/clock_line.clock_limit: raise LabscriptError('Commands have been issued to devices attached to clockline %s at t= %s s and %s s. '%(clock_line.name, str(t),str(change_time_list[i+1])) + 'One or more connected devices on ClockLine %s cannot support update delays shorter than %s sec.'%(clock_line.name, str(1.0/clock_line.clock_limit))) all_change_times_len = len(all_change_times) # increment j until we reach the current time while all_change_times[j] < t and j < all_change_times_len-1: j += 1 # j should now index all_change_times at "t" # Check that the next all change_time is not too close (and thus would force this clock tick to be faster than the clock_limit) dt = all_change_times[j+1] - t if dt < 1.0/clock_line.clock_limit: raise LabscriptError('Commands have been issued to devices attached to %s at t= %s s and %s s. '%(self.name, str(t),str(all_change_times[j+1])) + 'One or more connected devices on ClockLine %s cannot support update delays shorter than %s sec.'%(clock_line.name, str(1.0/clock_line.clock_limit)))

from line 826 of labscript.py. The problem is with the second error check (line 840) that compares the update time of all channels with the clock_line.clock_limit of the current channel. The resolution that worked for us is to replace this with the maximum clock_limit of all the clock_lines. It may have been better to replace with self.clock_limit to get the actual limit of the pseudoclock.

Looking through this history, this commit is the one that causes this bug 5618445 . From what I can see I think the issue is that for sequences using the 50/50 duty cycle pulses the change I propose above would change the time of the falling edge of the clock line.

One compromise would be keep the current check for the 50/50 duty cycle pulses and accept my change for the minimum duration pulses, but it seems to me that the better solution is to treat "returning edges" on the same footing as the change times and do an error check for these in this same code block.

The issue specifically is present on line 480 of the PB source code where we see

if self.pulse_width == 'symmetric': high_time = instruction['step']/2 else: high_time = self.pulse_width

so the PB code is deciding where the returning edges are located just before programming, whereas the more logical time to make this decision would have been during the high-level compilation stage not the low-level stage.

----------------

-- Ian

[Philip Starkey]

Hi Ian,

I think you're suggesting assymetric clocking should be supported in labscript rather than specific labscript devices? That's ultimately the route of the problem I think. The Novatech specs allow for assymetric clocking and it's half baked in labscript.

Definitely sounds like a good idea! Probably not trivial though.

It also seems like a separate pseudoclock for the novatechs would solve the issue? I'm not familiar with exactly how you trigger your novatechs (and whether it's the same as how we did it at Monash), but the PrawnBlaster can generate 4 independent symmetrical pseudoclocks for $4 (+ circuitry to generate an appropriate external reference). I'd be tempted to just add a PrawnBlaster to every Novatech...

Cheers,

Phil

--
You received this message because you are subscribed to the Google Groups "the labscript suite" group.
To unsubscribe from this group and stop receiving emails from it, send an email to labscriptsuit...@googlegroups.com.
To view this discussion on the web, visit https://groups.google.com/d/msgid/labscriptsuite/b377149e-4ce1-40a7-a3b9-6bfee687e84an%40googlegroups.com.