QRNG serial driver

[stavros11]

First attempt to address #1136. It provides an interface to sample random floats in [0, 1], using a QRNG device connected to the host machine via a serial port:

qrng = QRNG(address="/dev/ttyACM0")

qrng.connect()

samples = qrng.random((500, 10))
# `samples` will be a numpy array with shape (500, 10)

qrng.disconnect()

Most likely there are things to be explored further, such as the extraction algorithm, the default parameter values and maybe there is even room for optimizations, however this is probably already fine as a baseline. It is also missing some tests.

Example application

qrng = QRNG(address="/dev/ttyACM0")

qrng.connect()

z = qrng.random((5000, 2))

qrng.disconnect()

r = z[:, 0] ** 2 + z[:, 1] ** 2

print("pi estimate:", 4 * np.mean(r < 1))

inside = r <= 1
outside = r > 1
plt.scatter(z[inside, 0], z[inside, 1], color="blue", s=1)
plt.scatter(z[outside, 0], z[outside, 1], color="red", s=1)
x = np.linspace(0, 1, 1000)
plt.plot(x, np.sqrt(1 - x**2), color="black")
plt.axis('equal')
plt.xlim(0, 1)
plt.ylim(0, 1)
plt.show()

Output:

pi estimate: 3.1208

This took about 13sec to run with the actual device and the current implementation (needs 5000 * 2 * 2 * (32 / 4) = 160k samples from the device).

[codecov]

Codecov Report

Attention: Patch coverage is 86.11111% with 20 lines in your changes missing coverage. Please review.

Project coverage is 52.35%. Comparing base (2e00ff1) to head (280b82e).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
src/qibolab/_core/instruments/qrng/qrng.py	60.46%	17 Missing ⚠️
...olab/_core/instruments/qrng/extractors/toeplitz.py	94.44%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1138      +/-   ##
==========================================
+ Coverage   50.74%   52.35%   +1.61%     
==========================================
  Files          63       70       +7     
  Lines        3019     3163     +144     
==========================================
+ Hits         1532     1656     +124     
- Misses       1487     1507      +20     

