labscript-suite / labscript

The 𝗹𝗮𝗯𝘀𝗰𝗿𝗶𝗽𝘁 library provides a translation from expressive Python code to low-level hardware instructions.
http://labscriptsuite.org
BSD 2-Clause "Simplified" License
9 stars 48 forks source link

Non-looping clock ticks may be too short if they happen during a ramp #39

Open philipstarkey opened 7 years ago

philipstarkey commented 7 years ago

Original report (archived issue) by Philip Starkey (Bitbucket: pstarkey, GitHub: philipstarkey).


There is a bug in labscript where the non-looping clock ticks can be too short.

This is because ramps (a looping clock) is broken up into two pseudoclock instructions*, the first where non-looping clocks and looping clocks tick, and then second where only looping clocks tick. However, the maximum rate is calculated based on the fastest ramp, and the maximum allowed ramping rate (the local_clock_limit) determined by ramping clockline. This then ignores the clock limit for clocklines that are not ramping, but need updating during the first tick of the ramp.

I'm not sure what the fix should be. We have options like:

  1. Force the maximum ramp rate to be limited by the clock limit of any non-ramping clocks that are commanded at the start/end or during a ramp (this is likely to make everyone's existing labscript files fail to compile)

  2. Somehow break the ramps at the pseudoclock device instruction level so that non-looping clocks have a period that matches the length of the looping clock ramp (this seems quite complicated to implement though as I think it breaks the current structure of the internal representation of pseudoclock instructions). Effectively this would mean we revert to the slow/fast clock system we had a long time ago (but it would be better because the slow/fast determination is made on a per ramp basis, and we'd still maintain gating)

  3. ??

Looking for other better suggestions!

Notes:

* by this I mean the internal labscript dictionary that describes the pseudoclock instructions, which may translate to more pseudoclock device instructions depending on how many device instructions you need per clock tick

philipstarkey commented 7 years ago

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


So I've thought about this a little more...

We'll need to implement different behaviour depending on the capabilities of the pseudoclock. For the purposes of the below explanation, I'll call it supports_slow_clock (permanent variable name to be decided later).

For pseudoclocks with a single clockline or pseudoclocks with multiple clocklines without independent control over the time at which the clocklines go high and low (an theoretical example of this is below*) we should throw an error when the clock will tick too fast for other clocklines. I believe this can be achieved by de-indenting the block of code here which is only checked for ramping (looping) clocklines at the moment, and putting it inside an if not self.supports_slow_clock: statement (note that the comment there is incorrect...while there is a check for non-looping clocks to ensure that instructions are not too close together, this ignores the fact that the clock tick will be at the rate of the fastest ramp at that time).

For devices like the PulseBlaster that can support multiple clocklines and has independent instructions for changing the high/low state of each clockline, we can change the format of the clock dictionary that stores the internal description of the pseudoclock instructions. Here we will modify these two lines such that we only have one clock instruction that contains a keys for looping clocks and non looping clocks. The device implementation of generate_code will then be modified to the the previous 4 hardware instructions (two for each of the instructions in clock) into 6. These 6 will be

  1. looping clocks and non-looping clocks go high
  2. looping clocks go low
  3. looping clocks go high (and jump to above instruction for N/2-1 repeats where N=Y-1 where Y is number of ticks in the ramp)
  4. looping and non-looping clocks go low
  5. looping clocks go high
  6. looping clocks go low (and jump to above instruction for N/2 repeats)

This is then followed by the existing two instructions (1 clock instruction) that do the final tick of the ramp at the rate that makes the clock ready for the following labscript instruction.

These additional instructions could be negated if there are no non-looping clocks present for this ramp.

Note that this would mean the non-looping clock was slightly asymmetric due to the longer clock tick at the end of the ramp. Does anyone think this would be a problem? Alternatively, if someone has a better solution, please present it!


* An example of a theoretical pseudoclock device with multiple clocklines but without independent control over the time at which each clockline goes high and low would be a PineBlaster with multiple clockline outputs (obviously this doesn't exist -- it's just an example!). Because the PineBlaster is programmed with a list of clock periods and number of loops for each instruction, it is impossible to command a clock-line to tick at an independent rate to another. You cannot have independent rates when you program the high-wait-low-wait (a clock tick) with a single instruction (#). This is in contrast to devices like the PulseBlaster, that have one instruction for setting an output high, and one instruction for setting an output low.

# Of course you could have a device that had two outputs with two independent sets of instructions, but that would imply a multi-threaded device (which the chipkit is not) such as a custom FPGA. In such a case, the labscript object model would be one pseudoclock device containing multiple pseudoclocks, and each pseudoclock would have one clockline. This bug only concerns itself with the case of a multiple clocklines on a pseudoclock (regardless of how many independent pseudoclocks there might be in the pseudoclock device)

philipstarkey commented 7 years ago

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


My comment above about the asymmetric clock tick on non-looping clocks, while accurate, is a bogus concern.

Currently any clock tick may be asymmetric due to gating (the low of the clock tick is regularly longer than the high because it stays low while other clocks tick). The current implementation for ticking non-looping clocks at the start of a ramp is also very asymmetric. My proposal actually makes it more symmetric (although not perfectly symmetric for the reasons previously outlined).

Thus, the only down side to this are: