non interactive gate set characterization + action builder #239
Conversation
for more information, see https://pre-commit.ci
… into jadwiga/rb_architecture
for more information, see https://pre-commit.ci
… into jadwiga/rb_architecture
for more information, see https://pre-commit.ci
… into jadwiga/rb_architecture
for more information, see https://pre-commit.ci
…ons for every qubit gates
Simplify action builder + fix tests
For me the command is working. Also CI doesn't complain. Give it a try with a clean env to check if this error is still there. |
There was a problem hiding this comment.
Thanks for implementing this. A couple of quick comments regarding code style, I will have a more detailed look later.
The docs are indeed nice, particularly the figures. Would be great to use this in other parts of qibo docs. One thing I find weird is that the circuits drawings are inverted (measurements are on the left) compared to the common notation, however I am guessing you did that so the gate order matches the mathematical representation of the circuit (as product of operators).
| assert circuitfactory is None or isinstance( | ||
| circuitfactory, Iterable | ||
| ), """ | ||
| given circuit factory has wrong type {}, must be Iterable | None.""".format( | ||
| type(circuitfactory) | ||
| ) |
There was a problem hiding this comment.
I see that here and in other places below and in XIdrb.py you are using assert to catch errors. Usually we do something like
| assert circuitfactory is None or isinstance( | |
| circuitfactory, Iterable | |
| ), """ | |
| given circuit factory has wrong type {}, must be Iterable | None.""".format( | |
| type(circuitfactory) | |
| ) | |
| if circuitfactory is not None and not isinstance(circuitfactory, Iterable): | |
| raise TypeError("given circuit factory has wrong type {}, must be Iterable | None.".format(type(circuitfactory))) |
but I have no strong opinion on this.
There was a problem hiding this comment.
Is there any case where circuitfactory can be an arbitrary iterable or does it always need to be a Circuitfactory object? If it is the latter, why you don't check for Circuitfactory type directly instead of Iterable?
There was a problem hiding this comment.
I can be a list, or even another generator/iterator. The important thing is that is it iterable.
I can change the error handling.
|
I believe this is good to go, could you please confirm? |
|
The only open question from @andrea-pasquale is how to organize the files, else everything was cleared I think. |
|
@wilkensJ I would like to have these points solved before merging:
The other two about re-organizing folders and |
|
|
||
| def experiment_directory(name: str): | ||
| """Make the directory where the experiment will be stored.""" | ||
| from datetime import datetime |
There was a problem hiding this comment.
Generally prefer top level imports unless there is a strong motivation (eg. optional library) to import inside the method. Here datetime is standard library so I do not see an issue with importing on top. This comment may also be relevant in other parts of the code.
| from os import mkdir | ||
| from os.path import isdir |
There was a problem hiding this comment.
Usually I just import os and then use os.mkdir etc. but no strong opinion here.
| return depolp | ||
|
|
||
|
|
||
| def probabilities(allsamples: Union[list, np.ndarray]) -> np.ndarray: |
There was a problem hiding this comment.
I have not tracked exactly where this function is used, but if you are getting allsamples from qibo it may be simpler to use .frequencies to do this calculation (but probably you already know this).
| """ | ||
|
|
||
| def __init__( | ||
| self, nqubits: int, depths: list | np.ndarray | int, qubits: list = [] |
There was a problem hiding this comment.
Using lists (and generally mutable objects) as default arguments may lead to unexpected behavior. I believe this is not relevant here because you are not manipulating qubits in the function, so no change required, just letting you know in case you encounter this somewhere else.
|
@wilkensJ @andrea-pasquale if you can please implement Stavros suggestions so we can merge later today and include in the first release. |
Co-authored-by: Stavros Efthymiou <[email protected]>
|
I believe there is something wrong with one or more tests in this PR. |
Yes I know. I've approved it too soon. I will have a look now. |
|
Now it is ready to merge @scarrazza. |
|
Perfect, thank you. |
In this pull request the architecture and basic structure for a new way of implementing high level characterization for quantum hardware is implemented. We coined it 'Non-interactive Gate Set Characterization', short niGSC.
A new module can easily be implemented with that architecture using the predefined classes and functions along with a thorough documentation and explanation.
qqcan handle the new modules.A module consists of the following parts;
qibocal/calibration/niGSC/base/circuitfactoryp.y),qibocal/calibration/niGSC/base/experiment.py),qibocal/calibration/niGSC/base/plot.py,qibocal/calibration/niGSC/base/fitting.py.In the subfolder
qibocal/calibration/niGSC/base/the abstract classes and functions shared within all the modules are stored.A detailed explanation of a module can be found in the documentation
doc/source/getting-started/niGSC/niGSC_and_standardrb.rstexplaining the workflows inqibocal/calibration/niGSC/standardrb.py.How to write a module is explained in the comments of
qibocal/calibration/niGSC/XIdrb.py.Below a figure with a schematic diagram of niGSC can be found. First the instructions are put into qibo circuits, which are send to the hardware (or simulated) outputting data which, along with the used gates, are forming the gate set shadow. Different post processing methods calculate sequential and aggregational tasks (along with fitting mostly exponentials) which are then displayed in a report summarizing the found results of that protocol.