Stream line standard rb #391

wilkensJ · 2023-06-07T19:00:20Z

Checklist:

Reviewers confirm new code works as expected.
Tests are passing.
Coverage does not decrease.
Documentation is updated.

codecov · 2023-06-07T19:12:28Z

Codecov Report

Merging #391 (aeda2ff) into main (cae85be) will increase coverage by 0.78%.
The diff coverage is 98.97%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #391      +/-   ##
==========================================
+ Coverage   95.44%   96.23%   +0.78%     
==========================================
  Files          46       43       -3     
  Lines        2942     2866      -76     
==========================================
- Hits         2808     2758      -50     
+ Misses        134      108      -26

Flag	Coverage Δ
unittests	`96.23% <98.97%> (+0.78%)`	⬆️

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

Impacted Files	Coverage Δ
...cterization/randomized_benchmarking/standard_rb.py	`99.11% <98.75%> (-0.89%)`	⬇️
src/qibocal/auto/operation.py	`93.15% <100.00%> (+0.09%)`	⬆️
src/qibocal/auto/runcard.py	`97.82% <100.00%> (+0.04%)`	⬆️
src/qibocal/auto/task.py	`97.64% <100.00%> (ø)`
...haracterization/randomized_benchmarking/fitting.py	`93.54% <100.00%> (ø)`
...cterization/randomized_benchmarking/noisemodels.py	`100.00% <100.00%> (+22.22%)`	⬆️

... and 1 file with indirect coverage changes

Jacfomg

Also, can we add something saying that the layer_gen() allows the generation of proper random circuits to avoid confusion for the next people that will read the routines ? Just because my first thought was why were you not passing random_clifford() directly.

src/qibocal/protocols/characterization/randomized_benchmarking/standard_rb.py

vodovozovaliza · 2023-06-08T08:03:44Z

Would it be okay to add a runcard test_autocalibration_sim.yml with a numpy backend to test RB routines with noise models? Noise models simulation will not work with qibolab backend.

…/standard_rb.py Co-authored-by: Juan Cereijo <[email protected]>

Jacfomg

I think that for the time being if you try to specify noise_models but you have specified a platform not to have it change to a simulation backend on itself for now but check the platform and raise an error. I will look into that.

src/qibocal/protocols/characterization/randomized_benchmarking/standard_rb.py

Jacfomg · 2023-06-08T09:16:29Z

Would it be okay to add a runcard test_autocalibration_sim.yml with a numpy backend to test RB routines with noise models? Noise models simulation will not work with qibolab backend.

I would say yes for now, ideally we will want to have them both on the same runcard. I am not sure if how. Maybe runcards with both a global platform and backend and the routine chooses ?

Jacfomg · 2023-06-08T09:19:11Z

src/qibocal/protocols/characterization/randomized_benchmarking/standard_rb.py

 from qibocal.protocols.characterization.randomized_benchmarking.utils import (
    extract_from_data,
    random_clifford,
 )

-from .data import RBData
-from .params import RBParameters
+from .fitting import exp1B_func, fit_exp1B_func



Maybe a little example could be nice or will it be unnecessary @andrea-pasquale. Also specifies in a way what is Simulation or not.

Standard RB: Summary or paper ? An example action for using this routine is the following: qubits: [0, 1, 2] actions: - id: StdRB priority: 00 operation: standard_rb #main: parameters: nqubits: 5 qubits: [0,2] depths: start: 5 stop: 15 step: 3 niter: 5 nshots: 1024 noise_model: [Sim] noise_params: [Sim] """

For what does [Sim] stand for?:)

Simulation. Just thought of something to tell people which parameters they need for hardware and simulation, feel free to change it

You mean for the documentation? Yes totally! I mean the actions_qq_auto has the part already, but I totally agree, we have to add a documentation.

I know, I am also trying to think if we have new people joining, but maybe it's redundant.

Uhm, I would say that the best approach here would be to define a backend argument and to throw an error if the noise model is used with the qibolab backend. I still don't know if this is the best approach.

In #351 I decided to used the following approach to fix tests:

qibocal/tests/test_protocols.py

Lines 21 to 28 in 8e404f8

if "noise_model" in action["parameters"]:

yield {

"actions": [action],

"backend": "numpy",

"qubits": list(PLATFORM.qubits),

}

else:

yield {"actions": [action], "qubits": list(PLATFORM.qubits)}

Another idea could be to define a new type for those routine that are executable also in simulation.
I would say that this is out of the scope of this PR.

In principle, the noise model can also be applied through qibolab and is not at all restricted to simulation, or? Technically, it is just a transpiler that is run before passing the circuit to the platform. It is just that the current noise models will often result in circuits (with channels) that qibolab does not know how to compile, e.g. because probabilistic compilation is not implemented. A noise model with only coherent error for example, would already work. The reason we have the noise model parameters in the run card is because it actually also makes sense to allow for 'synthetic' noise when running things on the device. Thus, I think it might actually not be qibocal's job to catch the exception but qibolab's (in the sense it is already doing).

Thanks for the explanation @ingoroth.
I also agree that this should be addressed in qibolab (and perhaps even in qibo).
Until we solve the issue at least for qibocal we need to always use simulation to run a noise model.
We can merge this PR without this feature for the time being.

@andrea-pasquale can you show me what should be changed now for this PR? I am a bit lost in what should stay/be changed, sorry of the confusion.

Regarding this point you can open an issue if you want.
All the rest looks good to me.

…/standard_rb.py Co-authored-by: Juan Cereijo <[email protected]>

for more information, see https://pre-commit.ci

…/standard_rb.py Co-authored-by: Juan Cereijo <[email protected]>

for more information, see https://pre-commit.ci

…qibocal into standard_rb_streamline

…into standard_rb_streamline

…qibocal into standard_rb_streamline

…into standard_rb_streamline

…qibocal into standard_rb_streamline

andrea-pasquale

Thanks @wilkensJ @vodovozovaliza @Jacfomg for all the updates.
Generally the code looks good to me. You can find a few suggestions below.

src/qibocal/auto/operation.py

andrea-pasquale · 2023-06-09T06:27:23Z

src/qibocal/protocols/characterization/randomized_benchmarking/standard_rb.py

 from qibocal.protocols.characterization.randomized_benchmarking.utils import (
    extract_from_data,
    random_clifford,
 )

-from .data import RBData
-from .params import RBParameters
+from .fitting import exp1B_func, fit_exp1B_func



Uhm, I would say that the best approach here would be to define a backend argument and to throw an error if the noise model is used with the qibolab backend. I still don't know if this is the best approach.

In #351 I decided to used the following approach to fix tests:

qibocal/tests/test_protocols.py

Lines 21 to 28 in 8e404f8

if "noise_model" in action["parameters"]:

yield {

"actions": [action],

"backend": "numpy",

"qubits": list(PLATFORM.qubits),

}

else:

yield {"actions": [action], "qubits": list(PLATFORM.qubits)}

Another idea could be to define a new type for those routine that are executable also in simulation.
I would say that this is out of the scope of this PR.

src/qibocal/protocols/characterization/randomized_benchmarking/standard_rb.py

Jacfomg · 2023-06-09T14:27:43Z

Found an issue when ussing dummy that the probabilities get divided based on how many qubits you put on the parameters for the RB. Dummy seemed working properly, we may want to check what's happening next week with that.

This reverts commit 1173bb2.

Jacfomg · 2023-06-12T08:59:18Z

Also, now that we have the platform on the routine we can remove the nqubits as a parameter as @andrea-pasquale suggested

andrea-pasquale · 2023-06-12T11:41:18Z

src/qibocal/auto/runcard.py

+        set_backend(self.backend, self.platform)
        return construct_backend(self.backend, self.platform)


@stavros11 this is a temporary solution.
I don't know if there is a better way to do it. Currently it was not working since for the RB when executing circuits we don't pass the backend and by doing circuit() we will fall back to the default backend.
Currently I feel like this approach is a bit redundant. Do you have any suggestions?

andrea-pasquale

Thanks @vodovozovaliza @wilkensJ @Jacfomg.
Feel free to merge this PR.

streamline

12a5972

wilkensJ requested review from vodovozovaliza, scarrazza, andrea-pasquale, ingoroth and Jacfomg June 7, 2023 19:00

vodovozovaliza mentioned this pull request Jun 8, 2023

Error bars and uncertainties for Randomized Benchmarking #373

Merged

4 tasks

Jacfomg requested changes Jun 8, 2023

View reviewed changes

Fix tests

249e1e0

Update src/qibocal/protocols/characterization/randomized_benchmarking…

054965a

…/standard_rb.py Co-authored-by: Juan Cereijo <[email protected]>

Jacfomg requested changes Jun 8, 2023

View reviewed changes

Jacfomg reviewed Jun 8, 2023

View reviewed changes

wilkensJ and others added 15 commits June 8, 2023 11:23

Update src/qibocal/protocols/characterization/randomized_benchmarking…

329092e

…/standard_rb.py Co-authored-by: Juan Cereijo <[email protected]>

[pre-commit.ci] auto fixes from pre-commit.com hooks

313d70d

for more information, see https://pre-commit.ci

Update src/qibocal/protocols/characterization/randomized_benchmarking…

c3df68d

…/standard_rb.py Co-authored-by: Juan Cereijo <[email protected]>

Update src/qibocal/protocols/characterization/randomized_benchmarking…

7633224

…/standard_rb.py Co-authored-by: Juan Cereijo <[email protected]>

[pre-commit.ci] auto fixes from pre-commit.com hooks

1eeca6d

for more information, see https://pre-commit.ci

platform error when simulation prototype

14306d4

Merge branch 'standard_rb_streamline' of https://github.com/qiboteam/…

58fc569

…qibocal into standard_rb_streamline

Minor fix

d5b3b9b

fix tests

57586c1

Juans suggestions

15f8612

Merge branch 'standard_rb_streamline' of github.com:qiboteam/qibocal …

b99f926

…into standard_rb_streamline

fix tests

c516349

fix

10307b9

Merge branch 'standard_rb_streamline' of https://github.com/qiboteam/…

9e158d5

…qibocal into standard_rb_streamline

Merge branch 'standard_rb_streamline' of github.com:qiboteam/qibocal …

5698826

…into standard_rb_streamline

vodovozovaliza added 3 commits June 8, 2023 17:11

Check platform for noise models

a614b3c

Merge branch 'standard_rb_streamline' of https://github.com/qiboteam/…

dbee570

…qibocal into standard_rb_streamline

minor change

9cefb47

andrea-pasquale reviewed Jun 9, 2023

View reviewed changes

vodovozovaliza added 5 commits June 9, 2023 13:17

Andrea's suggestions

1a0d7d4

Minor change

3bb5ede

Fix tests

d204e36

Fix lint warnings

481a7e4

Minor change

6d2d573

vodovozovaliza and others added 8 commits June 12, 2023 11:06

Merge branch 'main' into standard_rb_streamline

00d9807

csv to json

87815fb

Fix tests

f72a097

Delete actions_qq.yml

1173bb2

Revert "Delete actions_qq.yml"

535049b

This reverts commit 1173bb2.

Delete actions_qq_auto.yml

04870bc

Set global backend

8d324e7

Minor change

75b6e98

Jacfomg approved these changes Jun 12, 2023

View reviewed changes

vodovozovaliza added 3 commits June 12, 2023 13:14

Remove nqubits

e4cd130

Remove optional nqubits from setup_scan

639ea3c

Fix coverage

aeda2ff

andrea-pasquale reviewed Jun 12, 2023

View reviewed changes

andrea-pasquale approved these changes Jun 13, 2023

View reviewed changes

vodovozovaliza added this pull request to the merge queue Jun 13, 2023

Merged via the queue into main with commit 10cbdf0 Jun 13, 2023

vodovozovaliza deleted the standard_rb_streamline branch June 13, 2023 09:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stream line standard rb #391

Stream line standard rb #391

wilkensJ commented Jun 7, 2023 •

edited by andrea-pasquale

Loading

codecov bot commented Jun 7, 2023 •

edited

Loading

Jacfomg left a comment

vodovozovaliza commented Jun 8, 2023

Jacfomg left a comment

Jacfomg commented Jun 8, 2023

Jacfomg Jun 8, 2023 •

edited

Loading

wilkensJ Jun 8, 2023

Jacfomg Jun 8, 2023

wilkensJ Jun 8, 2023

Jacfomg Jun 8, 2023

andrea-pasquale Jun 9, 2023

ingoroth Jun 9, 2023

andrea-pasquale Jun 9, 2023

wilkensJ Jun 12, 2023

andrea-pasquale Jun 12, 2023

andrea-pasquale left a comment

andrea-pasquale Jun 9, 2023

Jacfomg commented Jun 9, 2023

Jacfomg commented Jun 12, 2023

andrea-pasquale Jun 12, 2023

andrea-pasquale left a comment

	if "noise_model" in action["parameters"]:
	yield {
	"actions": [action],
	"backend": "numpy",
	"qubits": list(PLATFORM.qubits),
	}
	else:
	yield {"actions": [action], "qubits": list(PLATFORM.qubits)}

		set_backend(self.backend, self.platform)
		return construct_backend(self.backend, self.platform)

Stream line standard rb #391

Stream line standard rb #391

Conversation

wilkensJ commented Jun 7, 2023 • edited by andrea-pasquale Loading

codecov bot commented Jun 7, 2023 • edited Loading

Codecov Report

Jacfomg left a comment

Choose a reason for hiding this comment

vodovozovaliza commented Jun 8, 2023

Jacfomg left a comment

Choose a reason for hiding this comment

Jacfomg commented Jun 8, 2023

Jacfomg Jun 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrea-pasquale left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jacfomg commented Jun 9, 2023

Jacfomg commented Jun 12, 2023

Choose a reason for hiding this comment

andrea-pasquale left a comment

Choose a reason for hiding this comment

wilkensJ commented Jun 7, 2023 •

edited by andrea-pasquale

Loading

codecov bot commented Jun 7, 2023 •

edited

Loading

Jacfomg Jun 8, 2023 •

edited

Loading