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 CliffordSimulator to cirq.Sample #2600

Merged
merged 12 commits into from
Jan 23, 2020
Merged

Conversation

smitsanghavi
Copy link
Collaborator

  1. cirq.sample calls CliffordSimulator when appropriate.
  2. All simulators called by cirq.sample now support 0 repetitions even when measurements are not terminal
  3. ClifforSimulator exposes supported gates and supports Param resolution

@googlebot googlebot added the cla: yes Makes googlebot stop complaining. label Nov 26, 2019
@smitsanghavi
Copy link
Collaborator Author

#2423

Copy link
Collaborator

@vtomole vtomole left a comment

Choose a reason for hiding this comment

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

LGTM

cirq/sim/clifford/clifford_simulator_test.py Outdated Show resolved Hide resolved
@smitsanghavi smitsanghavi added the Ready for Re-Review For when reviewers take their time. label Dec 2, 2019
Copy link
Contributor

@Strilanc Strilanc left a comment

Choose a reason for hiding this comment

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

This is a good start, but we're going to need some pretty wide ranging changes to enable making this change. We need to define three new protocols:

  1. has_stabilizer_effect(self) -> bool

    Operations and other objects can implement _has_stabilizer_effect_ to indicate they are compatible with stabilizer simulation. We should add this to all existing stabilizer effects in Cirq, and also augment the standardized tests for gates to include a test like "if it has a stabilizer effect it better have the _has_stabilizer_effect_ method for efficiency" similar to the test we already have for has_unitary. If an object does not implement _has_stabilizer_effect_ the global has_stabilizer_effect method will fall back to calling or _decompose_into_stabilizer_operations_ or else falling back to calling _decompose_ and checking if all the resulting operations all have stabilizer effects.

  2. decompose_into_stabilizer_operations(self) -> List['cirq.Operation']

    Operations and other objects can implement _decompose_into_stabilizer_operations_ to specify a decomposition into operations with stabilizer effects.

    If an object does not implement _decompose_into_stabilizer_operations_, the global decompose_into_stabilizer_operations method will fall back to calling _decompose_.

  3. apply_stabilizer_effect_to_ch_form(self, args: 'cirq.ApplyStabilizerEffectToChFormArgs')

    "Atomic" operations that can't be decomposed into other stabilizer effects can specify this method for the Clifford simulator to call. The ApplyStabilizerEffectToChFormArgs would contain both the ch_form attribute and also a measurement record attribute.

We should include a unit test that defines a test gate that has a stabilizer effect via decomposition and via apply, and confirm that it works despite the simulator not knowing about it directly.

cirq/sim/mux.py Outdated Show resolved Hide resolved
cirq/sim/mux.py Outdated Show resolved Hide resolved
cirq/sim/mux.py Outdated

if TYPE_CHECKING:
import cirq


def _is_clifford_circuit(program: 'cirq.Circuit') -> bool:
return all(
op.gate in clifford_simulator.CliffordSimulator.get_supported_gates()
Copy link
Contributor

Choose a reason for hiding this comment

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

This check will fail on gates that decompose into Clifford operations, such as cirq.ISWAP.

I think what we need to do here is to define an is clifford protocol as well as formalize the decompose_into_clifford protocol. This line would then reduce to has_clifford(circuit).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a TODO for now

@Strilanc Strilanc removed the Ready for Re-Review For when reviewers take their time. label Dec 4, 2019
@smitsanghavi
Copy link
Collaborator Author

I agree with the need to implement all the changes you suggested to make this fully work as intended in #2423 . This is just the initial version which aims to get this to start working for the simplest eligible circuits. Added a TODO to document that this approach is not permanent.

@smitsanghavi smitsanghavi added the Ready for Re-Review For when reviewers take their time. label Dec 16, 2019
@smitsanghavi smitsanghavi removed the request for review from dstrain115 January 21, 2020 06:20
Copy link
Contributor

@Strilanc Strilanc left a comment

Choose a reason for hiding this comment

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

LGTM as an interim thing

@Strilanc Strilanc added automerge Tells CirqBot to sync and merge this PR. (If it's running.) and removed Ready for Re-Review For when reviewers take their time. labels Jan 23, 2020
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jan 23, 2020
@CirqBot CirqBot merged commit 28e3903 into quantumlib:master Jan 23, 2020
@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Jan 23, 2020
CirqBot pushed a commit that referenced this pull request Mar 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Makes googlebot stop complaining.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants