Closed JeanOlivier closed 3 years ago
Hi JO, everything looks good code-wise. Could you please change the line endings to LF instead CRLF so that files such as the PSICT_MultiPulse.ini
do not look like the whole file has been changed?
Well, this gave me more trouble than I'd care to admit. I have edited most of the code on a Windows machine and I didn't configure git to handle line endings properly.
I've added a .gitattributes file for that purpose, but I can remove it from the PR if you want.
Let me know if there's anything else!
My pleasure. Feel free to contact me with comments or questions.
The original MultiPulse code derived from SQPG uses explicit time arrays of float64 dtype and compares the pulse time to that array to address the proper pulse indexes. While this is perfectly fine for simple sequences, this is extremely inefficient for long sequences with tens or hundreds of pulses. The code was spending most of its time computing values using the time array.
Furthermore, the use of float types makes some Boolean comparisons ambiguous. e.g. 0.999999999998123645 < 1 yields
True
whereas the left-hand term is likely a logical 1 with a numerical error and the output should rather beFalse
.This PR modifies the code so that it accesses arrays using indexes. It still uses float64 values internally to compute the indexes, but care is taken to round the resulting "time-targets" to the sampling step, which has a 1:1 relation with the index.
As an added optimisation, we also work on smaller sub-arrays when generating each pulse instead of handling potentially large and mostly empty arrays, avoiding countless
0+0=0
computations.Legacy functions are kept for comparison/test purposes. The generatePulse_legacy function was modified to also round the time arrays to the sampling step, without otherwise changing the comparison ambiguities.
DRAG-less Gaussian pulses trains are now within numerical accuracy of the original ones (~1e-17). However, pulses with a square envelope can differ. I believe the difference is caused by numerical-Boolean error in the legacy implementation. One can see that kind of numerical accuracy issue in the original SQPG implementation in an experiment with varying delays (e.g. Ramsey). In this case, the starting point of a square pulse will have a one-sample jitter depending on the spacing values of the previous pulses. The new Drag terms will be slightly different than the original ones, but should be slightly more accurate because they're trimmed after the gradient has been computed. One can reproduce the original values by trimming beforehand and zero-padding.
The quadrature signal of each waveform is also computed for use with IQ mixers if need be.
Overall, the code is much more efficient, from a few 10x to 10,000x+ depending of the amount of pulses and sequence length. The performance is almost independent of the actual sequence length, typically a few tens of ms to generate a sequence of 4 pulses with associated quadratures.
Great care was taken to compare the results of the new code to that of the original one, and I believe that most remaining difference between the two are now attributable to improved numerical accuracy. However more extensive testing is required to be sure of that. Any improvement are welcome!