mitiq
mitiq copied to clipboard
Learn biased noise parameters
Description
Reintroduce the two-parameter opt
For single parameter, use depolarizing
representations instead of biased_noise
representations w/ zero bias.
Clean up the tests, e.g. operation
does not need to be parameterized for now (only test CNOT)
License
- [X] I license this contribution under the terms of the GNU GPL, version 3 and grant Unitary Fund the right to provide additional permissions as described in section 7 of the GNU GPL, version 3.
Before opening the PR, please ensure you have completed the following where appropriate.
- [X] I added unit tests for new code.
- [X] I used type hints in function signatures.
- [X] I used Google-style docstrings for functions.
- [X] I updated the documentation where relevant.
- [X] Added myself / the copyright holder to the AUTHORS file
Codecov Report
Merging #1510 (77da702) into master (37fa18b) will increase coverage by
0.01%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #1510 +/- ##
==========================================
+ Coverage 98.21% 98.23% +0.01%
==========================================
Files 63 63
Lines 2921 2946 +25
==========================================
+ Hits 2869 2894 +25
Misses 52 52
Impacted Files | Coverage Δ | |
---|---|---|
mitiq/pec/representations/__init__.py | 100.00% <ø> (ø) |
|
mitiq/pec/representations/learning.py | 100.00% <100.00%> (ø) |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
Tests are passing but running long on this two-parameter version. Propose to pre-execute PEC and store the values in a file as previously discussed, at least for learning biased noise and possibly for the depolarizing version to offset the introduction of more optimization loops in the tests
@Misty-W sounds good to me
Pushed by mistake. Converting to draft.
To do:
- Generate data for remaining test cases for biased noise and remove script for data generation from PR (added by accident) --> should satisfy
codecov
- Fix types --> should satisfy
validate
Marking for review since the only remaining task I'm aware of is to add tests for eta
= 2.
It takes a long time to pre-generate the data, so I thought I'd open this PR for review while the PEC loop is running for the remaining cases.
Thanks @Misty-W, nice to see this is working well! Two main comments:
-
The new argument
pec_data
is now used for CI tests. But do you think it should be exposed to users? If not, we should find a way of hiding that argument. It's not straightforward, but I have some ideas. -
If instead we want
pec_data
to be exposed to users, we need to explain in a clear way in all docstrings what is the propose ofpec_data
and what it should contain. With the current status of docstrings, I think users would be confused by it.
Thanks @Misty-W, nice to see this is working well! Two main comments:
- The new argument
pec_data
is now used for CI tests. But do you think it should be exposed to users? If not, we should find a way of hiding that argument. It's not straightforward, but I have some ideas.- If instead we want
pec_data
to be exposed to users, we need to explain in a clear way in all docstrings what is the propose ofpec_data
and what it should contain. With the current status of docstrings, I think users would be confused by it.
Thanks @andreamari !
I'm undecided about exposing pec_data
to the user. Is it likely to be useful? I thought since execute_w_pec
takes a long time, users might want to use the pec_data
argument for similar reasons. Of course the main purpose is for faster execution in CI, so maybe it's not so helpful for real use cases.
@andreamari I’m curious to hear your suggestions for hiding pec_data
from the user. My first thought is to use something like use_pre_executed_data: bool = False
at the top level learning function (although the user would still see it) and if True
load the data from inside learning.py
.
@andreamari I’m curious to hear your suggestions for hiding
pec_data
from the user. My first thought is to use something likeuse_pre_executed_data: bool = False
at the top level learning function (although the user would still see it) and ifTrue
load the data from insidelearning.py
.
Possible ways I had in mind are:
- Making the existing top level learning functions private and defining new public functions as
learn_biased_noise_parameters = partial(_learn_biased_noise_parameters, pec_data=None)
and using only private functions in tests. - Passing and extracting
pec_data
via**minimize_kwargs
. Technicallypec_data
is not an option for minimize, but since we need it just for internal tests it can be valid trick.
We can do something similar for loss functions or not. Having pec_data
in loss functions is not as bad as having it in the top-level learning functions.
These were my initial ideas but I think your idea is good as well! Optionally, it could be combined with (2) to pass use_pre_executed_data
as an "invisible" keyword argument.
Note: If you think this PR is urgent, I am fine with merging it and with working on any solution for pec_data
in a new PR.
I'm puzzled as to why asv
is failing- it doesn't call anything I've changed.
I'm not sure why asv is failing, but it looks like there is a test failing related to your change. Is this ready for review?
I thought it was ready but let’s hold off for a day or so while I figure out what’s going on with the tests. Everything passed locally before my last push, but now one of my files is missing. Definitely causing pytest to fail but I’m not sure about ASV, should be unrelated. Strangely I can’t find where in the commit history the file was deleted. I’m in the process of regenerating the file because my local copy is gone too. Silly that I didn’t back up the data files outside my branch until now, not good practice on my part.
@andreamari and @natestemen, ~~PR is ready for review now that tests (except for the strange error on ASV) are passing~~.
PR is close to ready, but after I wrote the comment I realized Codecov is complaining about missing test coverage for lines 111 and 220. ~I already have tests where I don't specify anything for learning_kwargs
so I'm not sure whether or how I should fix this.~
Edit: fixing above with a private function for parsing the learning_kwargs
.
Also I'm 95% sure the ASV failure is unrelated to the changes on this PR, as the error message is "No executable found for python 3.8" and I haven't changed anything that's called in the ASV benchmarks.
@andreamari and @natestemen, code is finally stable enough to review 😄 .
Nice work Misty, it's cool to see this get built out and be more functional.
Thanks for the review @natestemen!
A few comments:
- My biggest question right now is about the
pec_data
array that gets passed around. I wonder if the data should be stored in a different datatype to avoid all the index manipulation that goes with storing the data as a 3d array. If I understand correctly, each "layer" of the array has a different semantic meaning (Is that right?), so storing them all together isn't necessary.
You're correct in the case of biased noise, each layer is for a different training circuit. They don't have to be stored together, but I'm not sure what that saves, since each layer would have to be extracted from learning_kwargs
anyway.
- Try to avoid using the
# type: ignore
comments andcast
calls wherever possible.
Agreed, I think they're all gone now.
- Wondering if we should have a test where we use all of the defaults, rather than specifying them. Then if down the line we change the defaults we ensure they get tested. This also has the benefit of showcasing how simple we can make a test.
I have a test where I almost do this, passing an empty dictionary to _parse_learning_kwargs
. I strongly prefer to avoid running PEC live in an optimization loop on GitHub because it takes too long, so a full test w/ defaults isn't really practical.
- Storing all of the test data in the repo is okay, but not ideal. Did you consider any other approaches? Why
txt
files and not a more data-oriented format?
I'm not sure of other practical options for storing the data other than in the repo. We need to use pre-executed data so execute_with_pec
isn't called many times where tests are running on GitHub, especially for biased noise. Even running depolarizing noise in real time caused our tests to run long. I am open to using other formats, txt
is just the one I'm most familiar with. Any in particular you would suggest?
Recording on the PR how the test data was generated, thanks for the suggestion @natestemen !
# Script to generate error-mitigated expectation values from training circuits
# for learning-based PEC tests
import os
import numpy as np
from cirq import (
CXPowGate,
MixedUnitaryChannel,
I,
X,
Y,
Z,
LineQubit,
Circuit,
ops,
unitary,
)
from mitiq import Executor, Observable, PauliString
from mitiq.interface.mitiq_cirq import compute_density_matrix
from mitiq.cdr import generate_training_circuits
from mitiq.cdr._testing import random_x_z_cnot_circuit
from mitiq.pec import execute_with_pec
from mitiq.pec.representations import (
represent_operation_with_local_biased_noise,
)
circuit = random_x_z_cnot_circuit(
LineQubit.range(2), n_moments=5, random_state=np.random.RandomState(1)
)
CNOT_all_ops = list(circuit.findall_operations_with_gate_type(CXPowGate))
CNOT_op = CNOT_all_ops[0]
index = CNOT_op[0]
op = CNOT_op[1]
num_training_circuits = 5
training_circuits = generate_training_circuits(
circuit=circuit,
num_training_circuits=num_training_circuits,
fraction_non_clifford=0.2,
random_state=np.random.RandomState(1),
)
def biased_noise_channel(epsilon: float, eta: float) -> MixedUnitaryChannel:
a = 1 - epsilon
b = epsilon * (3 * eta + 1) / (3 * (eta + 1))
c = epsilon / (3 * (eta + 1))
mix = [
(a, unitary(I)),
(b, unitary(Z)),
(c, unitary(X)),
(c, unitary(Y)),
]
return ops.MixedUnitaryChannel(mix)
observable = Observable(PauliString("XZ"), PauliString("YY"))
true_epsilons = [0.05, 0.1]
true_etas = [0, 1, 2]
num_epsilons = num_etas = 121
for true_eta in true_etas:
for true_epsilon in true_epsilons:
# We assume the operation "op" appears just once in the circuit such
# that it's enough to add a single noise channel after that operation.
def noisy_execute(circ: Circuit) -> np.ndarray:
noisy_circ = circ.copy()
qubits = op.qubits
for q in qubits:
noisy_circ.insert(
index + 1,
biased_noise_channel(true_epsilon, true_eta)(q),
)
return compute_density_matrix(noisy_circ, noise_level=(0.0,))
noisy_executor = Executor(noisy_execute)
epsilons = np.linspace(true_epsilon - 0.03, true_epsilon + 0.03, num_epsilons)
etas = np.linspace(true_eta * 0.8, true_eta * 1.4, num_etas)
# 2-D array w/ dim1 noise strenth, dim2 noise bias
pec_values = np.zeros([num_epsilons + 1, num_etas + 1], dtype=float)
pec_values[1:, 0] = epsilons
pec_values[0, 1:] = etas
for tc in range(5):
pec_data = pec_values
for et in enumerate(etas):
for eps in enumerate(epsilons):
reps = [
represent_operation_with_local_biased_noise(
ideal_operation=Circuit(op),
epsilon=eps[1],
eta=et[1],
)
]
pec_data[eps[0] + 1, et[0] + 1] = execute_with_pec(
circuit=training_circuits[tc],
executor=noisy_executor,
observable=observable,
representations=reps,
num_samples=600,
random_state=np.random.RandomState(1),
)
eps_string = str(true_epsilon).replace(".", "_")
np.savetxt(
os.path.join(
"./mitiq/pec/representations/tests/learning_pec_data",
f"learning_pec_data_eps_{eps_string}eta_{true_eta}tc_{tc}.txt",
),
pec_data,
)
@natestemen and @andreamari, I think I addressed all of the comments, anything else that needs to be addressed before merging?
Side note that I created an issue for the intermittent problem uploading results to Codecov for the linux tests on python 3.10, #1584. Don't think I'll have time to look into it this week, but maybe next week.
About the test data:
Generally, the data files are text files containing error-mitigated expectation values evaluated for the 5 training circuits, at different values of noise strength and noise bias. That is, the representations are calculated at different values of epsilon
and eta
and then used in executing each of the training circuits with PEC.
The noise strength and noise bias values can be thought of each as linear 1-D arrays that start and stop slightly below and above the true values of noise bias and noise strength, with small increments in between elements. The true values are what we use in the noise model of the noisy executor, and thus what we hope the optimizer will return (in these contrived test cases)!
For depolarizing noise, we only vary noise strength of course. The leftmost column contains the value of noise strength corresponding to each row. Each training circuit has its own column of PEC-mitigated values, evaluated at different noise strengths. We have two of these files, learning_pec_data_eps_0_05.txt
and learning_pec_data_eps_0_1.txt
.
For biased noise, each training circuit has its own file of PEC-mitigated expectation values. As in depolarizing noise, the leftmost column contains the value of noise strength corresponding to each row. Now we also vary noise bias, so the top row contains the value of noise bias corresponding to each column. We have 20 of these files (5 training circuits x 4 combinations of noise strength and noise bias).
Hope this clears it up @natestemen!
hi @andreamari, any comments on this PR? Hope to close it this week or early next week at the latest :)
Sorry for the late review, @Misty-W. LGTM! Please consider the following comments, apart from double-checking the consistency of skipping the first element in pec_data, as optional suggestions.
Thanks @andreamari! Good questions and suggestions, I think they're all addressed now.
@natestemen, I've addressed Andrea's comments. If you agree this can be merged now, can you approve? Thanks!