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 random quantum circuit generation #2621

Merged
merged 20 commits into from
Feb 12, 2020

Conversation

kevinsung
Copy link
Collaborator

Adapted from pyle.

Let me know if the name random_quantum_circuit is too generic. What would be a better name?

@XiaoMiQC

@googlebot googlebot added the cla: yes Makes googlebot stop complaining. label Dec 5, 2019
@kevinsung
Copy link
Collaborator Author

mypy complains

cirq/experiments/random_quantum_circuit.py:224: error: List comprehension has incompatible type List[Operation]; expected List[GateOperation]

but #2626

@kevinsung
Copy link
Collaborator Author

Actually I modified the code so mypy no longer complains.

@@ -30,6 +30,13 @@
xeb_fidelity,
)

from cirq.experiments.random_quantum_circuit import (
Copy link
Contributor

Choose a reason for hiding this comment

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

The name of this code needs to be more descriptive. "Random quantum circuit" can mean a lot of things. Make it like "random_rotations_between_interaction_layers" or something. Be as descriptive as possible... although the name should fit on a line.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we could keep the module name as is or perhaps just add "generation" to it (as in random_quantum_circuit_generation or random_circuit_generation) with the intention for any future RQC-generation code to be added here as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I renamed the module to random_quantum_circuit_generation

Copy link
Collaborator Author

@kevinsung kevinsung Feb 7, 2020

Choose a reason for hiding this comment

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

@Strilanc you okay with @viathor's suggestion?

cirq/experiments/random_quantum_circuit_test.py Outdated Show resolved Hide resolved
cirq/experiments/random_quantum_circuit_test.py Outdated Show resolved Hide resolved
cirq/experiments/random_quantum_circuit.py Outdated Show resolved Hide resolved
cirq/experiments/random_quantum_circuit.py Outdated Show resolved Hide resolved
cirq/experiments/random_quantum_circuit.py Outdated Show resolved Hide resolved
@@ -30,6 +30,13 @@
xeb_fidelity,
)

from cirq.experiments.random_quantum_circuit import (
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we could keep the module name as is or perhaps just add "generation" to it (as in random_quantum_circuit_generation or random_circuit_generation) with the intention for any future RQC-generation code to be added here as well.

GMON_EASY_PATTERN,
GMON_HARD_PATTERN,
GmonLayer,
random_quantum_circuit,
Copy link
Collaborator

Choose a reason for hiding this comment

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

As suggested by @Strilanc above, we should name this more specific. This will also make the module future-proof by preventing name collisions. Example name for the function: make_layered_random_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.

I renamed it random_rotations_between_grid_interaction_layers_circuit.

@kevinsung kevinsung added the Ready for Re-Review For when reviewers take their time. label Feb 3, 2020
@kevinsung
Copy link
Collaborator Author

Windows build is failing.

Copy link
Collaborator

@viathor viathor 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 some minor nits.

cirq/experiments/__init__.py Outdated Show resolved Hide resolved
cirq/experiments/random_quantum_circuit_generation.py Outdated Show resolved Hide resolved
cirq/experiments/random_quantum_circuit_generation.py Outdated Show resolved Hide resolved
single_qubit_gates: Sequence['cirq.Gate'],
previous_single_qubit_layer: Dict['cirq.GridQubit', 'cirq.Gate'],
prng: 'np.random.RandomState',
) -> Dict['cirq.GridQubit', 'cirq.Gate']:
Copy link
Collaborator

Choose a reason for hiding this comment

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

For defense-in-depth, you could check consistency of qubits and previous layer:

if not previous_single_qubit_layer and set(qubits) != set(previous_single_qubit_layer.keys()):
    raise ValueError(...)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think you put the not there by mistake. I think this is quite unnecessary; I'd rather omit it.

@kevinsung kevinsung added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Feb 7, 2020
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Feb 7, 2020
@CirqBot
Copy link
Collaborator

CirqBot commented Feb 7, 2020

Automerge cancelled: Need a fresh 🍪.

@CirqBot CirqBot removed the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Feb 7, 2020
@CirqBot
Copy link
Collaborator

CirqBot commented Feb 7, 2020

Automerge cancelled: The automerge label was removed.

@CirqBot CirqBot removed the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Feb 7, 2020
vtomole
vtomole previously requested changes Feb 7, 2020
cirq/experiments/random_quantum_circuit.py Outdated Show resolved Hide resolved
@kevinsung kevinsung dismissed vtomole’s stale review February 12, 2020 03:35

Comment addressed.

@kevinsung kevinsung added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Feb 12, 2020
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Feb 12, 2020
@CirqBot
Copy link
Collaborator

CirqBot commented Feb 12, 2020

Automerge cancelled: Need a fresh 🍪.

@CirqBot CirqBot removed the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Feb 12, 2020
@CirqBot
Copy link
Collaborator

CirqBot commented Feb 12, 2020

Automerge cancelled: The automerge label was removed.

@CirqBot CirqBot removed the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Feb 12, 2020
@kevinsung kevinsung merged commit ce13a73 into quantumlib:master Feb 12, 2020
@kevinsung kevinsung deleted the xeb_circuit branch February 12, 2020 22:07
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. Ready for Re-Review For when reviewers take their time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants