Decompose protocol must create a context when given None and SimpleQubitManager must add a prefix to its qubits #6172
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…bitManager must add a prefix to its qubits
pavoljuhas
reviewed
Jun 28, 2023
| @@ -364,6 +367,11 @@ def decompose_once( | |||
| TypeError: `val` didn't have a `_decompose_` method (or that method returned | |||
| `NotImplemented` or `None`) and `default` wasn't set. | |||
| """ | |||
| if context is None: | |||
| context = DecompositionContext( | |||
| ops.SimpleQubitManager(prefix=f'_decompose_protocol_{time.time_ns()}') | |||
There was a problem hiding this comment.
Can we use some global counter instead of time_ns here?
It would be nice to have naming reproducibility in repeated executions.
Debugging and making sense of allocated qubit names should also get easier.
There was a problem hiding this comment.
added using itertools.count()
|
@pavoljuhas PTAL |
pavoljuhas
reviewed
Jun 30, 2023
pavoljuhas
approved these changes
Jun 30, 2023
harry-phasecraft
pushed a commit
to PhaseCraft/Cirq
that referenced
this pull request
Oct 31, 2024
…bitManager must add a prefix to its qubits (quantumlib#6172)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem:
Before this change successive calls to
cirq.decompose_oncecan break since they might endup making a gate act on the same qubits.The successive calls to
cirq.decompose_oncecan happen implicitly by calling other protocols. for examplesince
has_unitaryfalls todecompose_onceif the gate doesn't implement_has_unitary_. another case is when running a simulator on a decomposed operation. the simulator will callhas_unitaryin its checks.Solution: For
cirq.decompose_onceadd a unique prefix to the qubits allocated during decompose if decompose_once allocates its own qubit manager. forcirq.decomposeI think it is okay to have a constant prefix just to distinguish it from qubits allocated by other SimpleQubitManagers.