Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add noise amplification transformer #6665

Merged
merged 16 commits into from
Jul 12, 2024
Merged

Conversation

eliottrosenberg
Copy link
Collaborator

Adds a tool for adding depolarizing noise after the specified gate, which can be used on hardware for probabilistic error amplification and zero noise extrapolation.

@eliottrosenberg eliottrosenberg requested review from vtomole, cduck and a team as code owners July 11, 2024 00:29
@CirqBot CirqBot added the size: M 50< lines changed <250 label Jul 11, 2024
@NoureldinYosri NoureldinYosri self-assigned this Jul 11, 2024
Copy link

codecov bot commented Jul 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.82%. Comparing base (5d22a53) to head (6a34db2).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6665   +/-   ##
=======================================
  Coverage   97.82%   97.82%           
=======================================
  Files        1066     1068    +2     
  Lines       91864    91919   +55     
=======================================
+ Hits        89862    89917   +55     
  Misses       2002     2002           

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

self,
p: float | Mapping[tuple[ops.Qid, ops.Qid], float],
target_gate: ops.Gate = ops.CZ,
rng: np.random.Generator | None = None,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you move rng to be an argument of __call__?

Returns:
The transformed circuit.

Raises:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you move the validation of p to the constructor? ... this way you don't need to do it here

import numpy as np


def _gate_in_moment(gate: ops.Gate, moment: circuits.Moment) -> bool:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit for performance:

return any(op.gate == gate for op in moment)

transformed_circuit_p1 = na.DepolerizingNoiseTransformer(1.0)(circuit)
assert len(transformed_circuit_p1) == 20
transformed_circuit_p0_03 = na.DepolerizingNoiseTransformer(0.03)(circuit)
assert 10 <= len(transformed_circuit_p0_03) <= 20
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't the expected len of the circuit num_moment * (1 + p) ? so here we expect ~$10 * 1.03 = 13$ and 20 correspends to the extreme $p = 1$ ... I think these tests are bad tests ... instead of doing this randomly call the transformer with a specific generator so that the result is always the same circuit and check equality with that circuit

"p must either be a float or a mapping from" # pragma: no cover
+ "sorted qubit pairs to floats" # pragma: no cover
) # pragma: no cover
self.p = p
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can resolve p here and then use it in call as self.p_func(pair)

Suggested change
self.p = p
self.p_func = lambda _: p if isinstance(p, (int, float)) else lambda pair: p.get(pair, 0)

Copy link
Collaborator

@NoureldinYosri NoureldinYosri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one optional suggestion

p_i = self.p_func(pair_sorted_tuple)
apply = rng.choice([True, False], p=[p_i, 1 - p_i])
if apply:
choices = [
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[optional suggestion] this creates 17 objects for each pair, 16 pauli pairs + the numpy array. consider doing it this way

pauli_a_idx = np.choice(4)
if pauli_a_idx == 0:
   pauli_b_idx = np.choice(3) + 1
else:
   pauli_b_idx = np.choice(4)
paulit_to_apply = paulis[pauli_a_idx](pair[0]), paulis[pauli_b_idx](pair[1])

Copy link
Collaborator Author

@eliottrosenberg eliottrosenberg Jul 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Nour. I agree that this would be more efficient code, but I'm going to leave it as is for now so that I can move on. (It is already very fast as written.)

@eliottrosenberg eliottrosenberg merged commit 3922a63 into main Jul 12, 2024
37 checks passed
@eliottrosenberg eliottrosenberg deleted the u/eliottrosenbrg/noise_adding branch July 12, 2024 17:41
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
* Add noise amplification transformer

* add _gate_in_moment

* add copyright notice

* add raises documentation, remove import cirq, fix coverage

* transformer api

* * after `circuit` input

Co-authored-by: Noureldin <[email protected]>

* format

* update test

* Convert to a class

* types and lint

* suggestions from Nour

* lint

* suggestion from Nour

* add ()

* types

* lint

---------

Co-authored-by: Noureldin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: M 50< lines changed <250
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants