Error in dt comparison due to float64 precision

[RorySpeirs]

I have added a small margin to comparisons checking for the minimum instruction duration. In Runmanager, for particular requested transition times (that I knew to be allowed), I was getting this error:
File "C:\Users\rorys\anaconda3\lib\site-packages\labscript\labscript.py", line 828, in collect_change_times raise LabscriptError( labscript.labscript.LabscriptError: Commands have been issued to devices attached to Narwhal_Devices_Pulse_Generator_pseudoclock at t=3.0000000000000004e-08 and t=4e-08. This Pseudoclock cannot support update delays shorter than 1e-08 seconds. Compilation aborted.

Some "common" float64 values don't store "exactly". eg 40 nanoseconds compared with 30nanoseconds:
In [1]: 4*10E-9 Out[1]: 4e-08 In [2]: 3*10E-9 Out[2]: 3.0000000000000004e-08

So what is happening in the affected lines is this:
In [4]: (4*10E-9 - 3*10E-9) < 1*10E-9 Out[4]: True
Which raises the LabscriptError.

I have subtracted a small mI have added a small margin to comparisons checking for the minimum instruction duration. In Runmanager, for particular requested transition times (that I knew to be allowed), I was getting this error:
File "C:\Users\rorys\anaconda3\lib\site-packages\labscript\labscript.py", line 828, in collect_change_times raise LabscriptError( labscript.labscript.LabscriptError: Commands have been issued to devices attached to Narwhal_Devices_Pulse_Generator_pseudoclock at t=3.0000000000000004e-08 and t=4e-08. This Pseudoclock cannot support update delays shorter than 1e-08 seconds. Compilation aborted.

Some "common" float64 values don't store "exactly". eg 40 nanoseconds compared with 30nanoseconds:
In [1]: 4*10E-9 Out[1]: 4e-08 In [2]: 3*10E-9 Out[2]: 3.0000000000000004e-08

So what is happening in the affected lines is this:
In [4]: (4*10E-9 - 3*10E-9) < 1*10E-9 Out[4]: True
Which raises the LabscriptError.

I have subtracted a small margin from four instances where this seemed to likely affect the code. Only I only got errors for three of them, but it seemed likely that the 4th might just not have been "triggered" by the specific instructions I tried.

I'm not exactly sure how common these numbers are, but they aren't uncommon:
In [11]: a = np.arange(0, 100E-9, 10E-9) In [12]: [print(x) for x in a] Out [12]: 0.0 1e-08 2e-08 3.0000000000000004e-08 4e-08 5e-08 6.000000000000001e-08 7e-08 8e-08 9e-08

Other methods of adding a margin to account for float precision also worked, eg if dt < 1.0/self.clock_limit*0.999999999999999: works just fine. So use whatever you like. I prefer the explicit addition of epsilon.

I tested this only on Windows 11, running Python 3.8.16. Though I suspect it hasn't changed for newer versions.

[RorySpeirs]

After more experimentation, I realise this is not a good solution: it does not work for larger values of t.

The basic problem is that when checking if the increment is larger than the minimum dt, you are subtracting two floating point numbers that are possibly very similar in magnitude, and also potentially quite large. The larger the floating-point values get, the worse the precision.

I suggest not merging my pull request.

A better solution is to quantise all the times in integer number of dt=1.0/self.clock_limit.
Wherever this this quantisation is done, you can first do a floating point division, and then if the result is more than a certain distance from an integer, THEN you can raise an error. The integer would then be stored as an integer.

But I would have to look more closely at the code to see where to do this, and what side effects there would be.

Another possible solution (which might be cleaner) would be to convert all times for everything to an integer at the earliest possible stage. If everything was converted to an integer number of picoseconds. This would make the largest storable time with signed 64bit ints ~106 days. This is probably long enough. Or you could do integer numbers of nanoseconds, which is probably enough precision for most people, but maybe not everyone?

[philipstarkey]

Thanks @RorySpeirs ! As you say, integer storage of times is a much cleaner solution! I've logged it as #108. I'll close this for now as you don't think it should be merged. If you change your mind, or find another work around, feel free to open a new pull request 🙂