Flag	Coverage Δ
unittests	52.35% <86.11%> (+1.61%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[alecandido]

Is it mandatory to actually read one byte at a time?

[alecandido]

Just to clarify, I would have implemented the whole acquisition with something like:

        num_str = self.port.read(self.samples_per_number).decode()

[stavros11]

Several parts of the code, including this function, were copied from external scripts/tutorials on how to use the QRNG device. For your particular point, I agree and I would also try to do something similar, I would just prefer to have a better understanding of how it affects the results before starting to make changes. Also, probably worth mentioning that in the original code, self.samples_per_number is hard-coded to 4. Here I decided to make it a variable/instrument setting as I am not sure what is special about this 4.

In short, I am not sure about several things, so I decided to stick to existing code for a first iteration.

[alecandido]

This is the part I understand the least: why do you need pseudo-random numbers to generate random numbers?

Do you have a reference for the algorithm?

[stavros11]

Do you have a reference for the algorithm?

Some references from the original author of this code are https://arxiv.org/pdf/2402.09481 appendix A5 and https://github.com/CQCL/cryptomite, but I did not check in detail yet.

The outcome summarized is that the device produces 12-bit integers following a Gaussian distribution (these are accessible via qrng.read()) and the extraction procedure converts them to 4-bit integers following a uniform distribution (accessible via qrng.extract()). Then I concatenate multiple 4-bit samples to go to 32-bits (floats) (qrng.random()), which btw I am not sure if it is correct.

[alecandido]

Then I concatenate multiple 4-bit samples to go to 32-bits (floats) (qrng.random()), which btw I am not sure if it is correct.

This I guess it is: if it's uniform, it is also uniform over individual bits.
I.e. uniform means weighted with the area (original measure), and each bit divides the whole region in two equivalent regions (possibly disconnected), thus the two outcomes are equally likely. Conversely, if it's uniform over bits, it will also be uniform as a number.

the device produces 12-bit integers following a Gaussian distribution (these are accessible via qrng.read()) and the extraction procedure converts them to 4-bit integers following a uniform distribution (accessible via qrng.extract()).

Ok, I see.

The first thing that came to my mind was actually to make it more efficient, and to make sure the randomness was coming from the correct place.
But given that the authors are actually distributing a code, which I'm pretty sure is already catering for both these tasks, wouldn't be just simpler to use their code for the extraction?

[alecandido]

I'm not completely familiar with it, but mocker seems a fixture provided by the pytest-mock package, a Pytest plugin.

However, Pytest itself has its own mocking fixture, i.e. monkeypatch. Is there any advantage over using that?

[stavros11]

I'm not completely familiar with it, but mocker seems a fixture provided by the pytest-mock package, a Pytest plugin.

However, Pytest itself has its own mocking fixture, i.e. monkeypatch. Is there any advantage over using that?

I am also not very familiar with mocker, I basically copied it from

qibolab/tests/test_instruments_rfsoc.py

Line 385 in bfb48cc

mocker.patch("qibosoq.client.connect", return_value=server_results)

You are right it requires pytest-mock, but this is already in our dependencies for the tests group:

qibolab/pyproject.toml

Line 78 in bfb48cc

pytest-mock = ">=3.10.0"

I am guessing as a leftover from 0.1. If you wish to drop it, I can have a look at monkeypatch. I guess at some point we will use some mocking to test other drivers as well, so I would try to use the same everywhere.

In any case, it seems that CI failed due to an issue with the test itself, not the mocker. It is weird, as it is always passing locally, but I will investigate.

[alecandido]

I am guessing as a leftover from 0.1. If you wish to drop it, I can have a look at monkeypatch. I guess at some point we will use some mocking to test other drivers as well, so I would try to use the same everywhere.

Definitely not urgent, I just consider Pytest more reliable than its plugins. And tend to avoid any further dependency.
But, until a Pytest breaking release, most likely pytest-mock is also reliable.

In any case, it seems that CI failed due to an issue with the test itself, not the mocker. It is weird, as it is always passing locally, but I will investigate.

This I'd guess to be uncorrelated to mocker. If you wish/need, I can also have a look.

[scarrazza]

@stavros11 in view of results posted by @scarlehoff, could you please check with Jaideep about performance and more importantly run tests to validate that power supply is not disturbing hardware?

[stavros11]

I have added an additional extractor based on SHA-256, which is about 3x faster than the previously used Toeplitz approach, I guess because it requires less samples from the device per random float. Since it is faster, I made it the new default, however it is still possible to revert back to the previous behavior using:

from qibolab.instruments.qrng import QRNG, ToeplitzExtractor

qrng = QRNG("/dev/ttyACM0", extractor=ToeplitzExtractor())

It seems that there are several different choices for the extraction, which probably affect both performance and quality of samples, so I did a very quick abstraction to simplify testing different ones.

The next step is to find a figure of merit to validate the randomness of the results, which we can use to verify both whether the hardware is working well and also which extractor works better.

@scarlehoff you may be interested in this update.

[alecandido]

This comment is a little beyond the purpose of this PR, and more of a general statement about drivers' testing

I'm not even sure whether it is relevant to keep the option of testing on hardware through Pytest, since it will almost never happen (and in case, the device may happen to be mounted on a different path...).

Maybe we should collect all hardware tests in a completely separate folder.
Of course, we could use marks over folders, in order to privilege the current file structure. But while it's pretty nice to have CI tests as atomic as possible, for hardware integration tests should be more than fine (since they will be supplementary anyhow).

[stavros11]

I'm not even sure whether it is relevant to keep the option of testing on hardware through Pytest, since it will almost never happen (and in case, the device may happen to be mounted on a different path...).

The idea for this particular test is that it can run on both the CI (for coverage), in which case the exception is triggered and the mocker is used, but also locally in my computer with an actual QRNG plugged in USB. Currently it is passing in both cases. I agree about the path though, the idea that I had in mind was to pass it through command line options, however I did not go that far to implement it yet.

Maybe we should collect all hardware tests in a completely separate folder. Of course, we could use marks over folders, in order to privilege the current file structure. But while it's pretty nice to have CI tests as atomic as possible, for hardware integration tests should be more than fine (since they will be supplementary anyhow).

This issue of having pytests that cannot be executed in CI, seems to appear in several places, obviously in qibolab, but also in qibo (see GPU tests). I do not fully like the option of restricting pytest on CI available devices, because in that case it won't possible to test on the real devices even manually or with self-hosted runners, and this is also quite relevant. Using different folder sounds acceptable, however in that case I would try to avoid test code repetition as much as possible. For unit tests this should certainly be possible, I am not sure for more end-user tests.

This QRNG case is actually a great simple example. The end-user function is qrng.random which basically consists of qrng.read (requires hardware device) and extractor application (done fully on software) executed sequentially. Currently, mainly due to laziness, I added a single test which only tests the end-user case qrng.random and covers big chunk of the code. Alternatively, I could do unit tests for the extractors that are purely software and have a hardware test (in a different folder) that tests only read. In that case we will never be testing the full pipeline on hardware, though, which I am not sure if it is good or bad.

[alecandido]

Using different folder sounds acceptable, however in that case I would try to avoid test code repetition as much as possible.

I agree the problem is all about reducing code duplication.

To a certain extent, tests are not as critical as the package code - since they are only relevant to maintain it.
However, we still have to maintain them. So, reducing duplication is definitely relevant. Though, a mall amount of repetition is acceptable (while I have a zero-threshold for distributed code).

To be fair, I do not have a clear strategy to deduplicate test code. We may make use of the various conftest.py to share some code, but I would limit to a little amount. We may try to make more elaborate components still part of the tests/ package, but I'm not sure about that...
E.g. concerning Qblox mocking, for the time being, I made it part of the driver's subpackage itself. While I'm not fully convinced this is the way to go, in a sense it is not completely isolated, since even my beloved archetype package has a whole scope dedicated to it (i.e. numpy.testing).

This QRNG case is actually a great simple example. [...] I added a single test which only tests the end-user case qrng.random and covers big chunk of the code.

Well, this is in the direction of what I call integration tests, i.e. something that is testing some functionality of the code, more from the perspective of the end user. While unit tests are supposed to have the opposite focus, i.e. isolating the various components implemented.

Currently, mainly due to laziness

Well, laziness is also efficiency: saving effort on what is less needed will leave you more time to focus on priorities :P