mitiq icon indicating copy to clipboard operation
mitiq copied to clipboard

Learn biased noise parameters

Open Misty-W opened this issue 2 years ago • 4 comments

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

Misty-W avatar Sep 27 '22 00:09 Misty-W

Binder :point_left: Launch a binder notebook on branch unitaryfund/mitiq/learning_based_PEC_w_noise_bias

github-actions[bot] avatar Sep 27 '22 00:09 github-actions[bot]

Codecov Report

Merging #1510 (77da702) into master (37fa18b) will increase coverage by 0.01%. The diff coverage is 100.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.

codecov[bot] avatar Sep 27 '22 01:09 codecov[bot]

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 avatar Sep 27 '22 03:09 Misty-W

@Misty-W sounds good to me

nathanshammah avatar Sep 27 '22 09:09 nathanshammah

Pushed by mistake. Converting to draft.

Misty-W avatar Oct 19 '22 00:10 Misty-W

To do:

  1. Generate data for remaining test cases for biased noise and remove script for data generation from PR (added by accident) --> should satisfy codecov
  2. Fix types --> should satisfy validate

Misty-W avatar Oct 19 '22 20:10 Misty-W

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.

Misty-W avatar Oct 26 '22 00:10 Misty-W

Thanks @Misty-W, nice to see this is working well! Two main comments:

  1. 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.

  2. 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 of pec_data and what it should contain. With the current status of docstrings, I think users would be confused by it.

andreamari avatar Oct 26 '22 13:10 andreamari

Thanks @Misty-W, nice to see this is working well! Two main comments:

  1. 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.
  2. 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 of pec_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.

Misty-W avatar Oct 26 '22 16:10 Misty-W

@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.

Misty-W avatar Oct 27 '22 13:10 Misty-W

@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.

Possible ways I had in mind are:

  1. 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.
  2. Passing and extracting pec_data via **minimize_kwargs . Technically pec_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.

andreamari avatar Oct 28 '22 10:10 andreamari

I'm puzzled as to why asv is failing- it doesn't call anything I've changed.

Misty-W avatar Nov 01 '22 17:11 Misty-W

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?

natestemen avatar Nov 01 '22 19:11 natestemen

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.

Misty-W avatar Nov 01 '22 20:11 Misty-W

@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.

Misty-W avatar Nov 02 '22 00:11 Misty-W

@andreamari and @natestemen, code is finally stable enough to review 😄 .

Misty-W avatar Nov 02 '22 04:11 Misty-W

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 and cast 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?

Misty-W avatar Nov 04 '22 20:11 Misty-W

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,
            )

Misty-W avatar Nov 07 '22 20:11 Misty-W

@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.

Misty-W avatar Nov 07 '22 23:11 Misty-W

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!

Misty-W avatar Nov 09 '22 22:11 Misty-W

hi @andreamari, any comments on this PR? Hope to close it this week or early next week at the latest :)

Misty-W avatar Nov 14 '22 22:11 Misty-W

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.

Misty-W avatar Nov 15 '22 19:11 Misty-W

@natestemen, I've addressed Andrea's comments. If you agree this can be merged now, can you approve? Thanks!

Misty-W avatar Nov 15 '22 19:11 Misty-W